Commit 66efe30f authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Chromium LUCI CQ

Move media controls UA style into a separate ruleset

The current rules for media controls should only apply to elements in
the UA shadow dom for media elements, but were added to the ruleset for
all html UA style. Since the media controls styling adds a bunch of
rules in the universal tag bucket, once we add a media element to our
renderer, we were stuck with having to reject a bunch of these rules for
every html element.

Instead, create a different set of styles for media controls and only
try applying rules from that set for elements inside the UA shadow tree
for media elements.

Bug: 1166198, 1032957
Change-Id: I5b2fa635558a28a29bf69538f94763d158c7d2ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2631490
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844975}
parent 45e98de8
...@@ -161,6 +161,7 @@ void CSSDefaultStyleSheets::InitializeDefaultStyles() { ...@@ -161,6 +161,7 @@ void CSSDefaultStyleSheets::InitializeDefaultStyles() {
default_quirks_style_ = MakeGarbageCollected<RuleSet>(); default_quirks_style_ = MakeGarbageCollected<RuleSet>();
default_print_style_ = MakeGarbageCollected<RuleSet>(); default_print_style_ = MakeGarbageCollected<RuleSet>();
default_forced_color_style_ = MakeGarbageCollected<RuleSet>(); default_forced_color_style_ = MakeGarbageCollected<RuleSet>();
default_media_controls_style_ = MakeGarbageCollected<RuleSet>();
default_pseudo_element_style_.Clear(); default_pseudo_element_style_.Clear();
default_style_->AddRulesFromSheet(DefaultStyleSheet(), ScreenEval()); default_style_->AddRulesFromSheet(DefaultStyleSheet(), ScreenEval());
...@@ -248,7 +249,8 @@ bool CSSDefaultStyleSheets::EnsureDefaultStyleSheetsForElement( ...@@ -248,7 +249,8 @@ bool CSSDefaultStyleSheets::EnsureDefaultStyleSheetsForElement(
// and <audio>. // and <audio>.
media_controls_style_sheet_ = media_controls_style_sheet_ =
ParseUASheet(media_controls_style_sheet_loader_->GetUAStyleSheet()); ParseUASheet(media_controls_style_sheet_loader_->GetUAStyleSheet());
default_style_->AddRulesFromSheet(MediaControlsStyleSheet(), ScreenEval()); default_media_controls_style_->AddRulesFromSheet(MediaControlsStyleSheet(),
ScreenEval());
default_print_style_->AddRulesFromSheet(MediaControlsStyleSheet(), default_print_style_->AddRulesFromSheet(MediaControlsStyleSheet(),
PrintEval()); PrintEval());
default_forced_color_style_->AddRulesFromSheet(MediaControlsStyleSheet(), default_forced_color_style_->AddRulesFromSheet(MediaControlsStyleSheet(),
...@@ -284,7 +286,8 @@ bool CSSDefaultStyleSheets::EnsureDefaultStyleSheetsForElement( ...@@ -284,7 +286,8 @@ bool CSSDefaultStyleSheets::EnsureDefaultStyleSheetsForElement(
settings->GetTextTrackTextSize()); settings->GetTextTrackTextSize());
builder.Append(" } "); builder.Append(" } ");
text_track_style_sheet_ = ParseUASheet(builder.ToString()); text_track_style_sheet_ = ParseUASheet(builder.ToString());
default_style_->AddRulesFromSheet(text_track_style_sheet_, ScreenEval()); default_media_controls_style_->AddRulesFromSheet(text_track_style_sheet_,
ScreenEval());
default_print_style_->AddRulesFromSheet(text_track_style_sheet_, default_print_style_->AddRulesFromSheet(text_track_style_sheet_,
PrintEval()); PrintEval());
changed_default_style = true; changed_default_style = true;
...@@ -346,6 +349,16 @@ void CSSDefaultStyleSheets::EnsureDefaultStyleSheetForFullscreen() { ...@@ -346,6 +349,16 @@ void CSSDefaultStyleSheets::EnsureDefaultStyleSheetForFullscreen() {
ScreenEval()); ScreenEval());
} }
void CSSDefaultStyleSheets::CollectFeaturesTo(const Document& document,
RuleFeatureSet& features) {
if (DefaultStyle())
features.Add(DefaultStyle()->Features());
if (DefaultMediaControlsStyle())
features.Add(DefaultMediaControlsStyle()->Features());
if (document.IsViewSource() && DefaultViewSourceStyle())
features.Add(DefaultViewSourceStyle()->Features());
}
void CSSDefaultStyleSheets::Trace(Visitor* visitor) const { void CSSDefaultStyleSheets::Trace(Visitor* visitor) const {
visitor->Trace(default_style_); visitor->Trace(default_style_);
visitor->Trace(default_mathml_style_); visitor->Trace(default_mathml_style_);
...@@ -354,6 +367,7 @@ void CSSDefaultStyleSheets::Trace(Visitor* visitor) const { ...@@ -354,6 +367,7 @@ void CSSDefaultStyleSheets::Trace(Visitor* visitor) const {
visitor->Trace(default_print_style_); visitor->Trace(default_print_style_);
visitor->Trace(default_view_source_style_); visitor->Trace(default_view_source_style_);
visitor->Trace(default_forced_color_style_); visitor->Trace(default_forced_color_style_);
visitor->Trace(default_media_controls_style_);
visitor->Trace(default_style_sheet_); visitor->Trace(default_style_sheet_);
visitor->Trace(default_pseudo_element_style_); visitor->Trace(default_pseudo_element_style_);
visitor->Trace(mobile_viewport_style_sheet_); visitor->Trace(mobile_viewport_style_sheet_);
......
...@@ -32,7 +32,9 @@ ...@@ -32,7 +32,9 @@
namespace blink { namespace blink {
class Document;
class Element; class Element;
class RuleFeatureSet;
class RuleSet; class RuleSet;
class StyleSheetContents; class StyleSheetContents;
...@@ -62,6 +64,9 @@ class CSSDefaultStyleSheets final ...@@ -62,6 +64,9 @@ class CSSDefaultStyleSheets final
RuleSet* DefaultPseudoElementStyleOrNull() { RuleSet* DefaultPseudoElementStyleOrNull() {
return default_pseudo_element_style_.Get(); return default_pseudo_element_style_.Get();
} }
RuleSet* DefaultMediaControlsStyle() {
return default_media_controls_style_.Get();
}
StyleSheetContents* EnsureMobileViewportStyleSheet(); StyleSheetContents* EnsureMobileViewportStyleSheet();
StyleSheetContents* EnsureTelevisionViewportStyleSheet(); StyleSheetContents* EnsureTelevisionViewportStyleSheet();
...@@ -97,6 +102,8 @@ class CSSDefaultStyleSheets final ...@@ -97,6 +102,8 @@ class CSSDefaultStyleSheets final
return media_controls_style_sheet_loader_.get(); return media_controls_style_sheet_loader_.get();
} }
void CollectFeaturesTo(const Document&, RuleFeatureSet&);
void Trace(Visitor*) const; void Trace(Visitor*) const;
private: private:
...@@ -110,6 +117,7 @@ class CSSDefaultStyleSheets final ...@@ -110,6 +117,7 @@ class CSSDefaultStyleSheets final
Member<RuleSet> default_view_source_style_; Member<RuleSet> default_view_source_style_;
Member<RuleSet> default_forced_color_style_; Member<RuleSet> default_forced_color_style_;
Member<RuleSet> default_pseudo_element_style_; Member<RuleSet> default_pseudo_element_style_;
Member<RuleSet> default_media_controls_style_;
Member<StyleSheetContents> default_style_sheet_; Member<StyleSheetContents> default_style_sheet_;
Member<StyleSheetContents> mobile_viewport_style_sheet_; Member<StyleSheetContents> mobile_viewport_style_sheet_;
......
...@@ -35,17 +35,13 @@ void CSSGlobalRuleSet::Update(Document& document) { ...@@ -35,17 +35,13 @@ void CSSGlobalRuleSet::Update(Document& document) {
is_dirty_ = false; is_dirty_ = false;
features_.Clear(); features_.Clear();
has_fullscreen_ua_style_ = false;
CSSDefaultStyleSheets& default_style_sheets = CSSDefaultStyleSheets& default_style_sheets =
CSSDefaultStyleSheets::Instance(); CSSDefaultStyleSheets::Instance();
if (default_style_sheets.DefaultStyle()) {
features_.Add(default_style_sheets.DefaultStyle()->Features());
has_fullscreen_ua_style_ = default_style_sheets.FullscreenStyleSheet();
}
if (document.IsViewSource()) has_fullscreen_ua_style_ = default_style_sheets.FullscreenStyleSheet();
features_.Add(default_style_sheets.DefaultViewSourceStyle()->Features());
default_style_sheets.CollectFeaturesTo(document, features_);
if (watched_selectors_rule_set_) if (watched_selectors_rule_set_)
features_.Add(watched_selectors_rule_set_->Features()); features_.Add(watched_selectors_rule_set_->Features());
......
...@@ -522,6 +522,22 @@ void StyleResolver::MatchUserRules(ElementRuleCollector& collector) { ...@@ -522,6 +522,22 @@ void StyleResolver::MatchUserRules(ElementRuleCollector& collector) {
collector.FinishAddingUserRules(); collector.FinishAddingUserRules();
} }
namespace {
bool IsInMediaUAShadow(const Element& element) {
ShadowRoot* root = element.ContainingShadowRoot();
if (!root || !root->IsUserAgent())
return false;
ShadowRoot* outer_root;
do {
outer_root = root;
root = root->host().ContainingShadowRoot();
} while (root && root->IsUserAgent());
return outer_root->host().IsMediaElement();
}
} // namespace
void StyleResolver::MatchUARules(const Element& element, void StyleResolver::MatchUARules(const Element& element,
ElementRuleCollector& collector) { ElementRuleCollector& collector) {
collector.SetMatchingUARules(true); collector.SetMatchingUARules(true);
...@@ -529,12 +545,17 @@ void StyleResolver::MatchUARules(const Element& element, ...@@ -529,12 +545,17 @@ void StyleResolver::MatchUARules(const Element& element,
CSSDefaultStyleSheets& default_style_sheets = CSSDefaultStyleSheets& default_style_sheets =
CSSDefaultStyleSheets::Instance(); CSSDefaultStyleSheets::Instance();
if (!print_media_type_) { if (!print_media_type_) {
if (LIKELY(element.IsHTMLElement() || element.IsVTTElement())) if (LIKELY(element.IsHTMLElement() || element.IsVTTElement())) {
MatchRuleSet(collector, default_style_sheets.DefaultStyle()); MatchRuleSet(collector, default_style_sheets.DefaultStyle());
else if (element.IsSVGElement()) if (UNLIKELY(IsInMediaUAShadow(element))) {
MatchRuleSet(collector,
default_style_sheets.DefaultMediaControlsStyle());
}
} else if (element.IsSVGElement()) {
MatchRuleSet(collector, default_style_sheets.DefaultSVGStyle()); MatchRuleSet(collector, default_style_sheets.DefaultSVGStyle());
else if (element.namespaceURI() == mathml_names::kNamespaceURI) } else if (element.namespaceURI() == mathml_names::kNamespaceURI) {
MatchRuleSet(collector, default_style_sheets.DefaultMathMLStyle()); MatchRuleSet(collector, default_style_sheets.DefaultMathMLStyle());
}
} else { } else {
MatchRuleSet(collector, default_style_sheets.DefaultPrintStyle()); MatchRuleSet(collector, default_style_sheets.DefaultPrintStyle());
} }
......
...@@ -3934,6 +3934,35 @@ TEST_F(StyleEngineTest, MarkStyleDirtyFromContainerRecalc) { ...@@ -3934,6 +3934,35 @@ TEST_F(StyleEngineTest, MarkStyleDirtyFromContainerRecalc) {
EXPECT_NE(old_inner_style, new_inner_style); EXPECT_NE(old_inner_style, new_inner_style);
} }
TEST_F(StyleEngineTest, VideoControlsReject) {
GetDocument().body()->setInnerHTML(R"HTML(
<video controls></video>
<div id="target"></div>
)HTML");
UpdateAllLifecyclePhases();
StyleEngine& engine = GetStyleEngine();
// If the Stats() were already enabled, we would not start with 0 counts.
EXPECT_FALSE(engine.Stats());
engine.SetStatsEnabled(true);
StyleResolverStats* stats = engine.Stats();
ASSERT_TRUE(stats);
EXPECT_EQ(0u, stats->rules_fast_rejected);
EXPECT_EQ(0u, stats->rules_rejected);
Element* target = GetDocument().getElementById("target");
ASSERT_TRUE(target);
target->SetInlineStyleProperty(CSSPropertyID::kColor, "green");
GetDocument().Lifecycle().AdvanceTo(DocumentLifecycle::kInStyleRecalc);
GetStyleEngine().RecalcStyle();
// There should be no UA rules for a div to reject
EXPECT_EQ(0u, stats->rules_fast_rejected);
EXPECT_EQ(0u, stats->rules_rejected);
}
TEST_F(StyleEngineTest, FastRejectForHostChild) { TEST_F(StyleEngineTest, FastRejectForHostChild) {
GetDocument().body()->setInnerHTML(R"HTML( GetDocument().body()->setInnerHTML(R"HTML(
<style> <style>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment