ShadowRoot should have styleSheets property
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2018 23:33:39 +0000 (23:33 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2018 23:33:39 +0000 (23:33 +0000)
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.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc-expected.txt [new file with mode: 0644]
LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html [new file with mode: 0644]
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/cssom/selectorText-modification-restyle-002-expected.txt
LayoutTests/imported/w3c/web-platform-tests/shadow-dom/ShadowRoot-interface-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/StyleSheetList.cpp
Source/WebCore/css/StyleSheetList.h
Source/WebCore/css/StyleSheetList.idl
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.idl
Source/WebCore/dom/DocumentOrShadowRoot.idl
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/ShadowRoot.h

index a34df2f..7a5e9cf 100644 (file)
@@ -1,3 +1,17 @@
+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>
 
         Replace "auto fill" with "AutoFill" in some localizable strings
diff --git a/LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc-expected.txt b/LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc-expected.txt
new file mode 100644 (file)
index 0000000..93ad7ed
--- /dev/null
@@ -0,0 +1,13 @@
+This tests that StyleSheetList of a shadow root does not get collected as long as the shadow root is alive.
+
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+
diff --git a/LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html b/LayoutTests/fast/shadow-dom/shadowroot-stylesheets-wrapper-gc.html
new file mode 100644 (file)
index 0000000..c94c424
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/gc.js"></script>
+<p>This tests that StyleSheetList of a shadow root does not get collected as long as the shadow root is alive.</p>
+<pre><script>
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function createShadow() {
+    const host = document.createElement('div');
+    const shadowRoot = host.attachShadow({mode: 'closed'});
+    shadowRoot.styleSheets.alive = true;
+    return shadowRoot;
+}
+
+for (let i = 0; i < 10; i++) {
+    const shadowRoot = createShadow();
+    gc();
+    document.write(shadowRoot.styleSheets.alive ? 'PASS' : 'FAIL - styleSheets got collected');
+    document.write('<br>');
+}
+
+</script></pre>
+</body>
+</html>
index 0b06b28..af0905f 100644 (file)
@@ -1,3 +1,18 @@
+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>
 
         Update web-platform-tests/shadow-dom
index dd518e8..a18762e 100644 (file)
@@ -1,3 +1,3 @@
 
-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
 
index a3c7a3e..68cd8d3 100644 (file)
@@ -1,4 +1,4 @@
 
 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. 
 
index 5a53ec7..f9c665f 100644 (file)
@@ -9,6 +9,6 @@ PASS ShadowRoot.innerHTML must return the result of the HTML fragment serializat
 PASS ShadowRoot.innerHTML must return the result of the HTML fragment serialization algorithm when shadow root is closed. 
 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. 
 
index 36d909e..67cd96b 100644 (file)
@@ -1,3 +1,39 @@
+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>
 
         [PSON] We are sometimes swapping processes even though there is an opened window with an opener link to us
index dc792d5..f211dac 100644 (file)
@@ -25,6 +25,7 @@
 #include "Document.h"
 #include "HTMLNames.h"
 #include "HTMLStyleElement.h"
+#include "ShadowRoot.h"
 #include "StyleScope.h"
 #include <wtf/text/WTFString.h>
 
@@ -32,8 +33,13 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-StyleSheetList::StyleSheetList(Document* document)
-    : m_document(document)
+StyleSheetList::StyleSheetList(Document& document)
+    : m_document(&document)
+{
+}
+
+StyleSheetList::StyleSheetList(ShadowRoot& shadowRoot)
+    : m_shadowRoot(&shadowRoot)
 {
 }
 
@@ -41,15 +47,32 @@ StyleSheetList::~StyleSheetList() = default;
 
 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;
+}
+
+Node* StyleSheetList::ownerNode() const
+{
+    if (m_document)
+        return m_document;
+    return m_shadowRoot;
 }
 
-void StyleSheetList::detachFromDocument()
+void StyleSheetList::detach()
 {
-    m_detachedStyleSheets = m_document->styleScope().styleSheetsForStyleSheetList();
-    m_document = nullptr;
+    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();
 }
 
 unsigned StyleSheetList::length() const
@@ -65,6 +88,7 @@ StyleSheet* StyleSheetList::item(unsigned index)
 
 CSSStyleSheet* StyleSheetList::namedItem(const AtomicString& name) const
 {
+    // Support the named getter on document for backwards compatibility.
     if (!m_document)
         return nullptr;
 
index 341adff..8a8fd62 100644 (file)
@@ -28,12 +28,15 @@ namespace WebCore {
 
 class Document;
 class HTMLStyleElement;
+class Node;
+class ShadowRoot;
 class StyleSheet;
 class CSSStyleSheet;
 
 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();
 
     WEBCORE_EXPORT unsigned length() const;
@@ -42,15 +45,17 @@ public:
     CSSStyleSheet* namedItem(const AtomicString&) const;
     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;
 };
 
index af128a9..1139d31 100644 (file)
@@ -20,7 +20,7 @@
 
 [
     ExportToWrappedFunction,
-    GenerateIsReachable=ImplDocument,
+    GenerateIsReachable=ImplOwnerNodeRoot,
     ImplementationLacksVTable,
 ] interface StyleSheetList {
     readonly attribute unsigned long length;
index 92877e4..7f37235 100644 (file)
@@ -637,7 +637,7 @@ Document::~Document()
     m_decoder = nullptr;
 
     if (m_styleSheetList)
-        m_styleSheetList->detachFromDocument();
+        m_styleSheetList->detach();
 
     extensionStyleSheets().detachFromDocument();
 
@@ -3830,7 +3830,7 @@ void Document::cloneDataFromDocument(const Document& other)
 StyleSheetList& Document::styleSheets()
 {
     if (!m_styleSheetList)
-        m_styleSheetList = StyleSheetList::create(this);
+        m_styleSheetList = StyleSheetList::create(*this);
     return *m_styleSheetList;
 }
 
index 901c0c8..1b68f24 100644 (file)
@@ -130,10 +130,6 @@ typedef (
     // Special event handler IDL attributes that only apply to Document objects.
     [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;
 
index 9a6bcd0..6387665 100644 (file)
@@ -34,7 +34,9 @@
     sequence<Element> elementsFromPoint(double x, double y);
     // 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).
     [Conditional=POINTER_LOCK] readonly attribute Element? pointerLockElement;
index e44e6b2..6e8851e 100644 (file)
@@ -36,6 +36,7 @@
 #include "SlotAssignment.h"
 #include "StyleResolver.h"
 #include "StyleScope.h"
+#include "StyleSheetList.h"
 #include "markup.h"
 #include <wtf/IsoMallocInlines.h>
 
@@ -46,6 +47,7 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(ShadowRoot);
 struct SameSizeAsShadowRoot : public DocumentFragment, public TreeScope {
     unsigned countersAndFlags[1];
     void* styleScope;
+    void* styleSheetList;
     void* host;
     void* slotAssignment;
 };
@@ -76,6 +78,9 @@ ShadowRoot::~ShadowRoot()
     if (isConnected())
         document().didRemoveInDocumentShadowRoot(*this);
 
+    if (m_styleSheetList)
+        m_styleSheetList->detach();
+
     // We cannot let ContainerNode destructor call willBeDeletedFrom()
     // for this ShadowRoot instance because TreeScope destructor
     // clears Node::m_treeScope thus ContainerNode is no longer able
@@ -151,6 +156,13 @@ Style::Scope& ShadowRoot::styleScope()
     return *m_styleScope;
 }
 
+StyleSheetList& ShadowRoot::styleSheets()
+{
+    if (!m_styleSheetList)
+        m_styleSheetList = StyleSheetList::create(*this);
+    return *m_styleSheetList;
+}
+
 String ShadowRoot::innerHTML() const
 {
     return serializeFragment(*this, SerializedNodes::SubtreesOfChildren);
index 5bd2372..36ef614 100644 (file)
@@ -35,6 +35,7 @@ namespace WebCore {
 
 class HTMLSlotElement;
 class SlotAssignment;
+class StyleSheetList;
 
 class ShadowRoot final : public DocumentFragment, public TreeScope {
     WTF_MAKE_ISO_ALLOCATED(ShadowRoot);
@@ -54,6 +55,7 @@ public:
     using TreeScope::rootNode;
 
     Style::Scope& styleScope();
+    StyleSheetList& styleSheets();
 
     bool resetStyleInheritance() const { return m_resetStyleInheritance; }
     void setResetStyleInheritance(bool);
@@ -110,6 +112,7 @@ private:
     ShadowRootMode m_type { ShadowRootMode::UserAgent };
 
     Element* m_host { nullptr };
+    RefPtr<StyleSheetList> m_styleSheetList;
 
     std::unique_ptr<Style::Scope> m_styleScope;
     std::unique_ptr<SlotAssignment> m_slotAssignment;