- Revision
- 206641
- Author
- [email protected]
- Date
- 2016-09-30 10:00:22 -0700 (Fri, 30 Sep 2016)
Log Message
Remove "rem" unit optimization for document element font size changes
https://bugs.webkit.org/show_bug.cgi?id=162778
Reviewed by Alex Christensen.
We awkwardly track from the parser level if any stylesheet in a document uses any rem units. This is only used to minimally
optimize a case where document element's (<html>) font size changes dynamically.
In practice such changes are rare. Browsing around I couldn't find a single case where this optimization got used.
Even if it was used it would be of low value as a full style resolution is likely to happen anyway (as font inherits)
and the only thing really saved is that we don't need to invalidate the matched properties cache.
* css/CSSGrammar.y.in:
* css/StyleSheetContents.cpp:
(WebCore::StyleSheetContents::StyleSheetContents):
* css/StyleSheetContents.h:
* dom/AuthorStyleSheets.cpp:
(WebCore::AuthorStyleSheets::updateActiveStyleSheets):
* dom/AuthorStyleSheets.h:
(WebCore::AuthorStyleSheets::usesRemUnits): Deleted.
(WebCore::AuthorStyleSheets::setUsesRemUnit): Deleted.
* dom/Document.cpp:
(WebCore::Document::recalcStyle):
(WebCore::Document::updateBaseURL):
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveElement):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (206640 => 206641)
--- trunk/Source/WebCore/ChangeLog 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/ChangeLog 2016-09-30 17:00:22 UTC (rev 206641)
@@ -1,3 +1,32 @@
+2016-09-30 Antti Koivisto <[email protected]>
+
+ Remove "rem" unit optimization for document element font size changes
+ https://bugs.webkit.org/show_bug.cgi?id=162778
+
+ Reviewed by Alex Christensen.
+
+ We awkwardly track from the parser level if any stylesheet in a document uses any rem units. This is only used to minimally
+ optimize a case where document element's (<html>) font size changes dynamically.
+
+ In practice such changes are rare. Browsing around I couldn't find a single case where this optimization got used.
+ Even if it was used it would be of low value as a full style resolution is likely to happen anyway (as font inherits)
+ and the only thing really saved is that we don't need to invalidate the matched properties cache.
+
+ * css/CSSGrammar.y.in:
+ * css/StyleSheetContents.cpp:
+ (WebCore::StyleSheetContents::StyleSheetContents):
+ * css/StyleSheetContents.h:
+ * dom/AuthorStyleSheets.cpp:
+ (WebCore::AuthorStyleSheets::updateActiveStyleSheets):
+ * dom/AuthorStyleSheets.h:
+ (WebCore::AuthorStyleSheets::usesRemUnits): Deleted.
+ (WebCore::AuthorStyleSheets::setUsesRemUnit): Deleted.
+ * dom/Document.cpp:
+ (WebCore::Document::recalcStyle):
+ (WebCore::Document::updateBaseURL):
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::resolveElement):
+
2016-09-30 Zalan Bujtas <[email protected]>
RenderLayer::clipRects may return nullptr.
Modified: trunk/Source/WebCore/css/CSSGrammar.y.in (206640 => 206641)
--- trunk/Source/WebCore/css/CSSGrammar.y.in 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/css/CSSGrammar.y.in 2016-09-30 17:00:22 UTC (rev 206641)
@@ -1728,8 +1728,6 @@
$$.id = CSSValueInvalid;
$$.fValue = $1;
$$.unit = CSSPrimitiveValue::CSS_REMS;
- if (parser->m_styleSheet)
- parser->m_styleSheet->parserSetUsesRemUnits();
}
| CHS { $$.id = CSSValueInvalid; $$.fValue = $1; $$.unit = CSSPrimitiveValue::CSS_CHS; }
| VW { $$.id = CSSValueInvalid; $$.fValue = $1; $$.unit = CSSPrimitiveValue::CSS_VW; }
Modified: trunk/Source/WebCore/css/StyleSheetContents.cpp (206640 => 206641)
--- trunk/Source/WebCore/css/StyleSheetContents.cpp 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/css/StyleSheetContents.cpp 2016-09-30 17:00:22 UTC (rev 206641)
@@ -67,7 +67,6 @@
, m_isUserStyleSheet(ownerRule && ownerRule->parentStyleSheet() && ownerRule->parentStyleSheet()->isUserStyleSheet())
, m_hasSyntacticallyValidCSSHeader(true)
, m_didLoadErrorOccur(false)
- , m_usesRemUnits(false)
, m_usesStyleBasedEditability(false)
, m_isMutable(false)
, m_isInMemoryCache(false)
@@ -88,7 +87,6 @@
, m_isUserStyleSheet(o.m_isUserStyleSheet)
, m_hasSyntacticallyValidCSSHeader(o.m_hasSyntacticallyValidCSSHeader)
, m_didLoadErrorOccur(false)
- , m_usesRemUnits(o.m_usesRemUnits)
, m_usesStyleBasedEditability(o.m_usesStyleBasedEditability)
, m_isMutable(false)
, m_isInMemoryCache(false)
Modified: trunk/Source/WebCore/css/StyleSheetContents.h (206640 => 206641)
--- trunk/Source/WebCore/css/StyleSheetContents.h 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/css/StyleSheetContents.h 2016-09-30 17:00:22 UTC (rev 206641)
@@ -96,7 +96,6 @@
void parserAddNamespace(const AtomicString& prefix, const AtomicString& uri);
void parserAppendRule(Ref<StyleRuleBase>&&);
void parserSetEncodingFromCharsetRule(const String& encoding);
- void parserSetUsesRemUnits() { m_usesRemUnits = true; }
void parserSetUsesStyleBasedEditability() { m_usesStyleBasedEditability = true; }
void clearRules();
@@ -122,7 +121,6 @@
unsigned ruleCount() const;
StyleRuleBase* ruleAt(unsigned index) const;
- bool usesRemUnits() const { return m_usesRemUnits; }
bool usesStyleBasedEditability() const { return m_usesStyleBasedEditability; }
unsigned estimatedSizeInBytes() const;
@@ -166,7 +164,6 @@
bool m_isUserStyleSheet : 1;
bool m_hasSyntacticallyValidCSSHeader : 1;
bool m_didLoadErrorOccur : 1;
- bool m_usesRemUnits : 1;
bool m_usesStyleBasedEditability : 1;
bool m_isMutable : 1;
bool m_isInMemoryCache : 1;
Modified: trunk/Source/WebCore/dom/AuthorStyleSheets.cpp (206640 => 206641)
--- trunk/Source/WebCore/dom/AuthorStyleSheets.cpp 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/dom/AuthorStyleSheets.cpp 2016-09-30 17:00:22 UTC (rev 206641)
@@ -330,8 +330,6 @@
InspectorInstrumentation::activeStyleSheetsUpdated(m_document);
for (const auto& sheet : m_activeStyleSheets) {
- if (sheet->contents().usesRemUnits())
- m_usesRemUnits = true;
if (sheet->contents().usesStyleBasedEditability())
m_usesStyleBasedEditability = true;
}
Modified: trunk/Source/WebCore/dom/AuthorStyleSheets.h (206640 => 206641)
--- trunk/Source/WebCore/dom/AuthorStyleSheets.h 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/dom/AuthorStyleSheets.h 2016-09-30 17:00:22 UTC (rev 206641)
@@ -76,8 +76,6 @@
bool hasPendingSheets() const { return m_pendingStyleSheetCount > 0; }
- bool usesRemUnits() const { return m_usesRemUnits; }
- void setUsesRemUnit(bool b) { m_usesRemUnits = b; }
bool usesStyleBasedEditability() { return m_usesStyleBasedEditability; }
bool activeStyleSheetsContains(const CSSStyleSheet*) const;
@@ -131,7 +129,6 @@
String m_preferredStylesheetSetName;
String m_selectedStylesheetSetName;
- bool m_usesRemUnits { false };
bool m_usesStyleBasedEditability { false };
};
Modified: trunk/Source/WebCore/dom/Document.cpp (206640 => 206641)
--- trunk/Source/WebCore/dom/Document.cpp 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/dom/Document.cpp 2016-09-30 17:00:22 UTC (rev 206641)
@@ -1866,12 +1866,6 @@
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willRecalculateStyle(*this);
- // FIXME: We never reset this flags.
- if (m_elementSheet && m_elementSheet->contents().usesRemUnits())
- authorStyleSheets().setUsesRemUnit(true);
- // We don't call setUsesStyleBasedEditability here because the whole point of the flag is to avoid style recalc.
- // i.e. updating the flag here would be too late.
-
m_inStyleRecalc = true;
bool updatedCompositingLayers = false;
{
@@ -2972,12 +2966,9 @@
if (m_elementSheet) {
// Element sheet is silly. It never contains anything.
ASSERT(!m_elementSheet->contents().ruleCount());
- bool usesRemUnits = m_elementSheet->contents().usesRemUnits();
bool usesStyleBasedEditability = m_elementSheet->contents().usesStyleBasedEditability();
m_elementSheet = CSSStyleSheet::createInline(*this, m_baseURL);
// FIXME: So we are not really the parser. The right fix is to eliminate the element sheet completely.
- if (usesRemUnits)
- m_elementSheet->contents().parserSetUsesRemUnits();
if (usesStyleBasedEditability)
m_elementSheet->contents().parserSetUsesStyleBasedEditability();
}
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (206640 => 206641)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-09-30 16:59:24 UTC (rev 206640)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-09-30 17:00:22 UTC (rev 206641)
@@ -214,10 +214,9 @@
m_documentElementStyle = RenderStyle::clonePtr(*update.style);
scope().styleResolver.setOverrideDocumentElementStyle(m_documentElementStyle.get());
- // If "rem" units are used anywhere in the document, and if the document element's font size changes, then force font updating
- // all the way down the tree. This is simpler than having to maintain a cache of objects (and such font size changes should be rare anyway).
- if (m_document.authorStyleSheets().usesRemUnits() && update.change != NoChange && existingStyle && existingStyle->fontSize() != update.style->fontSize()) {
- // Cached RenderStyles may depend on the rem units.
+ if (update.change != NoChange && existingStyle && existingStyle->fontSize() != update.style->fontSize()) {
+ // "rem" units are relative to the document element's font size so we need to recompute everything.
+ // In practice this is rare.
scope().styleResolver.invalidateMatchedPropertiesCache();
update.change = Force;
}
