Changeset 260723 in webkit for trunk/Source/WebCore/css/CSSStyleDeclaration.cpp

Timestamp:

Apr 26, 2020, 12:37:24 AM (5 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):

File:

• trunk/Source/WebCore/css/CSSStyleDeclaration.cpp (modified) (3 diffs)

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;