Changeset 286949 in webkit

Timestamp:

Dec 13, 2021, 8:50:00 AM (5 years ago)

Author:

Antti Koivisto

Message:

[CSS Cascade Layers] Unflake and fix web-platform-tests/css/css-cascade/layer-statement-before-import.html
​https://bugs.webkit.org/show_bug.cgi?id=233944

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-cascade/layer-statement-before-import-expected.txt:
• web-platform-tests/css/css-cascade/layer-statement-before-import.html:

Data URLs in @imports are not guaranteed to decode synchronously so make the test async.
Also catch exceptions from asserts for cleanup so failures don't affect other subtests.

Source/WebCore:

Inserting rules before early layer statements may be legal if they are moved to the regular rule list.

• css/StyleSheetContents.cpp:

(WebCore::StyleSheetContents::wrapperInsertRule):

LayoutTests:

• TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/layer-statement-before-import-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/layer-statement-before-import.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/StyleSheetContents.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-12-13  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] Unflake and fix web-platform-tests/css/css-cascade/layer-statement-before-import.html
+        https://bugs.webkit.org/show_bug.cgi?id=233944
+
+        Reviewed by Simon Fraser.
+
+        * TestExpectations:
+
 2021-12-13  Tyler Wilcock  <tyler_w@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -2186 +2186 @@
 webkit.org/b/233937 imported/w3c/web-platform-tests/css/css-cascade/revert-layer-011.html [ ImageOnlyFailure ]
 webkit.org/b/233937 imported/w3c/web-platform-tests/css/css-cascade/revert-layer-012.html [ ImageOnlyFailure ]
-webkit.org/b/233944 imported/w3c/web-platform-tests/css/css-cascade/layer-statement-before-import.html [ Pass Failure ]
 
 webkit.org/b/148801 imported/w3c/web-platform-tests/css/css-color/t422-rgba-onscreen-b.xht [ ImageOnlyFailure ]

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2021-12-13  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] Unflake and fix web-platform-tests/css/css-cascade/layer-statement-before-import.html
+        https://bugs.webkit.org/show_bug.cgi?id=233944
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-cascade/layer-statement-before-import-expected.txt:
+        * web-platform-tests/css/css-cascade/layer-statement-before-import.html:
+
+        Data URLs in @imports are not guaranteed to decode synchronously so make the test async.
+        Also catch exceptions from asserts for cleanup so failures don't affect other subtests.
+
 2021-12-13  Youenn Fablet  <youenn@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/layer-statement-before-import-expected.txt

@@ -1 @@
-CONSOLE MESSAGE: TypeError: Illegal constructor
 
-Harness Error (FAIL), message = TypeError: Illegal constructor
+PASS length and item
+PASS insertRule before imports
+PASS insertRule after imports
+PASS insert other rules to pre-import layer statements fails
+PASS insert other rules before the first layer statement without imports
+PASS deleteRule before imports
+PASS deleteRule after imports
+FAIL replaceSync clears stale layer statements promise_test: Unhandled rejection with value: object "TypeError: Illegal constructor"
 
-FAIL length and item assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 0)"
-FAIL insertRule before imports assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 0)"
-FAIL insertRule after imports assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 0)"
-FAIL insert other rules to pre-import layer statements fails assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 0)"
-FAIL insert other rules before the first layer statement without imports The operation would yield an incorrect node tree.
-FAIL deleteRule before imports assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 0)"
-FAIL deleteRule after imports assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 0)"
-

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/layer-statement-before-import.html

@@ -160 +160 @@
 
 for (let testCase of testCases) {
-  let styleElement;
-  let sheet;
-  if (!testCase.constructedStyleSheet) {
-    styleElement = document.createElement('style');
-    styleElement.textContent = testCase.style;
-    document.head.append(styleElement);
-    sheet = styleElement.sheet;
-  } else {
-    sheet = new CSSStyleSheet();
-    sheet.replaceSync(testCase.style);
-    document.adoptedStyleSheets = [sheet];
-  }
+  promise_test(async t => {
+    let styleElement;
+    let sheet;
+    if (!testCase.constructedStyleSheet) {
+      styleElement = document.createElement('style');
+      styleElement.textContent = testCase.style;
+      await new Promise(resolve => {
+        styleElement.onload = resolve;
+        styleElement.onerror = resolve;
+        document.head.append(styleElement);
+      });
+      sheet = styleElement.sheet;
+    } else {
+      sheet = new CSSStyleSheet();
+      sheet.replaceSync(testCase.style);
+      document.adoptedStyleSheets = [sheet];
+    }
 
-  test(() => {
-    testCase.operations(sheet);
-    assert_equals(getComputedStyle(target).color, getComputedStyle(reference).color);
-  },testCase.title);
-
-  if (styleElement)
-    styleElement.remove();
-  document.adoptedStyleSheets = [];
+    try {
+      testCase.operations(sheet);
+      assert_equals(getComputedStyle(target).color, getComputedStyle(reference).color);
+    } finally {
+      if (styleElement)
+        styleElement.remove();
+      document.adoptedStyleSheets = [];
+    }
+  }, testCase.title);
 }
 </script>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-13  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] Unflake and fix web-platform-tests/css/css-cascade/layer-statement-before-import.html
+        https://bugs.webkit.org/show_bug.cgi?id=233944
+
+        Reviewed by Simon Fraser.
+
+        Inserting rules before early layer statements may be legal if they are moved to the regular rule list.
+
+        * css/StyleSheetContents.cpp:
+        (WebCore::StyleSheetContents::wrapperInsertRule):
+
 2021-12-13  Adrian Perez de Castro  <aperez@igalia.com>
 

trunk/Source/WebCore/css/StyleSheetContents.cpp

@@ -238 +238 @@
     // Parser::parseRule doesn't currently allow @charset so we don't need to deal with it.
     ASSERT(!rule->isCharsetRule());
-    
+
+    // Maybe the insert will be legal if we treat early layer statement rules as normal child rules?
+    auto shouldMoveLayerRulesBeforeImportToNormalChildRules = [&] {
+        if (index >= m_layerRulesBeforeImportRules.size())
+            return false;
+        if (!m_importRules.isEmpty() || !m_namespaceRules.isEmpty())
+            return false;
+        bool isLayerStatement = is<StyleRuleLayer>(rule) && downcast<StyleRuleLayer>(rule.get()).isStatement();
+        return !rule->isImportRule() && !rule->isNamespaceRule() && !isLayerStatement;
+    };
+
+    if (shouldMoveLayerRulesBeforeImportToNormalChildRules())
+        m_childRules.insertVector(0, std::exchange(m_layerRulesBeforeImportRules, { }));
+
     unsigned childVectorIndex = index;
     if (childVectorIndex < m_layerRulesBeforeImportRules.size() || (childVectorIndex == m_layerRulesBeforeImportRules.size() && is<StyleRuleLayer>(rule))) {
@@ -269 +282 @@
 
     if (childVectorIndex < m_namespaceRules.size() || (childVectorIndex == m_namespaceRules.size() && rule->isNamespaceRule())) {
-        // Inserting non-namespace rules other than import rule before @namespace is
+        // Inserting non-namespace rules other than import and layer statement rules before @namespace is
         // not allowed.
         if (!is<StyleRuleNamespace>(rule))