Changeset 260723 in webkit

Timestamp:

Apr 26, 2020 12:37:24 AM (4 years ago)

Author:

cathiechen

Message:

fast/scrolling/scroll-behavior-invalidate-if-disabled.html is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=210917

Reviewed by Darin Adler.

The flaky failure is caused by reusing the CSSPropertyInfo value cached propertyInfoCache
after experimental flags changed. Add propertyInfoFromJavaScriptCSSPropertyName
to perform disabled checking after parsing. If the property is disabled, it will return
an invalid CSSPropertyInfo instead.

• css/CSSStyleDeclaration.cpp:

(WebCore::CSSStyleDeclaration::getCSSPropertyIDFromJavaScriptPropertyName):
(WebCore::CSSStyleDeclaration::namedItem):
(WebCore::CSSStyleDeclaration::setNamedItem):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/CSSStyleDeclaration.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-04-26  Cathie Chen  <cathiechen@igalia.com>
+
+        fast/scrolling/scroll-behavior-invalidate-if-disabled.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=210917
+
+        Reviewed by Darin Adler.
+
+        The flaky failure is caused by reusing the CSSPropertyInfo value cached propertyInfoCache
+        after experimental flags changed. Add propertyInfoFromJavaScriptCSSPropertyName 
+        to perform disabled checking after parsing. If the property is disabled, it will return
+        an invalid CSSPropertyInfo instead.
+
+        * css/CSSStyleDeclaration.cpp:
+        (WebCore::CSSStyleDeclaration::getCSSPropertyIDFromJavaScriptPropertyName):
+        (WebCore::CSSStyleDeclaration::namedItem):
+        (WebCore::CSSStyleDeclaration::setNamedItem):
+
 2020-04-25  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Source/WebCore/css/CSSStyleDeclaration.cpp

@@ -153 +153 @@
 };
 
-static CSSPropertyInfo parseJavaScriptCSSPropertyName(const AtomString& propertyName, const Settings* settingsPtr)
+static CSSPropertyInfo parseJavaScriptCSSPropertyName(const AtomString& propertyName)
 {
     using CSSPropertyInfoMap = HashMap<String, CSSPropertyInfo>;
@@ -251 +251 @@
     if (auto propertyID = hashTableEntry ? hashTableEntry->id : 0) {
         auto id = static_cast<CSSPropertyID>(propertyID);
-        if (isEnabledCSSProperty(id) && isCSSPropertyEnabledBySettings(id, settingsPtr)) {
-            propertyInfo.hadPixelOrPosPrefix = hadPixelOrPosPrefix;
-            propertyInfo.propertyID = id;
-            propertyInfoCache.get().add(propertyNameString, propertyInfo);
-        }
+        propertyInfo.hadPixelOrPosPrefix = hadPixelOrPosPrefix;
+        propertyInfo.propertyID = id;
+        propertyInfoCache.get().add(propertyNameString, propertyInfo);
     }
     return propertyInfo;
 }
 
+static CSSPropertyInfo propertyInfoFromJavaScriptCSSPropertyName(const AtomString& propertyName, const Settings* settings)
+{
+    auto propertyInfo = parseJavaScriptCSSPropertyName(propertyName);
+    auto id = propertyInfo.propertyID;
+    if (!isEnabledCSSProperty(id) || !isCSSPropertyEnabledBySettings(id, settings))
+        return { CSSPropertyInvalid, false };
+    return propertyInfo;
+}
+
 }
 
 CSSPropertyID CSSStyleDeclaration::getCSSPropertyIDFromJavaScriptPropertyName(const AtomString& propertyName)
 {
-    return parseJavaScriptCSSPropertyName(propertyName, nullptr).propertyID;
+    return propertyInfoFromJavaScriptCSSPropertyName(propertyName, nullptr).propertyID;
 }
 
 Optional<Variant<String, double>> CSSStyleDeclaration::namedItem(const AtomString& propertyName)
 {
-    auto* settingsPtr = parentElement() ? &parentElement()->document().settings() : nullptr;
-    auto propertyInfo = parseJavaScriptCSSPropertyName(propertyName, settingsPtr);
+    auto* settings = parentElement() ? &parentElement()->document().settings() : nullptr;
+    auto propertyInfo = propertyInfoFromJavaScriptCSSPropertyName(propertyName, settings);
     if (!propertyInfo.propertyID)
         return WTF::nullopt;
@@ -292 +299 @@
 ExceptionOr<void> CSSStyleDeclaration::setNamedItem(const AtomString& propertyName, String value, bool& propertySupported)
 {
-    auto* settingsPtr = parentElement() ? &parentElement()->document().settings() : nullptr;
-    auto propertyInfo = parseJavaScriptCSSPropertyName(propertyName, settingsPtr);
+    auto* settings = parentElement() ? &parentElement()->document().settings() : nullptr;
+    auto propertyInfo = propertyInfoFromJavaScriptCSSPropertyName(propertyName, settings);
     if (!propertyInfo.propertyID) {
         propertySupported = false;