Changeset 238758 in webkit

Timestamp:

Nov 30, 2018 3:33:39 PM (5 years ago)

Author:

rniwa@webkit.org

Message:

ShadowRoot should have styleSheets property
​https://bugs.webkit.org/show_bug.cgi?id=191311
LayoutTests/imported/w3c:

Reviewed by Antti Koivisto.

Rebaselined the tests.

• web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt: Rebaselined. The test now

doesn't throw but fails with the actual check the test is intending to check.

• web-platform-tests/css/cssom/selectorText-modification-restyle-002-expected.txt: Rebaselined

now that all test cases pass.

• web-platform-tests/shadow-dom/ShadowRoot-interface-expected.txt: Ditto.

Source/WebCore:

Reviewed by Antti Koivisto.

Added the support for ShadowRoot.prototype.styleSheets by making StyleSheetList refer to either
a document or a shadow root. We don't support the named getter in shadow root since that's not
a standard feature: ​https://drafts.csswg.org/cssom/#the-stylesheetlist-interface

Tests: fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html

imported/w3c/web-platform-tests/shadow-dom/ShadowRoot-interface.html

• css/StyleSheetList.cpp:

(WebCore::StyleSheetList::StyleSheetList): Added a variant which takes ShadowRoot.
(WebCore::StyleSheetList::styleSheets const):
(WebCore::StyleSheetList::ownerNode): Added. The replacement for document() since now the opaque
root could be either a Document or a ShadowRoot.
(WebCore::StyleSheetList::detach): Renamed from detachFromDocument.
(WebCore::StyleSheetList::namedItem const): Added a comment that the named getter is only supported
for Document since it's not in the standard.

• css/StyleSheetList.h:

(WebCore::StyleSheetList::create): Added a variant which takes ShadowRoot.
(WebCore::StyleSheetList::document): Deleted. Replaced by StyleSheetList::ownerNode in .cpp file.

• css/StyleSheetList.idl:
• dom/Document.cpp:

(WebCore::Document::~Document):
(WebCore::Document::styleSheets):

• dom/Document.idl:
• dom/DocumentOrShadowRoot.idl: Moved the declaration of styleSheets here from Document.idl.
• dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::~ShadowRoot): Call detach.
(WebCore::ShadowRoot::styleSheets):

• dom/ShadowRoot.h:

LayoutTests:

<rdar://problem/46333290>

Reviewed by Antti Koivisto.

Added a regression test for testing that the JS wrapper of a StyleSheetList does not get collected
as long as its shadow root is alive.

• fast/shadow-dom/shadowroot-stylesheets-wrapper-gc-expected.txt: Added.
• fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc-expected.txt (added)
• LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html (added)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/cssom/selectorText-modification-restyle-002-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/shadow-dom/ShadowRoot-interface-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/StyleSheetList.cpp (modified) (4 diffs)
• Source/WebCore/css/StyleSheetList.h (modified) (3 diffs)
• Source/WebCore/css/StyleSheetList.idl (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (2 diffs)
• Source/WebCore/dom/Document.idl (modified) (1 diff)
• Source/WebCore/dom/DocumentOrShadowRoot.idl (modified) (1 diff)
• Source/WebCore/dom/ShadowRoot.cpp (modified) (4 diffs)
• Source/WebCore/dom/ShadowRoot.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        ShadowRoot should have styleSheets property
+        https://bugs.webkit.org/show_bug.cgi?id=191311
+        <rdar://problem/46333290>
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test for testing that the JS wrapper of a StyleSheetList does not get collected
+        as long as its shadow root is alive.
+
+        * fast/shadow-dom/shadowroot-stylesheets-wrapper-gc-expected.txt: Added.
+        * fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html: Added.
+
 2018-11-30  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        ShadowRoot should have styleSheets property
+        https://bugs.webkit.org/show_bug.cgi?id=191311
+
+        Reviewed by Antti Koivisto.
+
+        Rebaselined the tests.
+
+        * web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt: Rebaselined. The test now
+        doesn't throw but fails with the actual check the test is intending to check.
+        * web-platform-tests/css/cssom/selectorText-modification-restyle-002-expected.txt: Rebaselined
+        now that all test cases pass.
+        * web-platform-tests/shadow-dom/ShadowRoot-interface-expected.txt: Ditto.
+
 2018-11-28  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt

@@ -1 +1 @@
 
-FAIL Title attribute in stylesheets not in the document tree is ignored undefined is not an object (evaluating 'host.shadowRoot.styleSheets.length')
+FAIL Title attribute in stylesheets not in the document tree is ignored assert_equals: expected 3 but got 2
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom/selectorText-modification-restyle-002-expected.txt

@@ -1 +1 @@
 
 PASS Check initial color. 
-FAIL Check that color changes correctly after shadow stylesheet selector and #container class is changed. undefined is not an object (evaluating 'root.styleSheets[0]')
+PASS Check that color changes correctly after shadow stylesheet selector and #container class is changed. 
 

trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/ShadowRoot-interface-expected.txt

@@ -10 +10 @@
 PASS ShadowRoot.innerHTML must replace all with the result of invoking the fragment parsing algorithm when shadow root is open. 
 PASS ShadowRoot.innerHTML must replace all with the result of invoking the fragment parsing algorithm when shadow root is closed. 
-FAIL ShadowRoot.styleSheets must return a StyleSheetList sequence containing the shadow root style sheets when shadow root is open. undefined is not an object (evaluating 'shadowRoot.styleSheets.length')
-FAIL ShadowRoot.styleSheets must return a StyleSheetList sequence containing the shadow root style sheets when shadow root is closed. undefined is not an object (evaluating 'shadowRoot.styleSheets.length')
+PASS ShadowRoot.styleSheets must return a StyleSheetList sequence containing the shadow root style sheets when shadow root is open. 
+PASS ShadowRoot.styleSheets must return a StyleSheetList sequence containing the shadow root style sheets when shadow root is closed. 
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        ShadowRoot should have styleSheets property
+        https://bugs.webkit.org/show_bug.cgi?id=191311
+
+        Reviewed by Antti Koivisto.
+
+        Added the support for ShadowRoot.prototype.styleSheets by making StyleSheetList refer to either
+        a document or a shadow root. We don't support the named getter in shadow root since that's not
+        a standard feature: https://drafts.csswg.org/cssom/#the-stylesheetlist-interface
+
+        Tests: fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html
+               imported/w3c/web-platform-tests/shadow-dom/ShadowRoot-interface.html
+
+        * css/StyleSheetList.cpp:
+        (WebCore::StyleSheetList::StyleSheetList): Added a variant which takes ShadowRoot.
+        (WebCore::StyleSheetList::styleSheets const):
+        (WebCore::StyleSheetList::ownerNode): Added. The replacement for document() since now the opaque
+        root could be either a Document or a ShadowRoot.
+        (WebCore::StyleSheetList::detach): Renamed from detachFromDocument.
+        (WebCore::StyleSheetList::namedItem const): Added a comment that the named getter is only supported
+        for Document since it's not in the standard.
+        * css/StyleSheetList.h:
+        (WebCore::StyleSheetList::create): Added a variant which takes ShadowRoot.
+        (WebCore::StyleSheetList::document): Deleted. Replaced by StyleSheetList::ownerNode in .cpp file.
+        * css/StyleSheetList.idl:
+        * dom/Document.cpp:
+        (WebCore::Document::~Document):
+        (WebCore::Document::styleSheets):
+        * dom/Document.idl:
+        * dom/DocumentOrShadowRoot.idl: Moved the declaration of styleSheets here from Document.idl.
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::~ShadowRoot): Call detach.
+        (WebCore::ShadowRoot::styleSheets):
+        * dom/ShadowRoot.h:
+
 2018-11-30  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/css/StyleSheetList.cpp

@@ -26 +26 @@
 #include "HTMLNames.h"
 #include "HTMLStyleElement.h"
+#include "ShadowRoot.h"
 #include "StyleScope.h"
 #include <wtf/text/WTFString.h>
@@ -33 +34 @@
 using namespace HTMLNames;
 
-StyleSheetList::StyleSheetList(Document* document)
-    : m_document(document)
+StyleSheetList::StyleSheetList(Document& document)
+    : m_document(&document)
+{
+}
+
+StyleSheetList::StyleSheetList(ShadowRoot& shadowRoot)
+    : m_shadowRoot(&shadowRoot)
 {
 }
@@ -42 +48 @@
 inline const Vector<RefPtr<StyleSheet>>& StyleSheetList::styleSheets() const
 {
-    if (!m_document)
-        return m_detachedStyleSheets;
-    return m_document->styleScope().styleSheetsForStyleSheetList();
+    if (m_document)
+        return m_document->styleScope().styleSheetsForStyleSheetList();
+    if (m_shadowRoot)
+        return m_shadowRoot->styleScope().styleSheetsForStyleSheetList();
+    return m_detachedStyleSheets;
 }
 
-void StyleSheetList::detachFromDocument()
+Node* StyleSheetList::ownerNode() const
 {
-    m_detachedStyleSheets = m_document->styleScope().styleSheetsForStyleSheetList();
-    m_document = nullptr;
+    if (m_document)
+        return m_document;
+    return m_shadowRoot;
+}
+
+void StyleSheetList::detach()
+{
+    if (m_document) {
+        ASSERT(!m_shadowRoot);
+        m_detachedStyleSheets = m_document->styleScope().styleSheetsForStyleSheetList();
+        m_document = nullptr;
+    } else if (m_shadowRoot) {
+        ASSERT(!m_document);
+        m_detachedStyleSheets = m_shadowRoot->styleScope().styleSheetsForStyleSheetList();
+        m_shadowRoot = nullptr;
+    } else
+        ASSERT_NOT_REACHED();
 }
 
@@ -66 +89 @@
 CSSStyleSheet* StyleSheetList::namedItem(const AtomicString& name) const
 {
+    // Support the named getter on document for backwards compatibility.
     if (!m_document)
         return nullptr;

trunk/Source/WebCore/css/StyleSheetList.h

@@ -29 +29 @@
 class Document;
 class HTMLStyleElement;
+class Node;
+class ShadowRoot;
 class StyleSheet;
 class CSSStyleSheet;
@@ -34 +36 @@
 class StyleSheetList final : public RefCounted<StyleSheetList> {
 public:
-    static Ref<StyleSheetList> create(Document* document) { return adoptRef(*new StyleSheetList(document)); }
+    static Ref<StyleSheetList> create(Document& document) { return adoptRef(*new StyleSheetList(document)); }
+    static Ref<StyleSheetList> create(ShadowRoot& shadowRoot) { return adoptRef(*new StyleSheetList(shadowRoot)); }
     WEBCORE_EXPORT ~StyleSheetList();
 
@@ -43 +46 @@
     Vector<AtomicString> supportedPropertyNames();
 
-    Document* document() { return m_document; }
+    Node* ownerNode() const;
 
-    void detachFromDocument();
+    void detach();
 
 private:
-    StyleSheetList(Document*);
+    StyleSheetList(Document&);
+    StyleSheetList(ShadowRoot&);
     const Vector<RefPtr<StyleSheet>>& styleSheets() const;
 
-    Document* m_document;
+    Document* m_document { nullptr };
+    ShadowRoot* m_shadowRoot { nullptr };
     Vector<RefPtr<StyleSheet>> m_detachedStyleSheets;
 };

trunk/Source/WebCore/css/StyleSheetList.idl

@@ -21 +21 @@
 [
     ExportToWrappedFunction,
-    GenerateIsReachable=ImplDocument,
+    GenerateIsReachable=ImplOwnerNodeRoot,
     ImplementationLacksVTable,
 ] interface StyleSheetList {

trunk/Source/WebCore/dom/Document.cpp

@@ -638 +638 @@
 
     if (m_styleSheetList)
-        m_styleSheetList->detachFromDocument();
+        m_styleSheetList->detach();
 
     extensionStyleSheets().detachFromDocument();
@@ -3831 +3831 @@
 {
     if (!m_styleSheetList)
-        m_styleSheetList = StyleSheetList::create(this);
+        m_styleSheetList = StyleSheetList::create(*this);
     return *m_styleSheetList;
 }

trunk/Source/WebCore/dom/Document.idl

@@ -131 +131 @@
     [LenientThis] attribute EventHandler onreadystatechange;
 
-    // Extensions from the CSSOM specification (https://drafts.csswg.org/cssom/#extensions-to-the-document-interface).
-    // FIXME: Should likely be moved to DocumentOrShadowRoot.
-    readonly attribute StyleSheetList styleSheets; // FIXME: Should be [SameObject].
-
     // Extensions from the CSSOM-View specification (https://drafts.csswg.org/cssom-view/#extensions-to-the-document-interface).
     [ImplementedAs=scrollingElementForAPI] readonly attribute Element? scrollingElement;

trunk/Source/WebCore/dom/DocumentOrShadowRoot.idl

@@ -35 +35 @@
     // CaretPosition? caretPositionFromPoint(double x, double y); // FIXME: Implement this.
     readonly attribute Element? activeElement;
-    // readonly attribute StyleSheetList styleSheets; // FIXME: Implement this.
+
+    // Extensions from the CSSOM specification (https://drafts.csswg.org/cssom/#extensions-to-the-document-interface).
+    readonly attribute StyleSheetList styleSheets; // FIXME: Should be [SameObject].
 
     // Extensions from Pointer Lock API (https://w3c.github.io/pointerlock/#extensions-to-the-documentorshadowroot-mixin).

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -37 +37 @@
 #include "StyleResolver.h"
 #include "StyleScope.h"
+#include "StyleSheetList.h"
 #include "markup.h"
 #include <wtf/IsoMallocInlines.h>
@@ -47 +48 @@
     unsigned countersAndFlags[1];
     void* styleScope;
+    void* styleSheetList;
     void* host;
     void* slotAssignment;
@@ -76 +78 @@
     if (isConnected())
         document().didRemoveInDocumentShadowRoot(*this);
+
+    if (m_styleSheetList)
+        m_styleSheetList->detach();
 
     // We cannot let ContainerNode destructor call willBeDeletedFrom()
@@ -152 +157 @@
 }
 
+StyleSheetList& ShadowRoot::styleSheets()
+{
+    if (!m_styleSheetList)
+        m_styleSheetList = StyleSheetList::create(*this);
+    return *m_styleSheetList;
+}
+
 String ShadowRoot::innerHTML() const
 {

trunk/Source/WebCore/dom/ShadowRoot.h

@@ -36 +36 @@
 class HTMLSlotElement;
 class SlotAssignment;
+class StyleSheetList;
 
 class ShadowRoot final : public DocumentFragment, public TreeScope {
@@ -55 +56 @@
 
     Style::Scope& styleScope();
+    StyleSheetList& styleSheets();
 
     bool resetStyleInheritance() const { return m_resetStyleInheritance; }
@@ -111 +113 @@
 
     Element* m_host { nullptr };
+    RefPtr<StyleSheetList> m_styleSheetList;
 
     std::unique_ptr<Style::Scope> m_styleScope;