Title: [141844] trunk/Source/WebCore
- Revision
- 141844
- Author
- [email protected]
- Date
- 2013-02-04 18:57:26 -0800 (Mon, 04 Feb 2013)
Log Message
Default element styles are not always collected for sharing detection
https://bugs.webkit.org/show_bug.cgi?id=108404
Reviewed by Antti Koivisto.
The method ensureDefaultStyleSheetsForElement is run as we add elements
to the document. This may update the defaultStyle of the document, but
does not recollect any changes into the StyleResolver. This means that
style sharing might be overly ambitious, thinking it can share a style
for an element which was matched in the new rules. This showed up most
often in the Shadow Root for media elements, which would add a set of
style rules, but the shadow children would sometimes share styles even
when they shouldn't.
The fix is to detect if we need to collect after adding a
style for an element. This might cause a little more work, but
in my testing it doesn't happen very often.
Unfortunately it is hard to get a reproducible test for this.
* css/CSSDefaultStyleSheets.cpp:
(WebCore::CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement): As we load in
styles, keep track of whether or not we change the default style.
* css/CSSDefaultStyleSheets.h:
(CSSDefaultStyleSheets): New boolean parameter indicating if the style has changed.
* css/StyleResolver.cpp:
(WebCore::StyleResolver::styleForElement): Collect features if the element
caused the default style to update.
(WebCore::StyleResolver::collectFeatures): Protect for null in updates.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (141843 => 141844)
--- trunk/Source/WebCore/ChangeLog 2013-02-05 02:25:46 UTC (rev 141843)
+++ trunk/Source/WebCore/ChangeLog 2013-02-05 02:57:26 UTC (rev 141844)
@@ -1,3 +1,35 @@
+2013-02-04 Dean Jackson <[email protected]>
+
+ Default element styles are not always collected for sharing detection
+ https://bugs.webkit.org/show_bug.cgi?id=108404
+
+ Reviewed by Antti Koivisto.
+
+ The method ensureDefaultStyleSheetsForElement is run as we add elements
+ to the document. This may update the defaultStyle of the document, but
+ does not recollect any changes into the StyleResolver. This means that
+ style sharing might be overly ambitious, thinking it can share a style
+ for an element which was matched in the new rules. This showed up most
+ often in the Shadow Root for media elements, which would add a set of
+ style rules, but the shadow children would sometimes share styles even
+ when they shouldn't.
+
+ The fix is to detect if we need to collect after adding a
+ style for an element. This might cause a little more work, but
+ in my testing it doesn't happen very often.
+
+ Unfortunately it is hard to get a reproducible test for this.
+
+ * css/CSSDefaultStyleSheets.cpp:
+ (WebCore::CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement): As we load in
+ styles, keep track of whether or not we change the default style.
+ * css/CSSDefaultStyleSheets.h:
+ (CSSDefaultStyleSheets): New boolean parameter indicating if the style has changed.
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::styleForElement): Collect features if the element
+ caused the default style to update.
+ (WebCore::StyleResolver::collectFeatures): Protect for null in updates.
+
2013-02-04 Jun Jiang <[email protected]>
Fix the issue that some possible source formats are ignored for float textures in texture packing for CG port
Modified: trunk/Source/WebCore/css/CSSDefaultStyleSheets.cpp (141843 => 141844)
--- trunk/Source/WebCore/css/CSSDefaultStyleSheets.cpp 2013-02-05 02:25:46 UTC (rev 141843)
+++ trunk/Source/WebCore/css/CSSDefaultStyleSheets.cpp 2013-02-05 02:57:26 UTC (rev 141844)
@@ -153,10 +153,12 @@
}
-void CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement(Element* element)
+void CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement(Element* element, bool& changedDefaultStyle)
{
- if (simpleDefaultStyleSheet && !elementCanUseSimpleDefaultStyle(element))
+ if (simpleDefaultStyleSheet && !elementCanUseSimpleDefaultStyle(element)) {
loadFullDefaultStyle();
+ changedDefaultStyle = true;
+ }
#if ENABLE(SVG)
if (element->isSVGElement() && !svgStyleSheet) {
@@ -164,6 +166,7 @@
svgStyleSheet = parseUASheet(svgUserAgentStyleSheet, sizeof(svgUserAgentStyleSheet));
defaultStyle->addRulesFromSheet(svgStyleSheet, screenEval());
defaultPrintStyle->addRulesFromSheet(svgStyleSheet, printEval());
+ changedDefaultStyle = true;
}
#endif
@@ -173,6 +176,7 @@
mathMLStyleSheet = parseUASheet(mathmlUserAgentStyleSheet, sizeof(mathmlUserAgentStyleSheet));
defaultStyle->addRulesFromSheet(mathMLStyleSheet, screenEval());
defaultPrintStyle->addRulesFromSheet(mathMLStyleSheet, printEval());
+ changedDefaultStyle = true;
}
#endif
@@ -182,6 +186,7 @@
mediaControlsStyleSheet = parseUASheet(mediaRules);
defaultStyle->addRulesFromSheet(mediaControlsStyleSheet, screenEval());
defaultPrintStyle->addRulesFromSheet(mediaControlsStyleSheet, printEval());
+ changedDefaultStyle = true;
}
#endif
@@ -191,6 +196,7 @@
fullscreenStyleSheet = parseUASheet(fullscreenRules);
defaultStyle->addRulesFromSheet(fullscreenStyleSheet, screenEval());
defaultQuirksStyle->addRulesFromSheet(fullscreenStyleSheet, screenEval());
+ changedDefaultStyle = true;
}
#endif
@@ -198,6 +204,7 @@
String plugInsRules = String(plugInsUserAgentStyleSheet, sizeof(plugInsUserAgentStyleSheet)) + RenderTheme::themeForPage(element->document()->page())->extraPlugInsStyleSheet() + element->document()->page()->chrome()->client()->plugInExtraStyleSheet();
plugInsStyleSheet = parseUASheet(plugInsRules);
defaultStyle->addRulesFromSheet(plugInsStyleSheet, screenEval());
+ changedDefaultStyle = true;
}
ASSERT(defaultStyle->features().idsInRules.isEmpty());
Modified: trunk/Source/WebCore/css/CSSDefaultStyleSheets.h (141843 => 141844)
--- trunk/Source/WebCore/css/CSSDefaultStyleSheets.h 2013-02-05 02:25:46 UTC (rev 141843)
+++ trunk/Source/WebCore/css/CSSDefaultStyleSheets.h 2013-02-05 02:57:26 UTC (rev 141844)
@@ -45,7 +45,7 @@
static StyleSheetContents* fullscreenStyleSheet;
static StyleSheetContents* plugInsStyleSheet;
- static void ensureDefaultStyleSheetsForElement(Element*);
+ static void ensureDefaultStyleSheetsForElement(Element*, bool& changedDefaultStyle);
static void loadFullDefaultStyle();
static void loadSimpleDefaultStyle();
static void initDefaultStyle(Element*);
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (141843 => 141844)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2013-02-05 02:25:46 UTC (rev 141843)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2013-02-05 02:57:26 UTC (rev 141844)
@@ -1433,7 +1433,10 @@
state.style->setInsideLink(linkState);
}
- CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement(element);
+ bool needsCollection = false;
+ CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement(element, needsCollection);
+ if (needsCollection)
+ collectFeatures();
MatchResult matchResult;
if (matchingBehavior == MatchOnlyUserAgentRules)
@@ -5169,8 +5172,10 @@
// Collect all ids and rules using sibling selectors (:first-child and similar)
// in the current set of stylesheets. Style sharing code uses this information to reject
// sharing candidates.
- m_features.add(CSSDefaultStyleSheets::defaultStyle->features());
- m_features.add(m_authorStyle->features());
+ if (CSSDefaultStyleSheets::defaultStyle)
+ m_features.add(CSSDefaultStyleSheets::defaultStyle->features());
+ if (m_authorStyle)
+ m_features.add(m_authorStyle->features());
if (document()->isViewSource())
m_features.add(CSSDefaultStyleSheets::viewSourceStyle()->features());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
