isContentEditable shouldn't trigger synchronous style recalc in most cases
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 1 Mar 2015 19:52:21 +0000 (19:52 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 1 Mar 2015 19:52:21 +0000 (19:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129034

Reviewed by Antti Koivisto.

Source/WebCore:

Avoid style recalc inside isContentEditable when the document doesn't contain -webkit-user-modify or
-webkit-user-select: all. Instead, compute the value from contenteditable attributes in ancestors.
However, still compute the editability from the style tree when it's up-to-date in order to avoid
repeatedly walking up the DOM tree in a hot code path inside editing.

Test: fast/dom/HTMLElement/dynamic-editability-change.html

* css/CSSGrammar.y.in: No need to pass in "true" as we never call this function with false.
* css/CSSParser.cpp:
(WebCore::isValidKeywordPropertyAndValue): Calls parserSetUsesStyleBasedEditability as needed.
(WebCore::parseKeywordValue): Passes around StyleSheetContents*.
(WebCore::CSSParser::parseValue): Ditto.
(WebCore::CSSParser::parseFont): Ditto.

* css/StyleSheetContents.cpp:
(WebCore::StyleSheetContents::StyleSheetContents): Initializes and copies m_usesStyleBasedEditability.

* css/StyleSheetContents.h:
(WebCore::StyleSheetContents::parserSetUsesRemUnits): Removed the argument since it was always true.
(WebCore::StyleSheetContents::parserSetUsesStyleBasedEditability): Added.
(WebCore::StyleSheetContents::usesStyleBasedEditability): Added.

* dom/Document.cpp:
(WebCore::Document::recalcStyle): Added a FIXME as well as a comment explaining why we don't call
setUsesStyleBasedEditability. Since Node::computeEditability triggers style recalc only when the flag
is set to true, it's too late to update the flag here.
(WebCore::Document::updateStyleIfNeeded): Uses a newly extracted needsStyleRecalc.
(WebCore::Document::updateBaseURL): Preserves m_usesStyleBasedEditability as well as m_usesRemUnit.
(WebCore::Document::usesStyleBasedEditability): Added. Returns true when inline style declarations or
any active stylesheet uses -webkit-user-modify or -webkit-user-select: all. Flushing pending stylesheet
changes here is fine because the alternative is to trigger a full blown style recalc.

* dom/Document.h:
(WebCore::Document::needsStyleRecalc): Added. Extracted from updateStyleIfNeeded.

* dom/DocumentStyleSheetCollection.cpp:
(WebCore::DocumentStyleSheetCollection::DocumentStyleSheetCollection):
(WebCore::styleSheetsUseRemUnits): Deleted.
(WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets): Updates m_usesStyleBasedEditability
as well as m_usesRemUnit.

* dom/DocumentStyleSheetCollection.h:
(WebCore::DocumentStyleSheetCollection::usesStyleBasedEditability): Added.
(WebCore::DocumentStyleSheetCollection::setUsesStyleBasedEditability): Added.

* dom/Node.cpp:
(WebCore::computeEditabilityFromComputedStyle): Extracted from computeEditability.
(WebCore::Node::computeEditability): When the style recalc is requested and the render tree is dirty,
check if the document uses any CSS property that can affect the editability of elements. If it doesn't,
compute the editability from contenteditable attributes in the anchors via matchesReadWritePseudoClass.
Continue to use the style-based computation when the render tree isn't dirty to avoid the tree walk.

* html/HTMLElement.cpp:
(WebCore::HTMLElement::editabilityFromContentEditableAttr): Extracted from matchesReadWritePseudoClass
to be called in Node::computeEditability. Also made it return Editability instead of boolean.
(WebCore::HTMLElement::matchesReadWritePseudoClass):
* html/HTMLElement.h:

LayoutTests:

Added a regression test to update the editability of elements dynamically. Also rebaselined
tests per style recalc timing changes.

* fast/dom/HTMLElement/dynamic-editability-change-expected.txt: Added.
* fast/dom/HTMLElement/dynamic-editability-change.html: Added.
* platform/mac/editing/execCommand/5142012-1-expected.txt: anonymous render block differences.
* platform/mac/editing/execCommand/nsresponder-outdent-expected.txt: Ditto.
* platform/mac/editing/inserting/insert-at-end-02-expected.txt: Empty render text differences.
* platform/mac/editing/pasteboard/4989774-expected.txt: Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@180867 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLElement/dynamic-editability-change-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLElement/dynamic-editability-change.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/execCommand/5142012-1-expected.txt
LayoutTests/platform/mac/editing/execCommand/nsresponder-outdent-expected.txt
LayoutTests/platform/mac/editing/inserting/insert-at-end-02-expected.txt
LayoutTests/platform/mac/editing/pasteboard/4989774-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/CSSGrammar.y.in
Source/WebCore/css/CSSParser.cpp
Source/WebCore/css/StyleSheetContents.cpp
Source/WebCore/css/StyleSheetContents.h
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/DocumentStyleSheetCollection.cpp
Source/WebCore/dom/DocumentStyleSheetCollection.h
Source/WebCore/dom/Node.cpp
Source/WebCore/html/HTMLElement.cpp
Source/WebCore/html/HTMLElement.h

index 0093555aa5eefb56c92d36cfc8354e89de6d337b..f69949fb33720ffb7ff771b2a82d7ec970075837 100644 (file)
@@ -1,3 +1,20 @@
+2015-03-01  Ryosuke Niwa  <rniwa@webkit.org>
+
+        isContentEditable shouldn't trigger synchronous style recalc in most cases
+        https://bugs.webkit.org/show_bug.cgi?id=129034
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test to update the editability of elements dynamically. Also rebaselined
+        tests per style recalc timing changes.
+
+        * fast/dom/HTMLElement/dynamic-editability-change-expected.txt: Added.
+        * fast/dom/HTMLElement/dynamic-editability-change.html: Added.
+        * platform/mac/editing/execCommand/5142012-1-expected.txt: anonymous render block differences.
+        * platform/mac/editing/execCommand/nsresponder-outdent-expected.txt: Ditto.
+        * platform/mac/editing/inserting/insert-at-end-02-expected.txt: Empty render text differences.
+        * platform/mac/editing/pasteboard/4989774-expected.txt: Ditto.
+
 2015-03-01  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Document some more debug assertions.
diff --git a/LayoutTests/fast/dom/HTMLElement/dynamic-editability-change-expected.txt b/LayoutTests/fast/dom/HTMLElement/dynamic-editability-change-expected.txt
new file mode 100644 (file)
index 0000000..b663837
--- /dev/null
@@ -0,0 +1,35 @@
+This test updates contenteditable content attribute, contentEditable IDL property, and -webkit-user-modify CSS property dynamically.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS newDoc("<div></div>"); $("div").isContentEditable is false
+PASS $("div").contentEditable = "true"; $("div").isContentEditable is true
+PASS newDoc("<div contenteditable=true></div>"); $("div").isContentEditable is true
+PASS $("div").contentEditable = "false"; $("div").isContentEditable is false
+PASS newDoc("<div contenteditable=plaintext-only></div>"); $("div").isContentEditable is true
+PASS $("div").contentEditable = "false"; $("div").isContentEditable is false
+
+Style rules
+PASS newDoc("<div></div><style> div { -webkit-user-modify: read-write; } </style>"); $("div").isContentEditable is true
+PASS $("style").textContent = ""; $("div").isContentEditable is false
+PASS $("style").textContent = "* { -webkit-user-modify: read-write-plaintext-only; }"; $("div").isContentEditable is true
+PASS newDoc("<div></div>"); style = element("style", "* { -webkit-user-modify: read-write }"); $("div").isContentEditable is false
+PASS $("body").appendChild(style); $("div").isContentEditable is true
+PASS newDoc("<div></div><style></style>"); $("div").isContentEditable is false
+PASS $("style").sheet.insertRule("* { -webkit-user-modify: read-write; }"); $("div").isContentEditable is true
+PASS $("style").sheet.insertRule("* { -webkit-user-modify: read-only !important; }"); $("div").isContentEditable is false
+
+Inline styles
+PASS newDoc("<div style='-webkit-user-modify:read-write'></div>"); $("div").isContentEditable is true
+PASS $("head").innerHTML = "<base href='http://localhost/'>"; $("div").isContentEditable is true
+PASS $("div").style.webkitUserModify = ""; $("div").isContentEditable is false
+PASS newDoc("<div></div>"); $("div").style.webkitUserModify = "read-write"; $("div").isContentEditable is true
+PASS $("div").setAttribute("style", ""); $("div").isContentEditable is false
+PASS newDoc("<div></div>"); $("div").isContentEditable is false
+PASS $("div").setAttribute("style", "-webkit-user-modify: read-write"); $("div").isContentEditable is true
+PASS $("div").setAttribute("style", "-webkit-user-modify: read-only"); $("div").isContentEditable is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/HTMLElement/dynamic-editability-change.html b/LayoutTests/fast/dom/HTMLElement/dynamic-editability-change.html
new file mode 100644 (file)
index 0000000..18c621e
--- /dev/null
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test-pre.js"></script>
+<script>
+
+function newDoc(markup) {
+    var iframe = document.createElement('iframe');
+    document.body.appendChild(iframe);
+    iframe.contentDocument.documentElement.innerHTML = '<head></head><body></body>';
+    iframe.contentDocument.body.innerHTML = markup;
+    window.doc = iframe.contentDocument;
+    window.$ = iframe.contentDocument.querySelector.bind(iframe.contentDocument);
+}
+
+function element(name, markup) {
+    var element = doc.createElement(name);
+    element.textContent = markup;
+    return element;
+}
+
+description('This test updates contenteditable content attribute, contentEditable IDL property, and -webkit-user-modify CSS property dynamically.');
+
+shouldBeFalse('newDoc("<div></div>"); $("div").isContentEditable');
+shouldBeTrue('$("div").contentEditable = "true"; $("div").isContentEditable');
+shouldBeTrue('newDoc("<div contenteditable=true></div>"); $("div").isContentEditable');
+shouldBeFalse('$("div").contentEditable = "false"; $("div").isContentEditable');
+shouldBeTrue('newDoc("<div contenteditable=plaintext-only></div>"); $("div").isContentEditable');
+shouldBeFalse('$("div").contentEditable = "false"; $("div").isContentEditable');
+
+debug('');
+debug('Style rules');
+shouldBeTrue('newDoc("<div></div><style> div { -webkit-user-modify: read-write; } </style>"); $("div").isContentEditable');
+shouldBeFalse('$("style").textContent = ""; $("div").isContentEditable');
+shouldBeTrue('$("style").textContent = "* { -webkit-user-modify: read-write-plaintext-only; }"; $("div").isContentEditable');
+shouldBeFalse('newDoc("<div></div>"); style = element("style", "* { -webkit-user-modify: read-write }"); $("div").isContentEditable');
+shouldBeTrue('$("body").appendChild(style); $("div").isContentEditable');
+shouldBeFalse('newDoc("<div></div><style></style>"); $("div").isContentEditable');
+shouldBeTrue('$("style").sheet.insertRule("* { -webkit-user-modify: read-write; }"); $("div").isContentEditable');
+shouldBeFalse('$("style").sheet.insertRule("* { -webkit-user-modify: read-only !important; }"); $("div").isContentEditable');
+
+debug('');
+debug('Inline styles');
+shouldBeTrue('newDoc("<div style=\'-webkit-user-modify:read-write\'></div>"); $("div").isContentEditable');
+shouldBeTrue('$("head").innerHTML = "<base href=\'http://localhost/\'>"; $("div").isContentEditable');
+shouldBeFalse('$("div").style.webkitUserModify = ""; $("div").isContentEditable');
+shouldBeTrue('newDoc("<div></div>"); $("div").style.webkitUserModify = "read-write"; $("div").isContentEditable');
+shouldBeFalse('$("div").setAttribute("style", ""); $("div").isContentEditable');
+shouldBeFalse('newDoc("<div></div>"); $("div").isContentEditable');
+shouldBeTrue('$("div").setAttribute("style", "-webkit-user-modify: read-write"); $("div").isContentEditable');
+shouldBeFalse('$("div").setAttribute("style", "-webkit-user-modify: read-only"); $("div").isContentEditable');
+
+var iframes = document.querySelectorAll('iframe');
+for (var i = 0; i < iframes.length; i++)
+    document.body.removeChild(iframes[i]);
+
+var successfullyParsed = true;
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index 45d02388cc7401964bc4729fdae4b95ab8a9b0aa..ffa52b9f5ec21f743fa3b941cabd67a8723eef4e 100644 (file)
@@ -9,6 +9,7 @@ layer at (0,0) size 800x600
           text run at (457,0) width 324: "There shouldn't be any links *inside the selection*"
           text run at (0,18) width 43: "below."
       RenderBlock {DIV} at (0,52) size 784x36
+        RenderBlock (anonymous) at (0,0) size 784x0
         RenderBlock {DIV} at (0,0) size 784x18
           RenderInline {A} at (0,0) size 24x18 [color=#0000EE]
             RenderText {#text} at (0,0) size 24x18
@@ -16,7 +17,6 @@ layer at (0,0) size 800x600
           RenderInline {SPAN} at (0,0) size 13x18
             RenderText {#text} at (23,0) size 13x18
               text run at (23,0) width 13: "lo"
-        RenderBlock (anonymous) at (0,18) size 784x0
         RenderBlock {DIV} at (0,18) size 784x18
           RenderInline {SPAN} at (0,0) size 16x18
             RenderText {#text} at (0,0) size 16x18
index 43b3d22ed4c4317ffac74be66544c56508fbec2f..42b0718020a3e8816276564c4598ecb01dc75529 100644 (file)
@@ -14,6 +14,7 @@ layer at (0,0) size 800x600
           text run at (0,0) width 201: "This tests the outdent: method. "
           text run at (200,0) width 257: "You should see an undented 'foo' below."
       RenderBlock {DIV} at (0,34) size 784x18
-        RenderText {#text} at (0,0) size 22x18
-          text run at (0,0) width 22: "foo"
+        RenderBlock (anonymous) at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 22x18
+            text run at (0,0) width 22: "foo"
 caret: position 0 of child 0 {#text} of child 2 {DIV} of body
index 05690fc73cfba1d07167b335e0ee21e092607036..3f8e292b285383cf0a1ca620abb69afa1579a3da 100644 (file)
@@ -29,4 +29,5 @@ layer at (0,0) size 800x600
         RenderBlock (anonymous) at (2,46) size 780x18
           RenderText {#text} at (0,0) size 8x18
             text run at (0,0) width 8: "x"
+          RenderText {#text} at (0,0) size 0x0
 caret: position 1 of child 5 {#text} of child 5 {DIV} of body
index 97488f3b9ae8058971158a65cdec426aab0dd327..50ef27ddca42bb0b7ae5a34acadeee08e8141a7e 100644 (file)
@@ -12,4 +12,6 @@ layer at (0,0) size 800x600
         text run at (634,103) width 102: "You should see"
         text run at (735,103) width 5: " "
         text run at (0,121) width 364: "several pictures above all in the same line/paragraph."
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
 caret: position 164 of child 4 {#text} of body
index c008379012e3475662e38835239fc6455b3cab44..f8db0aa2a967e9ff2c09094dc56ee84dc700550f 100644 (file)
@@ -1,3 +1,68 @@
+2015-03-01  Ryosuke Niwa  <rniwa@webkit.org>
+
+        isContentEditable shouldn't trigger synchronous style recalc in most cases
+        https://bugs.webkit.org/show_bug.cgi?id=129034
+
+        Reviewed by Antti Koivisto.
+
+        Avoid style recalc inside isContentEditable when the document doesn't contain -webkit-user-modify or
+        -webkit-user-select: all. Instead, compute the value from contenteditable attributes in ancestors.
+        However, still compute the editability from the style tree when it's up-to-date in order to avoid
+        repeatedly walking up the DOM tree in a hot code path inside editing.
+
+        Test: fast/dom/HTMLElement/dynamic-editability-change.html
+
+        * css/CSSGrammar.y.in: No need to pass in "true" as we never call this function with false.
+        * css/CSSParser.cpp:
+        (WebCore::isValidKeywordPropertyAndValue): Calls parserSetUsesStyleBasedEditability as needed.
+        (WebCore::parseKeywordValue): Passes around StyleSheetContents*.
+        (WebCore::CSSParser::parseValue): Ditto.
+        (WebCore::CSSParser::parseFont): Ditto.
+
+        * css/StyleSheetContents.cpp:
+        (WebCore::StyleSheetContents::StyleSheetContents): Initializes and copies m_usesStyleBasedEditability.
+
+        * css/StyleSheetContents.h:
+        (WebCore::StyleSheetContents::parserSetUsesRemUnits): Removed the argument since it was always true.
+        (WebCore::StyleSheetContents::parserSetUsesStyleBasedEditability): Added.
+        (WebCore::StyleSheetContents::usesStyleBasedEditability): Added.
+
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle): Added a FIXME as well as a comment explaining why we don't call
+        setUsesStyleBasedEditability. Since Node::computeEditability triggers style recalc only when the flag
+        is set to true, it's too late to update the flag here.
+        (WebCore::Document::updateStyleIfNeeded): Uses a newly extracted needsStyleRecalc.
+        (WebCore::Document::updateBaseURL): Preserves m_usesStyleBasedEditability as well as m_usesRemUnit.
+        (WebCore::Document::usesStyleBasedEditability): Added. Returns true when inline style declarations or
+        any active stylesheet uses -webkit-user-modify or -webkit-user-select: all. Flushing pending stylesheet
+        changes here is fine because the alternative is to trigger a full blown style recalc.
+
+        * dom/Document.h:
+        (WebCore::Document::needsStyleRecalc): Added. Extracted from updateStyleIfNeeded.
+
+        * dom/DocumentStyleSheetCollection.cpp:
+        (WebCore::DocumentStyleSheetCollection::DocumentStyleSheetCollection):
+        (WebCore::styleSheetsUseRemUnits): Deleted.
+        (WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets): Updates m_usesStyleBasedEditability
+        as well as m_usesRemUnit.
+
+        * dom/DocumentStyleSheetCollection.h:
+        (WebCore::DocumentStyleSheetCollection::usesStyleBasedEditability): Added.
+        (WebCore::DocumentStyleSheetCollection::setUsesStyleBasedEditability): Added.
+
+        * dom/Node.cpp:
+        (WebCore::computeEditabilityFromComputedStyle): Extracted from computeEditability.
+        (WebCore::Node::computeEditability): When the style recalc is requested and the render tree is dirty,
+        check if the document uses any CSS property that can affect the editability of elements. If it doesn't,
+        compute the editability from contenteditable attributes in the anchors via matchesReadWritePseudoClass.
+        Continue to use the style-based computation when the render tree isn't dirty to avoid the tree walk.
+
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::editabilityFromContentEditableAttr): Extracted from matchesReadWritePseudoClass
+        to be called in Node::computeEditability. Also made it return Editability instead of boolean.
+        (WebCore::HTMLElement::matchesReadWritePseudoClass):
+        * html/HTMLElement.h:
+
 2015-03-01  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Unreviewed build fix.
index efdb2bb848833bbc6464df5d0f8a8a9c3f2502c0..1918fc9985b14b6d8917b6d62c66798da02d022e 100644 (file)
@@ -1700,7 +1700,7 @@ unary_term:
       $$.fValue = $1;
       $$.unit = CSSPrimitiveValue::CSS_REMS;
       if (parser->m_styleSheet)
-          parser->m_styleSheet->parserSetUsesRemUnits(true);
+          parser->m_styleSheet->parserSetUsesRemUnits();
   }
   | CHS { $$.id = CSSValueInvalid; $$.fValue = $1; $$.unit = CSSPrimitiveValue::CSS_CHS; }
   | VW { $$.id = CSSValueInvalid; $$.fValue = $1; $$.unit = CSSPrimitiveValue::CSS_VW; }
index 8e4635e8c2623b78dc7b21bf0d76244f046039d6..3583d210adbde56eaef69ddbaa9d32b305ddcb9d 100644 (file)
@@ -640,7 +640,7 @@ static bool parseSimpleLengthValue(MutableStyleProperties* declaration, CSSPrope
     return true;
 }
 
-static inline bool isValidKeywordPropertyAndValue(CSSPropertyID propertyId, int valueID, const CSSParserContext& parserContext)
+static inline bool isValidKeywordPropertyAndValue(CSSPropertyID propertyId, int valueID, const CSSParserContext& parserContext, StyleSheetContents* styleSheetContents)
 {
     if (!valueID)
         return false;
@@ -1015,12 +1015,20 @@ static inline bool isValidKeywordPropertyAndValue(CSSPropertyID propertyId, int
             return true;
         break;
     case CSSPropertyWebkitUserModify: // read-only | read-write
-        if (valueID == CSSValueReadOnly || valueID == CSSValueReadWrite || valueID == CSSValueReadWritePlaintextOnly)
+        if (valueID == CSSValueReadOnly || valueID == CSSValueReadWrite || valueID == CSSValueReadWritePlaintextOnly) {
+            if (styleSheetContents)
+                styleSheetContents->parserSetUsesStyleBasedEditability();
             return true;
+        }
         break;
     case CSSPropertyWebkitUserSelect: // auto | none | text | all
-        if (valueID == CSSValueAuto || valueID == CSSValueNone || valueID == CSSValueText || valueID == CSSValueAll)
+        if (valueID == CSSValueAuto || valueID == CSSValueNone || valueID == CSSValueText)
+            return true;
+        if (valueID == CSSValueAll) {
+            if (styleSheetContents)
+                styleSheetContents->parserSetUsesStyleBasedEditability();
             return true;
+        }
         break;
     case CSSPropertyWebkitWritingMode:
         if (valueID >= CSSValueHorizontalTb && valueID <= CSSValueHorizontalBt)
@@ -1176,7 +1184,7 @@ static inline bool isKeywordPropertyID(CSSPropertyID propertyId)
     }
 }
 
-static bool parseKeywordValue(MutableStyleProperties* declaration, CSSPropertyID propertyId, const String& string, bool important, const CSSParserContext& parserContext)
+static bool parseKeywordValue(MutableStyleProperties* declaration, CSSPropertyID propertyId, const String& string, bool important, const CSSParserContext& parserContext, StyleSheetContents* styleSheetContents)
 {
     ASSERT(!string.isEmpty());
 
@@ -1203,7 +1211,7 @@ static bool parseKeywordValue(MutableStyleProperties* declaration, CSSPropertyID
         value = cssValuePool().createInheritedValue();
     else if (valueID == CSSValueInitial)
         value = cssValuePool().createExplicitInitialValue();
-    else if (isValidKeywordPropertyAndValue(propertyId, valueID, parserContext))
+    else if (isValidKeywordPropertyAndValue(propertyId, valueID, parserContext, styleSheetContents))
         value = cssValuePool().createIdentifierValue(valueID);
     else
         return false;
@@ -1308,7 +1316,7 @@ bool CSSParser::parseValue(MutableStyleProperties* declaration, CSSPropertyID pr
         context.mode = cssParserMode;
     }
 
-    if (parseKeywordValue(declaration, propertyID, string, important, context))
+    if (parseKeywordValue(declaration, propertyID, string, important, context, contextStyleSheet))
         return true;
     if (parseTranslateTransformValue(declaration, propertyID, string, important))
         return true;
@@ -1886,7 +1894,7 @@ bool CSSParser::parseValue(CSSPropertyID propId, bool important)
     }
 
     if (isKeywordPropertyID(propId)) {
-        if (!isValidKeywordPropertyAndValue(propId, id, m_context))
+        if (!isValidKeywordPropertyAndValue(propId, id, m_context, m_styleSheet))
             return false;
         if (m_valueList->next() && !inShorthand())
             return false;
@@ -6297,7 +6305,7 @@ bool CSSParser::parseFont(bool important)
     bool fontWeightParsed = false;
     CSSParserValue* value;
     while ((value = m_valueList->current())) {
-        if (!fontStyleParsed && isValidKeywordPropertyAndValue(CSSPropertyFontStyle, value->id, m_context)) {
+        if (!fontStyleParsed && isValidKeywordPropertyAndValue(CSSPropertyFontStyle, value->id, m_context, m_styleSheet)) {
             addProperty(CSSPropertyFontStyle, cssValuePool().createIdentifierValue(value->id), important);
             fontStyleParsed = true;
         } else if (!fontVariantParsed && (value->id == CSSValueNormal || value->id == CSSValueSmallCaps)) {
index eafbd920d68292f803369a9747c43016feafd592..741cb64abb483a721edb015c86a0cf10dbf28a52 100644 (file)
@@ -64,6 +64,7 @@ StyleSheetContents::StyleSheetContents(StyleRuleImport* ownerRule, const String&
     , m_hasSyntacticallyValidCSSHeader(true)
     , m_didLoadErrorOccur(false)
     , m_usesRemUnits(false)
+    , m_usesStyleBasedEditability(false)
     , m_isMutable(false)
     , m_isInMemoryCache(false)
     , m_parserContext(context)
@@ -83,6 +84,7 @@ StyleSheetContents::StyleSheetContents(const StyleSheetContents& o)
     , m_hasSyntacticallyValidCSSHeader(o.m_hasSyntacticallyValidCSSHeader)
     , m_didLoadErrorOccur(false)
     , m_usesRemUnits(o.m_usesRemUnits)
+    , m_usesStyleBasedEditability(o.m_usesStyleBasedEditability)
     , m_isMutable(false)
     , m_isInMemoryCache(false)
     , m_parserContext(o.m_parserContext)
index 62a93d5032c789909c0e013233a05f8251c10f72..e14b07a3b33226308137d387658a7e51d1b1903b 100644 (file)
@@ -91,7 +91,8 @@ public:
     void parserAddNamespace(const AtomicString& prefix, const AtomicString& uri);
     void parserAppendRule(PassRefPtr<StyleRuleBase>);
     void parserSetEncodingFromCharsetRule(const String& encoding); 
-    void parserSetUsesRemUnits(bool b) { m_usesRemUnits = b; }
+    void parserSetUsesRemUnits() { m_usesRemUnits = true; }
+    void parserSetUsesStyleBasedEditability() { m_usesStyleBasedEditability = true; }
 
     void clearRules();
 
@@ -117,6 +118,7 @@ public:
     StyleRuleBase* ruleAt(unsigned index) const;
 
     bool usesRemUnits() const { return m_usesRemUnits; }
+    bool usesStyleBasedEditability() const { return m_usesStyleBasedEditability; }
 
     unsigned estimatedSizeInBytes() const;
     
@@ -159,6 +161,7 @@ private:
     bool m_hasSyntacticallyValidCSSHeader : 1;
     bool m_didLoadErrorOccur : 1;
     bool m_usesRemUnits : 1;
+    bool m_usesStyleBasedEditability : 1;
     bool m_isMutable : 1;
     bool m_isInMemoryCache : 1;
     
index 2881acf86e45a801fd0bceddc1a6bcc07cfbfc2c..bade24956cd33538ef10c270752eb7252473bb50 100644 (file)
@@ -1747,8 +1747,11 @@ void Document::recalcStyle(Style::Change change)
 
     InspectorInstrumentationCookie cookie = InspectorInstrumentation::willRecalculateStyle(*this);
 
+    // FIXME: We never reset this flags.
     if (m_elementSheet && m_elementSheet->contents().usesRemUnits())
         m_styleSheetCollection.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;
     {
@@ -1808,7 +1811,7 @@ void Document::updateStyleIfNeeded()
     if (m_optimizedStyleSheetUpdateTimer.isActive())
         styleResolverChanged(RecalcStyleIfNeeded);
 
-    if ((!m_pendingStyleRecalcShouldForce && !childNeedsStyleRecalc()) || inPageCache())
+    if (!needsStyleRecalc())
         return;
 
     recalcStyle(Style::NoChange);
@@ -2683,9 +2686,13 @@ void Document::updateBaseURL()
         // 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.
-        m_elementSheet->contents().parserSetUsesRemUnits(usesRemUnits);
+        if (usesRemUnits)
+            m_elementSheet->contents().parserSetUsesRemUnits();
+        if (usesStyleBasedEditability)
+            m_elementSheet->contents().parserSetUsesStyleBasedEditability();
     }
 
     if (!equalIgnoringFragmentIdentifier(oldBaseURL, m_baseURL)) {
@@ -2852,6 +2859,19 @@ CSSStyleSheet& Document::elementSheet()
     return *m_elementSheet;
 }
 
+bool Document::usesStyleBasedEditability() const
+{
+    if (m_elementSheet && m_elementSheet->contents().usesStyleBasedEditability())
+        return true;
+
+    ASSERT(!m_renderView || !m_renderView->frameView().isPainting());
+    ASSERT(!m_inStyleRecalc);
+
+    auto& collection = document().styleSheetCollection();
+    collection.flushPendingUpdates();
+    return collection.usesStyleBasedEditability();
+}
+
 void Document::processHttpEquiv(const String& equiv, const String& content)
 {
     ASSERT(!equiv.isNull() && !content.isNull());
index 76fb1b79ccebc468513ac56eb0b4380ff9f3d0e3..15444a6e19eb27c1a6a237dab9822ce7d7c689b1 100644 (file)
@@ -601,6 +601,7 @@ public:
 
     void recalcStyle(Style::Change = Style::NoChange);
     WEBCORE_EXPORT void updateStyleIfNeeded();
+    bool needsStyleRecalc() const { return !inPageCache() && (m_pendingStyleRecalcShouldForce || childNeedsStyleRecalc() || m_optimizedStyleSheetUpdateTimer.isActive()); }
 
     WEBCORE_EXPORT void updateLayout();
     
@@ -693,6 +694,7 @@ public:
     Frame* findUnsafeParentScrollPropagationBoundary();
 
     CSSStyleSheet& elementSheet();
+    bool usesStyleBasedEditability() const;
     
     virtual Ref<DocumentParser> createParser();
     DocumentParser* parser() const { return m_parser.get(); }
index 8ae31e96314e2b989783c7ffba65ff9992343edb..0fcb81b6e990101fc0fdffd537f488fbc805cd23 100644 (file)
@@ -60,6 +60,7 @@ DocumentStyleSheetCollection::DocumentStyleSheetCollection(Document& document)
     , m_usesFirstLineRules(false)
     , m_usesFirstLetterRules(false)
     , m_usesRemUnits(false)
+    , m_usesStyleBasedEditability(false)
 {
 }
 
@@ -390,15 +391,6 @@ void DocumentStyleSheetCollection::analyzeStyleSheetChange(UpdateFlag updateFlag
     requiresFullStyleRecalc = false;
 }
 
-static bool styleSheetsUseRemUnits(const Vector<RefPtr<CSSStyleSheet>>& sheets)
-{
-    for (unsigned i = 0; i < sheets.size(); ++i) {
-        if (sheets[i]->contents().usesRemUnits())
-            return true;
-    }
-    return false;
-}
-
 static void filterEnabledNonemptyCSSStyleSheets(Vector<RefPtr<CSSStyleSheet>>& result, const Vector<RefPtr<StyleSheet>>& sheets)
 {
     for (unsigned i = 0; i < sheets.size(); ++i) {
@@ -457,7 +449,12 @@ bool DocumentStyleSheetCollection::updateActiveStyleSheets(UpdateFlag updateFlag
     m_activeAuthorStyleSheets.swap(activeCSSStyleSheets);
     m_styleSheetsForStyleSheetList.swap(activeStyleSheets);
 
-    m_usesRemUnits = styleSheetsUseRemUnits(m_activeAuthorStyleSheets);
+    for (const auto& sheet : m_activeAuthorStyleSheets) {
+        if (sheet->contents().usesRemUnits())
+            m_usesRemUnits = true;
+        if (sheet->contents().usesStyleBasedEditability())
+            m_usesStyleBasedEditability = true;
+    }
     m_pendingUpdateType = NoUpdate;
 
     return requiresFullStyleRecalc;
index eede7bf42514dbed9a420a4df86d63652245b628..60fb783a77553074f51f1fc5fb99137d81e001ad 100644 (file)
@@ -105,6 +105,8 @@ public:
     bool usesFirstLetterRules() const { return m_usesFirstLetterRules; }
     bool usesRemUnits() const { return m_usesRemUnits; }
     void setUsesRemUnit(bool b) { m_usesRemUnits = b; }
+    bool usesStyleBasedEditability() { return m_usesStyleBasedEditability; }
+    void setUsesStyleBasedEditability(bool b) { m_usesStyleBasedEditability = b; }
 
     void combineCSSFeatureFlags();
     void resetCSSFeatureFlags();
@@ -157,6 +159,7 @@ private:
     bool m_usesFirstLineRules;
     bool m_usesFirstLetterRules;
     bool m_usesRemUnits;
+    bool m_usesStyleBasedEditability;
 };
 
 }
index f44876474d43b28859ebadd05428dfa2e29d2899..35724875b7d1820d8f6a6fc1e3084a9a25782f22 100644 (file)
@@ -71,6 +71,7 @@
 #include "Settings.h"
 #include "StorageEvent.h"
 #include "StyleResolver.h"
+#include "StyleSheetContents.h"
 #include "TemplateContentDocumentFragment.h"
 #include "TextEvent.h"
 #include "TouchEvent.h"
@@ -553,24 +554,13 @@ void Node::inspect()
         document().page()->inspectorController().inspect(this);
 }
 
-Node::Editability Node::computeEditability(UserSelectAllTreatment treatment, ShouldUpdateStyle shouldUpdateStyle) const
+static Node::Editability computeEditabilityFromComputedStyle(const Node& startNode, Node::UserSelectAllTreatment treatment)
 {
-    if (shouldUpdateStyle == ShouldUpdateStyle::Update)
-        document().updateStyleIfNeeded();
-
-    if (!document().hasLivingRenderTree())
-        return Editability::ReadOnly;
-    if (document().frame() && document().frame()->page() && document().frame()->page()->isEditable() && !containingShadowRoot())
-        return Editability::CanEditRichly;
-
-    if (isPseudoElement())
-        return Editability::ReadOnly;
-
     // Ideally we'd call ASSERT(!needsStyleRecalc()) here, but
     // ContainerNode::setFocus() calls setNeedsStyleRecalc(), so the assertion
     // would fire in the middle of Document::setFocusedElement().
 
-    for (const Node* node = this; node; node = node->parentNode()) {
+    for (const Node* node = &startNode; node; node = node->parentNode()) {
         RenderStyle* style = node->isDocumentNode() ? node->renderStyle() : const_cast<Node*>(node)->computedStyle();
         if (!style)
             continue;
@@ -579,23 +569,39 @@ Node::Editability Node::computeEditability(UserSelectAllTreatment treatment, Sho
 #if ENABLE(USERSELECT_ALL)
         // Elements with user-select: all style are considered atomic
         // therefore non editable.
-        if (treatment == UserSelectAllIsAlwaysNonEditable && style->userSelect() == SELECT_ALL)
-            return Editability::ReadOnly;
+        if (treatment == Node::UserSelectAllIsAlwaysNonEditable && style->userSelect() == SELECT_ALL)
+            return Node::Editability::ReadOnly;
 #else
         UNUSED_PARAM(treatment);
 #endif
         switch (style->userModify()) {
         case READ_ONLY:
-            return Editability::ReadOnly;
+            return Node::Editability::ReadOnly;
         case READ_WRITE:
-            return Editability::CanEditRichly;
+            return Node::Editability::CanEditRichly;
         case READ_WRITE_PLAINTEXT_ONLY:
-            return Editability::CanEditPlainText;
+            return Node::Editability::CanEditPlainText;
         }
         ASSERT_NOT_REACHED();
+        return Node::Editability::ReadOnly;
+    }
+    return Node::Editability::ReadOnly;
+}
+
+Node::Editability Node::computeEditability(UserSelectAllTreatment treatment, ShouldUpdateStyle shouldUpdateStyle) const
+{
+    if (!document().hasLivingRenderTree() || isPseudoElement())
         return Editability::ReadOnly;
+
+    if (document().frame() && document().frame()->page() && document().frame()->page()->isEditable() && !containingShadowRoot())
+        return Editability::CanEditRichly;
+
+    if (shouldUpdateStyle == ShouldUpdateStyle::Update && document().needsStyleRecalc()) {
+        if (!document().usesStyleBasedEditability())
+            return HTMLElement::editabilityFromContentEditableAttr(*this);
+        document().updateStyleIfNeeded();
     }
-    return Editability::ReadOnly;
+    return computeEditabilityFromComputedStyle(*this, treatment);
 }
 
 RenderBox* Node::renderBox() const
index 8ef5fdd9a597a195f3e43938de235ef9dc4b5963..38f489c35a8f8289e6d48b6165df5dea05998c06 100644 (file)
@@ -367,28 +367,35 @@ void HTMLElement::populateEventNameForAttributeLocalNameMap(HashMap<AtomicString
         map.add(customTable[i].attributeName.localName().impl(), customTable[i].eventName);
 }
 
-bool HTMLElement::matchesReadWritePseudoClass() const
+Node::Editability HTMLElement::editabilityFromContentEditableAttr(const Node& node)
 {
-    const Element* currentElement = this;
+    const Node* currentNode = &node;
     do {
-        if (is<HTMLElement>(*currentElement)) {
-            switch (contentEditableType(downcast<HTMLElement>(*currentElement))) {
+        if (is<HTMLElement>(*currentNode)) {
+            switch (contentEditableType(downcast<HTMLElement>(*currentNode))) {
             case ContentEditableType::True:
+                return Node::Editability::CanEditRichly;
             case ContentEditableType::PlaintextOnly:
-                return true;
+                return Node::Editability::CanEditPlainText;
             case ContentEditableType::False:
-                return false;
+                return Node::Editability::ReadOnly;
             case ContentEditableType::Inherit:
                 break;
             }
         }
-        currentElement = currentElement->parentElement();
-    } while (currentElement);
+        currentNode = currentNode->parentNode();
+    } while (currentNode);
 
-    const Document& document = this->document();
+    const Document& document = node.document();
     if (is<HTMLDocument>(document))
-        return downcast<HTMLDocument>(document).inDesignMode();
-    return false;
+        return downcast<HTMLDocument>(document).inDesignMode() ? Node::Editability::CanEditRichly : Node::Editability::ReadOnly;
+
+    return Node::Editability::ReadOnly;
+}
+
+bool HTMLElement::matchesReadWritePseudoClass() const
+{
+    return editabilityFromContentEditableAttr(*this) != Editability::ReadOnly;
 }
 
 void HTMLElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
index 20dd3c1ded28187228a669a81f00bb7be1db4383..58053b7a27bb7ac8fb2fc167e7605d3a9f46adf2 100644 (file)
@@ -61,6 +61,8 @@ public:
     String contentEditable() const;
     void setContentEditable(const String&, ExceptionCode&);
 
+    static Editability editabilityFromContentEditableAttr(const Node&);
+
     virtual bool draggable() const;
     void setDraggable(bool);