Changeset 208356 in webkit

Timestamp:

Nov 3, 2016 4:18:01 PM (8 years ago)

Author:

Ryan Haddad

Message:

Unreviewed, rolling out r208302.

This change causes LayoutTest crashes under GuardMalloc.

Reverted changeset:

"Load stylesheets in link elements inside a connected shadow
tree"
​https://bugs.webkit.org/show_bug.cgi?id=160683
​http://trac.webkit.org/changeset/208302

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/shadow-dom/link-element-in-shadow-tree-expected.txt (deleted)
• LayoutTests/fast/shadow-dom/link-element-in-shadow-tree.html (deleted)
• LayoutTests/fast/shadow-dom/resources/green-host.css (deleted)
• LayoutTests/fast/shadow-dom/selected-stylesheet-in-shadow-tree-expected.txt (deleted)
• LayoutTests/fast/shadow-dom/selected-stylesheet-in-shadow-tree.html (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (2 diffs)
• Source/WebCore/dom/Element.cpp (modified) (1 diff)
• Source/WebCore/dom/ShadowRoot.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLLinkElement.cpp (modified) (11 diffs)
• Source/WebCore/html/HTMLLinkElement.h (modified) (2 diffs)
• Source/WebCore/html/HTMLStyleElement.cpp (modified) (2 diffs)
• Source/WebCore/svg/SVGStyleElement.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-11-03  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r208302.
+
+        This change causes LayoutTest crashes under GuardMalloc.
+
+        Reverted changeset:
+
+        "Load stylesheets in link elements inside a connected shadow
+        tree"
+        https://bugs.webkit.org/show_bug.cgi?id=160683
+        http://trac.webkit.org/changeset/208302
+
 2016-11-03  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-11-03  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r208302.
+
+        This change causes LayoutTest crashes under GuardMalloc.
+
+        Reverted changeset:
+
+        "Load stylesheets in link elements inside a connected shadow
+        tree"
+        https://bugs.webkit.org/show_bug.cgi?id=160683
+        http://trac.webkit.org/changeset/208302
+
 2016-11-03  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -6981 +6981 @@
 {
     ASSERT(shadowRoot.inDocument());
-    ASSERT(!m_inDocumentShadowRoots.contains(&shadowRoot));
     m_inDocumentShadowRoots.add(&shadowRoot);
 }
@@ -6987 +6986 @@
 void Document::didRemoveInDocumentShadowRoot(ShadowRoot& shadowRoot)
 {
-    ASSERT(!shadowRoot.inDocument());
     ASSERT(m_inDocumentShadowRoots.contains(&shadowRoot));
     m_inDocumentShadowRoots.remove(&shadowRoot);

trunk/Source/WebCore/dom/Element.cpp

@@ -1785 +1785 @@
     oldRoot->setHost(nullptr);
     oldRoot->setParentTreeScope(&document());
+
+    notifyChildNodeRemoved(*this, *oldRoot);
 }
 

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -85 +85 @@
 Node::InsertionNotificationRequest ShadowRoot::insertedInto(ContainerNode& insertionPoint)
 {
-    bool wasInDocument = inDocument();
-    DocumentFragment::insertedInto(insertionPoint);
-    if (insertionPoint.inDocument() && !wasInDocument)
+    auto result = DocumentFragment::insertedInto(insertionPoint);
+    if (inDocument())
         document().didInsertInDocumentShadowRoot(*this);
-    return InsertionDone;
+    return result;
 }
 
 void ShadowRoot::removedFrom(ContainerNode& insertionPoint)
 {
+    if (inDocument())
+        document().didRemoveInDocumentShadowRoot(*this);
     DocumentFragment::removedFrom(insertionPoint);
-    if (insertionPoint.inDocument() && !inDocument())
-        document().didRemoveInDocumentShadowRoot(*this);
 }
 

trunk/Source/WebCore/html/HTMLLinkElement.cpp

@@ -81 +81 @@
     , m_loading(false)
     , m_createdByParser(createdByParser)
+    , m_isInShadowTree(false)
     , m_firedLoad(false)
     , m_loadedResource(false)
@@ -101 +102 @@
         m_cachedSheet->removeClient(*this);
 
-    if (m_styleScope)
-        m_styleScope->removeStyleSheetCandidateNode(*this);
+    if (inDocument())
+        document().styleScope().removeStyleSheetCandidateNode(*this);
 
     linkLoadEventSender().cancelEvent(*this);
@@ -112 +113 @@
     DisabledState oldDisabledState = m_disabledState;
     m_disabledState = disabled ? Disabled : EnabledViaScript;
-    if (oldDisabledState == m_disabledState)
-        return;
-
-    ASSERT(inDocument() || !styleSheetIsLoading());
-    if (!inDocument())
-        return;
-
-    // If we change the disabled state while the sheet is still loading, then we have to
-    // perform three checks:
-    if (styleSheetIsLoading()) {
-        // Check #1: The sheet becomes disabled while loading.
-        if (m_disabledState == Disabled)
-            removePendingSheet();
-
-        // Check #2: An alternate sheet becomes enabled while it is still loading.
-        if (m_relAttribute.isAlternate && m_disabledState == EnabledViaScript)
-            addPendingSheet(ActiveSheet);
-
-        // Check #3: A main sheet becomes enabled while it was still loading and
-        // after it was disabled via script. It takes really terrible code to make this
-        // happen (a double toggle for no reason essentially). This happens on
-        // virtualplastic.net, which manages to do about 12 enable/disables on only 3
-        // sheets. :)
-        if (!m_relAttribute.isAlternate && m_disabledState == EnabledViaScript && oldDisabledState == Disabled)
-            addPendingSheet(ActiveSheet);
-
-        // If the sheet is already loading just bail.
-        return;
-    }
-
-    // Load the sheet, since it's never been loaded before.
-    if (!m_sheet && m_disabledState == EnabledViaScript)
-        process();
-    else {
-        ASSERT(m_styleScope);
-        m_styleScope->didChangeActiveStyleSheetCandidates();
+    if (oldDisabledState != m_disabledState) {
+        // If we change the disabled state while the sheet is still loading, then we have to
+        // perform three checks:
+        if (styleSheetIsLoading()) {
+            // Check #1: The sheet becomes disabled while loading.
+            if (m_disabledState == Disabled)
+                removePendingSheet();
+
+            // Check #2: An alternate sheet becomes enabled while it is still loading.
+            if (m_relAttribute.isAlternate && m_disabledState == EnabledViaScript)
+                addPendingSheet(ActiveSheet);
+
+            // Check #3: A main sheet becomes enabled while it was still loading and
+            // after it was disabled via script. It takes really terrible code to make this
+            // happen (a double toggle for no reason essentially). This happens on
+            // virtualplastic.net, which manages to do about 12 enable/disables on only 3
+            // sheets. :)
+            if (!m_relAttribute.isAlternate && m_disabledState == EnabledViaScript && oldDisabledState == Disabled)
+                addPendingSheet(ActiveSheet);
+
+            // If the sheet is already loading just bail.
+            return;
+        }
+
+        // Load the sheet, since it's never been loaded before.
+        if (!m_sheet && m_disabledState == EnabledViaScript)
+            process();
+        else
+            document().styleScope().didChangeActiveStyleSheetCandidates();
     }
 }
@@ -183 +177 @@
         process();
         if (m_sheet && !isDisabled())
-            m_styleScope->didChangeActiveStyleSheetCandidates();
+            document().styleScope().didChangeActiveStyleSheetCandidates();
         return;
     }
@@ -221 +215 @@
 void HTMLLinkElement::process()
 {
-    if (!inDocument()) {
+    if (!inDocument() || m_isInShadowTree) {
         ASSERT(!m_sheet);
         return;
@@ -290 +284 @@
         // we no longer contain a stylesheet, e.g. perhaps rel or type was changed
         clearSheet();
-        m_styleScope->didChangeActiveStyleSheetCandidates();
+        document().styleScope().didChangeActiveStyleSheetCandidates();
     }
 }
@@ -304 +298 @@
 Node::InsertionNotificationRequest HTMLLinkElement::insertedInto(ContainerNode& insertionPoint)
 {
-    bool wasInDocument = inDocument();
     HTMLElement::insertedInto(insertionPoint);
-    if (!insertionPoint.inDocument() || wasInDocument)
+    if (!insertionPoint.inDocument())
         return InsertionDone;
 
-    m_styleScope = &Style::Scope::forNode(*this);
-    m_styleScope->addStyleSheetCandidateNode(*this, m_createdByParser);
+    m_isInShadowTree = isInShadowTree();
+    if (m_isInShadowTree)
+        return InsertionDone;
+
+    document().styleScope().addStyleSheetCandidateNode(*this, m_createdByParser);
 
     process();
@@ -319 +315 @@
 {
     HTMLElement::removedFrom(insertionPoint);
-    if (!insertionPoint.inDocument() || inDocument())
-        return;
+    if (!insertionPoint.inDocument())
+        return;
+
+    if (m_isInShadowTree) {
+        ASSERT(!m_sheet);
+        return;
+    }
+    document().styleScope().removeStyleSheetCandidateNode(*this);
 
     if (m_sheet)
@@ -327 +329 @@
     if (styleSheetIsLoading())
         removePendingSheet(RemovePendingSheetNotifyLater);
-    
-    if (m_styleScope) {
-        m_styleScope->removeStyleSheetCandidateNode(*this);
-        m_styleScope = nullptr;
-    }
-
 }
 
@@ -549 +545 @@
     if (m_pendingSheetType == InactiveSheet)
         return;
-    ASSERT(m_styleScope);
-    m_styleScope->addPendingSheet();
+    document().styleScope().addPendingSheet();
 }
 
@@ -561 +556 @@
         return;
 
-    ASSERT(m_styleScope);
     if (type == InactiveSheet) {
         // Document just needs to know about the sheet for exposure through document.styleSheets
-        m_styleScope->didChangeActiveStyleSheetCandidates();
-        return;
-    }
-
-    m_styleScope->removePendingSheet(
+        document().styleScope().didChangeActiveStyleSheetCandidates();
+        return;
+    }
+
+    document().styleScope().removePendingSheet(
         notification == RemovePendingSheetNotifyImmediately
         ? Style::Scope::RemovePendingSheetNotifyImmediately

trunk/Source/WebCore/html/HTMLLinkElement.h

@@ -120 +120 @@
 
     LinkLoader m_linkLoader;
-    Style::Scope* m_styleScope { nullptr };
     CachedResourceHandle<CachedCSSStyleSheet> m_cachedSheet;
     RefPtr<CSSStyleSheet> m_sheet;
@@ -136 +135 @@
     bool m_loading;
     bool m_createdByParser;
+    bool m_isInShadowTree;
     bool m_firedLoad;
     bool m_loadedResource;

trunk/Source/WebCore/html/HTMLStyleElement.cpp

@@ -95 +95 @@
 Node::InsertionNotificationRequest HTMLStyleElement::insertedInto(ContainerNode& insertionPoint)
 {
-    bool wasInDocument = inDocument();
-    auto result = HTMLElement::insertedInto(insertionPoint);
-    if (insertionPoint.inDocument() && !wasInDocument)
+    HTMLElement::insertedInto(insertionPoint);
+    if (insertionPoint.inDocument())
         m_styleSheetOwner.insertedIntoDocument(*this);
-    return result;
+
+    return InsertionDone;
 }
 
@@ -105 +105 @@
 {
     HTMLElement::removedFrom(insertionPoint);
-    if (insertionPoint.inDocument() && !inDocument())
+
+    if (insertionPoint.inDocument())
         m_styleSheetOwner.removedFromDocument(*this);
 }

trunk/Source/WebCore/svg/SVGStyleElement.cpp

@@ -117 +117 @@
 Node::InsertionNotificationRequest SVGStyleElement::insertedInto(ContainerNode& rootParent)
 {
-    bool wasInDocument = inDocument();
-    auto result = SVGElement::insertedInto(rootParent);
-    if (rootParent.inDocument() && !wasInDocument)
+    SVGElement::insertedInto(rootParent);
+    if (rootParent.inDocument())
         m_styleSheetOwner.insertedIntoDocument(*this);
-    return result;
+    return InsertionDone;
 }
 
@@ -127 +126 @@
 {
     SVGElement::removedFrom(rootParent);
-    if (rootParent.inDocument() && !inDocument())
+    if (rootParent.inDocument())
         m_styleSheetOwner.removedFromDocument(*this);
 }