Skip to content

Commit

Permalink
Implement user style sheets
Browse files Browse the repository at this point in the history 
This is based on the design document at https://goo.gl/jzD8Zk

Browser extension developers expect their style sheets to be treated as
user style sheets, as "!important" user declarations have a higher
precedence than "!important" author declarations.

Therefore, in order to implement this behavior, this patch makes the
following changes:

 1.  MatchResult now maintains user rules in addition to UA and author
     rules
 2.  DocumentStyleSheetCollection no longer cares about injected style
     sheets
 3.  StyleEngine now treats injected style sheets as user style sheets
 4.  StyleResolver now calls its new MatchUserRules method right after
     calling MatchUARules
 5.  The ApplyMatchedStandardProperties method of StyleResolver applies
     important properties in the correct order

BUG=632009

Change-Id: If752b1af762f233dae49bb5bffd6d5e6a4b54acd
Reviewed-on: https://chromium-review.googlesource.com/641294
Commit-Queue: Rune Lillesveen <rune@opera.com>
Reviewed-by: Rune Lillesveen <rune@opera.com>
Cr-Commit-Position: refs/heads/master@{#509096}
  • Loading branch information
Manish Jethani authored and Commit Bot committed Oct 16, 2017
1 parent e4488e2 commit 30168b9
Show file tree
Hide file tree
Showing 14 changed files with 408 additions and 134 deletions.
1 change: 1 addition & 0 deletions AUTHORS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ Mahesh Machavolu <mahesh.ma@samsung.com>
Maksim Sisov <maksim.sisov@intel.com>
Malcolm Wang <malcolm.2.wang@gmail.com>
Manish Chhajer <chhajer.m@samsung.com>
Manish Jethani <m.jethani@eyeo.com>
Manojkumar Bhosale <manojkumar.bhosale@imgtec.com>
Manuel Braun <thembrown@gmail.com>
Mao Yujie <maojie0924@gmail.com>
Expand Down
50 changes: 25 additions & 25 deletions ...y/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/inject-stylesheet-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ element.style { ()

[expanded]
#main { (injected stylesheet)
color: red;
-webkit-border-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAQAAAC1+jfqAAAAiElEQ…Ms+LS30CAhBN5nNxeT5hbJ1zwmji2k+aF6NENIPf/hs54f0sZFUVAMigAAAABJRU5ErkJggg==);
color: red;
border-style: solid;
border-top-style: solid;
border-right-style: solid;
Expand All @@ -70,35 +70,35 @@ div { (user agent stylesheet)

iframe style:
background-attachment: scroll;
initial - #iframeBody injected stylesheet
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-clip: border-box;
initial - #iframeBody injected stylesheet
background-clip: border-box;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
initial - #iframeBody injected stylesheet
background-color: rgb(255, 0, 0);
red - #iframeBody injected stylesheet
OVERLOADED green - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
red - #iframeBody injected stylesheet
background-image: none;
initial - #iframeBody injected stylesheet
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-origin: padding-box;
initial - #iframeBody injected stylesheet
background-origin: padding-box;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-position-x: 0%;
initial - #iframeBody injected stylesheet
background-position-x: 0%;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-position-y: 0%;
initial - #iframeBody injected stylesheet
background-position-y: 0%;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-repeat-x: ;
initial - #iframeBody injected stylesheet
background-repeat-x: ;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-repeat-y: ;
initial - #iframeBody injected stylesheet
background-repeat-y: ;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
background-size: auto;
initial - #iframeBody injected stylesheet
background-size: auto;
OVERLOADED initial - body inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9
initial - #iframeBody injected stylesheet
display: block;
block - body user agent stylesheet
margin-bottom: 8px;
Expand All @@ -112,20 +112,6 @@ margin-top: 8px;
[expanded]
element.style { ()

[expanded]
#iframeBody { (injected stylesheet)
background: red;
background-image: initial;
background-position-x: initial;
background-position-y: initial;
background-size: initial;
background-repeat-x: initial;
background-repeat-y: initial;
background-attachment: initial;
background-origin: initial;
background-clip: initial;
background-color: red;

[expanded]
body { (inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4:9)
/-- overloaded --/ background: green;
Expand All @@ -140,6 +126,20 @@ body { (inject-styleshe…me-data.html:4 -> inject-stylesheet-iframe-data.html:4
/-- overloaded --/ background-clip: initial;
/-- overloaded --/ background-color: green;

[expanded]
#iframeBody { (injected stylesheet)
background: red !important;
background-image: initial;
background-position-x: initial;
background-position-y: initial;
background-size: initial;
background-repeat-x: initial;
background-repeat-y: initial;
background-attachment: initial;
background-origin: initial;
background-clip: initial;
background-color: red;

[expanded]
body { (user agent stylesheet)
display: block;
Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/LayoutTests/http/tests/devtools/elements/styles-2/inject-stylesheet.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

function injectStyleSheet(context)
{
var styleSheet = "#main { color: red; border-style: solid; -webkit-border-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAQAAAC1+jfqAAAAiElEQVR42r2RsQrDMAxEBRdl8SDcX8lQPGg1GBI6lvz/h7QyRRXV0qUULwfvwZ1tenw5PxToRPWMC52eA9+WDnlh3HFQ/xBQl86NFYJqeGflkiogrOvVlIFhqURFVho3x1moGAa3deMs+LS30CAhBN5nNxeT5hbJ1zwmji2k+aF6NENIPf/hs54f0sZFUVAMigAAAABJRU5ErkJggg==) } #iframeBody { background: red }";
var styleSheet = "#main { color: red !important; border-style: solid; -webkit-border-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAQAAAC1+jfqAAAAiElEQVR42r2RsQrDMAxEBRdl8SDcX8lQPGg1GBI6lvz/h7QyRRXV0qUULwfvwZ1tenw5PxToRPWMC52eA9+WDnlh3HFQ/xBQl86NFYJqeGflkiogrOvVlIFhqURFVho3x1moGAa3deMs+LS30CAhBN5nNxeT5hbJ1zwmji2k+aF6NENIPf/hs54f0sZFUVAMigAAAABJRU5ErkJggg==) } #iframeBody { background: red !important }";
if (context.testRunner)
context.testRunner.insertStyleSheet(styleSheet);
}
Expand Down
6 changes: 0 additions & 6 deletions third_party/WebKit/Source/core/css/DocumentStyleSheetCollection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,6 @@ void DocumentStyleSheetCollection::CollectStyleSheetsFromCandidates(
void DocumentStyleSheetCollection::CollectStyleSheets(
StyleEngine& master_engine,
DocumentStyleSheetCollector& collector) {
for (auto& sheet :
GetDocument().GetStyleEngine().InjectedAuthorStyleSheets()) {
collector.AppendActiveStyleSheet(std::make_pair(
sheet.second,
GetDocument().GetStyleEngine().RuleSetForSheet(*sheet.second)));
}
CollectStyleSheetsFromCandidates(master_engine, collector);
if (CSSStyleSheet* inspector_sheet =
GetDocument().GetStyleEngine().InspectorStyleSheet()) {
Expand Down
3 changes: 3 additions & 0 deletions third_party/WebKit/Source/core/css/ElementRuleCollector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ class ElementRuleCollector {
void AddElementStyleProperties(const StylePropertySet*,
bool is_cacheable = true);
void FinishAddingUARules() { result_.FinishAddingUARules(); }
void FinishAddingUserRules() {
result_.FinishAddingUserRules();
}
void FinishAddingAuthorRulesForTreeScope() {
result_.FinishAddingAuthorRulesForTreeScope();
}
Expand Down
132 changes: 93 additions & 39 deletions third_party/WebKit/Source/core/css/StyleEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "core/css/resolver/StyleRuleUsageTracker.h"
#include "core/css/resolver/ViewportStyleResolver.h"
#include "core/dom/Element.h"
#include "core/dom/ElementShadow.h"
#include "core/dom/ElementTraversal.h"
#include "core/dom/ProcessingInstruction.h"
#include "core/dom/ShadowRoot.h"
Expand Down Expand Up @@ -139,21 +140,20 @@ StyleEngine::StyleSheetsForStyleSheetList(TreeScope& tree_scope) {
return collection.StyleSheetsForStyleSheetList();
}

WebStyleSheetId StyleEngine::InjectAuthorSheet(
StyleSheetContents* author_sheet) {
injected_author_style_sheets_.push_back(
std::make_pair(++injected_author_sheets_id_count_,
CSSStyleSheet::Create(author_sheet, *document_)));
WebStyleSheetId StyleEngine::AddUserSheet(StyleSheetContents* sheet) {
user_style_sheets_.push_back(
std::make_pair(++user_sheets_id_count_,
CSSStyleSheet::Create(sheet, *document_)));

MarkDocumentDirty();
return injected_author_sheets_id_count_;
MarkUserStyleDirty();
return user_sheets_id_count_;
}

void StyleEngine::RemoveInjectedAuthorSheet(WebStyleSheetId author_sheet_id) {
for (size_t i = 0; i < injected_author_style_sheets_.size(); ++i) {
if (injected_author_style_sheets_[i].first == author_sheet_id) {
injected_author_style_sheets_.EraseAt(i);
MarkDocumentDirty();
void StyleEngine::RemoveUserSheet(WebStyleSheetId sheet_id) {
for (size_t i = 0; i < user_style_sheets_.size(); ++i) {
if (user_style_sheets_[i].first == sheet_id) {
user_style_sheets_.EraseAt(i);
MarkUserStyleDirty();
}
}
}
Expand Down Expand Up @@ -339,6 +339,21 @@ void StyleEngine::UpdateActiveStyleSheetsInShadow(
}
}

void StyleEngine::UpdateActiveUserStyleSheets() {
DCHECK(user_style_dirty_);

ActiveStyleSheetVector new_active_sheets;
for (auto& sheet : user_style_sheets_) {
if (RuleSet* rule_set = RuleSetForSheet(*sheet.second))
new_active_sheets.push_back(std::make_pair(sheet.second, rule_set));
}

ApplyRuleSetChanges(*document_, active_user_style_sheets_, new_active_sheets,
kInvalidateAllScopes);

new_active_sheets.swap(active_user_style_sheets_);
}

void StyleEngine::UpdateActiveStyleSheets() {
if (!NeedsActiveStyleSheetUpdate())
return;
Expand All @@ -349,6 +364,9 @@ void StyleEngine::UpdateActiveStyleSheets() {

TRACE_EVENT0("blink,blink_style", "StyleEngine::updateActiveStyleSheets");

if (user_style_dirty_)
UpdateActiveUserStyleSheets();

if (ShouldUpdateDocumentStyleSheetCollection())
GetDocumentStyleSheetCollection().UpdateActiveStyleSheets(*this);

Expand All @@ -372,6 +390,7 @@ void StyleEngine::UpdateActiveStyleSheets() {
document_scope_dirty_ = false;
all_tree_scopes_dirty_ = false;
tree_scopes_removed_ = false;
user_style_dirty_ = false;
}

void StyleEngine::UpdateViewport() {
Expand Down Expand Up @@ -554,6 +573,11 @@ void StyleEngine::MarkDocumentDirty() {
GetDocument().ScheduleLayoutTreeUpdateIfNeeded();
}

void StyleEngine::MarkUserStyleDirty() {
user_style_dirty_ = true;
GetDocument().ScheduleLayoutTreeUpdateIfNeeded();
}

CSSStyleSheet* StyleEngine::CreateSheet(Element& element,
const String& text,
TextPosition start_position,
Expand Down Expand Up @@ -929,7 +953,8 @@ void StyleEngine::InvalidateSlottedElements(HTMLSlotElement& slot) {

void StyleEngine::ScheduleInvalidationsForRuleSets(
TreeScope& tree_scope,
const HeapHashSet<Member<RuleSet>>& rule_sets) {
const HeapHashSet<Member<RuleSet>>& rule_sets,
InvalidationScope invalidation_scope) {
#if DCHECK_IS_ON()
// Full scope recalcs should be handled while collecting the ruleSets before
// calling this method.
Expand Down Expand Up @@ -963,6 +988,16 @@ void StyleEngine::ScheduleInvalidationsForRuleSets(
if (invalidate_slotted && IsHTMLSlotElement(element))
InvalidateSlottedElements(ToHTMLSlotElement(*element));

if (invalidation_scope == kInvalidateAllScopes) {
ElementShadow* shadow = element->Shadow();
ShadowRoot* shadow_root = shadow ? &shadow->OldestShadowRoot() : nullptr;
while (shadow_root) {
ScheduleInvalidationsForRuleSets(*shadow_root, rule_sets,
kInvalidateAllScopes);
shadow_root = shadow_root->YoungerShadowRoot();
}
}

if (element->GetStyleChangeType() < kSubtreeStyleChange)
element = ElementTraversal::Next(*element, stay_within);
else
Expand Down Expand Up @@ -1098,19 +1133,24 @@ unsigned GetRuleSetFlags(const HeapHashSet<Member<RuleSet>> rule_sets) {
void StyleEngine::ApplyRuleSetChanges(
TreeScope& tree_scope,
const ActiveStyleSheetVector& old_style_sheets,
const ActiveStyleSheetVector& new_style_sheets) {
const ActiveStyleSheetVector& new_style_sheets,
InvalidationScope invalidation_scope) {
DCHECK(IsMaster());
DCHECK(global_rule_set_);
HeapHashSet<Member<RuleSet>> changed_rule_sets;

ScopedStyleResolver* scoped_resolver = tree_scope.GetScopedStyleResolver();
bool append_all_sheets =
scoped_resolver && scoped_resolver->NeedsAppendAllSheets();

ActiveSheetsChange change = CompareActiveStyleSheets(
old_style_sheets, new_style_sheets, changed_rule_sets);
if (change == kNoActiveSheetsChanged && !append_all_sheets)
return;
bool append_all_sheets = false;

if (invalidation_scope == kInvalidateCurrentScope) {
if (ScopedStyleResolver* scoped_resolver =
tree_scope.GetScopedStyleResolver())
append_all_sheets = scoped_resolver->NeedsAppendAllSheets();

if (change == kNoActiveSheetsChanged && !append_all_sheets)
return;
}

// With rules added or removed, we need to re-aggregate rule meta data.
global_rule_set_->MarkDirty();
Expand All @@ -1124,23 +1164,25 @@ void StyleEngine::ApplyRuleSetChanges(
if (fonts_changed && change == kActiveSheetsChanged)
ClearFontCache();

// - If all sheets were removed, we remove the ScopedStyleResolver.
// - If new sheets were appended to existing ones, start appending after the
// common prefix.
// - For other diffs, reset author style and re-add all sheets for the
// TreeScope.
if (tree_scope.GetScopedStyleResolver()) {
if (new_style_sheets.IsEmpty())
ResetAuthorStyle(tree_scope);
else if (change == kActiveSheetsAppended && !append_all_sheets)
append_start_index = old_style_sheets.size();
else
tree_scope.GetScopedStyleResolver()->ResetAuthorStyle();
}
if (invalidation_scope == kInvalidateCurrentScope) {
// - If all sheets were removed, we remove the ScopedStyleResolver.
// - If new sheets were appended to existing ones, start appending after the
// common prefix.
// - For other diffs, reset author style and re-add all sheets for the
// TreeScope.
if (tree_scope.GetScopedStyleResolver()) {
if (new_style_sheets.IsEmpty())
ResetAuthorStyle(tree_scope);
else if (change == kActiveSheetsAppended && !append_all_sheets)
append_start_index = old_style_sheets.size();
else
tree_scope.GetScopedStyleResolver()->ResetAuthorStyle();
}

if (!new_style_sheets.IsEmpty()) {
tree_scope.EnsureScopedStyleResolver().AppendActiveStyleSheets(
append_start_index, new_style_sheets);
if (!new_style_sheets.IsEmpty()) {
tree_scope.EnsureScopedStyleResolver().AppendActiveStyleSheets(
append_start_index, new_style_sheets);
}
}

if (tree_scope.GetDocument().HasPendingForcedStyleRecalc())
Expand Down Expand Up @@ -1172,7 +1214,8 @@ void StyleEngine::ApplyRuleSetChanges(
return;
}

ScheduleInvalidationsForRuleSets(tree_scope, changed_rule_sets);
ScheduleInvalidationsForRuleSets(tree_scope, changed_rule_sets,
invalidation_scope);
}

const MediaQueryEvaluator& StyleEngine::EnsureMediaQueryEvaluator() {
Expand Down Expand Up @@ -1275,9 +1318,20 @@ void StyleEngine::NodeWillBeRemoved(Node& node) {
}
}

void StyleEngine::CollectMatchingUserRules(
ElementRuleCollector& collector) const {
for (unsigned i = 0; i < active_user_style_sheets_.size(); ++i) {
DCHECK(active_user_style_sheets_[i].second);
collector.CollectMatchingRules(
MatchRequest(active_user_style_sheets_[i].second, nullptr,
active_user_style_sheets_[i].first, i));
}
}

DEFINE_TRACE(StyleEngine) {
visitor->Trace(document_);
visitor->Trace(injected_author_style_sheets_);
visitor->Trace(user_style_sheets_);
visitor->Trace(active_user_style_sheets_);
visitor->Trace(inspector_style_sheet_);
visitor->Trace(document_style_sheet_collection_);
visitor->Trace(style_sheet_collection_map_);
Expand All @@ -1298,7 +1352,7 @@ DEFINE_TRACE(StyleEngine) {
}

DEFINE_TRACE_WRAPPERS(StyleEngine) {
for (const auto& sheet : injected_author_style_sheets_) {
for (const auto& sheet : user_style_sheets_) {
visitor->TraceWrappers(sheet.second);
}
visitor->TraceWrappers(document_style_sheet_collection_);
Expand Down
Loading

0 comments on commit 30168b9

Please sign in to comment.