Eliminate isMainThread() checks in most call sites of NoEventDispatchAssertion
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Nov 2017 21:56:30 +0000 (21:56 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Nov 2017 21:56:30 +0000 (21:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179161

Reviewed by Zalan Bujtas.

Introduced NoEventDispatchAssertion::InMainThread which bypasses the expensive isMainThread() check
in order to turn NoEventDispatchAssertion into a release assertion in a separate patch.

Also removed instances of NoEventDispatchAssertion in notifyChildNodeInserted and notifyChildNodeRemoved
and asserted that the caller has instantiated NoEventDispatchAssertion instead.

No new tests since there should be no behavioral changes.

* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::canExecuteScripts):
* dom/Attr.cpp:
* dom/CharacterData.cpp:
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeAllChildrenWithScriptAssertion):
(WebCore::ContainerNode::removeNodeWithScriptAssertion):
(WebCore::executeNodeInsertionWithScriptAssertion):
(WebCore::ContainerNode::removeDetachedChildren): Instantiated NoEventDispatchAssertion::InMainThread
so that notifyChildNodeRemoved would be called inside NoEventDispatchAssertion.
(WebCore::ContainerNode::insertBeforeCommon):
(WebCore::ContainerNode::appendChildCommon):
(WebCore::ContainerNode::removeBetween):
(WebCore::dispatchChildInsertionEvents):
(WebCore::dispatchChildRemovalEvents):
* dom/ContainerNodeAlgorithms.cpp:
(WebCore::notifyChildNodeInserted): Assert that the caller has instantiated NoEventDispatchAssertion.
(WebCore::notifyChildNodeRemoved): Ditto.
* dom/Document.cpp:
(WebCore::Document::resolveStyle):
(WebCore::Document::updateStyleIfNeeded):
(WebCore::Document::nodeChildrenWillBeRemoved):
(WebCore::Document::nodeWillBeRemoved):
(WebCore::Document::dispatchWindowEvent): Replaced RELEASE_ASSERT with ASSERT_WITH_SECURITY_IMPLICATION
for clarity since NoEventDispatchAssertion::isEventAllowedInMainThread() always returns true in release
builds right now.
(WebCore::Document::dispatchWindowLoadEvent): Ditto.
(WebCore::Document::applyPendingXSLTransformsTimerFired): Use ASSERT_WITH_SECURITY_IMPLICATION instead
of regular ASSERT.
* dom/Element.cpp:
(WebCore::Element::addShadowRoot): Instantiate NoEventDispatchAssertion::InMainThread to call
notifyChildNodeInserted will it.
(WebCore::Element::attachAttributeNodeIfNeeded):
(WebCore::Element::setAttributeNode): Fixed the indentation.
(WebCore::Element::setAttributeNodeNS): Ditto.
(WebCore::Element::dispatchFocusInEvent):
(WebCore::Element::dispatchFocusOutEvent):
* dom/EventDispatcher.cpp:
(WebCore::EventDispatcher::dispatchEvent):
* dom/NoEventDispatchAssertion.h:
(WebCore::NoEventDispatchAssertion::isEventDispatchAllowedInSubtree): Moved to InMainThread.
(WebCore::NoEventDispatchAssertion::InMainThread): Added.
(WebCore::NoEventDispatchAssertion::InMainThread::InMainThread): Assert that we're in the main thread
instead of exiting early.
(WebCore::NoEventDispatchAssertion::InMainThread::~InMainThread): Ditto.
(WebCore::NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree): Moved here.
(WebCore::NoEventDispatchAssertion::InMainThread::isEventAllowed):
* dom/Node.cpp:
(WebCore::Node::dispatchSubtreeModifiedEvent):
(WebCore::Node::dispatchDOMActivateEvent):
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension):
(WebCore::ScriptExecutionContext::suspendActiveDOMObjects):
(WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
* history/CachedPage.cpp:
(WebCore::CachedPage::restore):
* history/PageCache.cpp:
(WebCore::PageCache::addIfCacheable):
* page/LayoutContext.cpp:
(WebCore::LayoutContext::layout):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/ScriptController.cpp
Source/WebCore/dom/Attr.cpp
Source/WebCore/dom/CharacterData.cpp
Source/WebCore/dom/ContainerNode.cpp
Source/WebCore/dom/ContainerNodeAlgorithms.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/EventDispatcher.cpp
Source/WebCore/dom/NoEventDispatchAssertion.h
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/ScriptExecutionContext.cpp
Source/WebCore/history/CachedPage.cpp
Source/WebCore/history/PageCache.cpp
Source/WebCore/page/LayoutContext.cpp
Source/WebCore/rendering/RenderLayer.cpp

index 683283f..3c5d2f2 100644 (file)
@@ -1,3 +1,81 @@
+2017-11-02  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Eliminate isMainThread() checks in most call sites of NoEventDispatchAssertion
+        https://bugs.webkit.org/show_bug.cgi?id=179161
+
+        Reviewed by Zalan Bujtas.
+
+        Introduced NoEventDispatchAssertion::InMainThread which bypasses the expensive isMainThread() check
+        in order to turn NoEventDispatchAssertion into a release assertion in a separate patch.
+
+        Also removed instances of NoEventDispatchAssertion in notifyChildNodeInserted and notifyChildNodeRemoved
+        and asserted that the caller has instantiated NoEventDispatchAssertion instead.
+
+        No new tests since there should be no behavioral changes.
+
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::canExecuteScripts):
+        * dom/Attr.cpp:
+        * dom/CharacterData.cpp:
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::removeAllChildrenWithScriptAssertion):
+        (WebCore::ContainerNode::removeNodeWithScriptAssertion):
+        (WebCore::executeNodeInsertionWithScriptAssertion):
+        (WebCore::ContainerNode::removeDetachedChildren): Instantiated NoEventDispatchAssertion::InMainThread
+        so that notifyChildNodeRemoved would be called inside NoEventDispatchAssertion. 
+        (WebCore::ContainerNode::insertBeforeCommon):
+        (WebCore::ContainerNode::appendChildCommon):
+        (WebCore::ContainerNode::removeBetween):
+        (WebCore::dispatchChildInsertionEvents):
+        (WebCore::dispatchChildRemovalEvents):
+        * dom/ContainerNodeAlgorithms.cpp:
+        (WebCore::notifyChildNodeInserted): Assert that the caller has instantiated NoEventDispatchAssertion.
+        (WebCore::notifyChildNodeRemoved): Ditto.
+        * dom/Document.cpp:
+        (WebCore::Document::resolveStyle):
+        (WebCore::Document::updateStyleIfNeeded):
+        (WebCore::Document::nodeChildrenWillBeRemoved):
+        (WebCore::Document::nodeWillBeRemoved):
+        (WebCore::Document::dispatchWindowEvent): Replaced RELEASE_ASSERT with ASSERT_WITH_SECURITY_IMPLICATION
+        for clarity since NoEventDispatchAssertion::isEventAllowedInMainThread() always returns true in release
+        builds right now.
+        (WebCore::Document::dispatchWindowLoadEvent): Ditto.
+        (WebCore::Document::applyPendingXSLTransformsTimerFired): Use ASSERT_WITH_SECURITY_IMPLICATION instead
+        of regular ASSERT.
+        * dom/Element.cpp:
+        (WebCore::Element::addShadowRoot): Instantiate NoEventDispatchAssertion::InMainThread to call
+        notifyChildNodeInserted will it.
+        (WebCore::Element::attachAttributeNodeIfNeeded):
+        (WebCore::Element::setAttributeNode): Fixed the indentation.
+        (WebCore::Element::setAttributeNodeNS): Ditto.
+        (WebCore::Element::dispatchFocusInEvent):
+        (WebCore::Element::dispatchFocusOutEvent):
+        * dom/EventDispatcher.cpp:
+        (WebCore::EventDispatcher::dispatchEvent):
+        * dom/NoEventDispatchAssertion.h:
+        (WebCore::NoEventDispatchAssertion::isEventDispatchAllowedInSubtree): Moved to InMainThread.
+        (WebCore::NoEventDispatchAssertion::InMainThread): Added.
+        (WebCore::NoEventDispatchAssertion::InMainThread::InMainThread): Assert that we're in the main thread
+        instead of exiting early.
+        (WebCore::NoEventDispatchAssertion::InMainThread::~InMainThread): Ditto.
+        (WebCore::NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree): Moved here.
+        (WebCore::NoEventDispatchAssertion::InMainThread::isEventAllowed):
+        * dom/Node.cpp:
+        (WebCore::Node::dispatchSubtreeModifiedEvent):
+        (WebCore::Node::dispatchDOMActivateEvent):
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension):
+        (WebCore::ScriptExecutionContext::suspendActiveDOMObjects):
+        (WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
+        * history/CachedPage.cpp:
+        (WebCore::CachedPage::restore):
+        * history/PageCache.cpp:
+        (WebCore::PageCache::addIfCacheable):
+        * page/LayoutContext.cpp:
+        (WebCore::LayoutContext::layout):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollRectToVisible):
+
 2017-11-02  John Wilander  <wilander@apple.com>
 
         Ignore HSTS for partitioned, cross-origin subresource requests
index d43a143..5f3f64c 100644 (file)
@@ -678,7 +678,7 @@ JSValue ScriptController::executeScriptInWorld(DOMWrapperWorld& world, const Str
 bool ScriptController::canExecuteScripts(ReasonForCallingCanExecuteScripts reason)
 {
     if (reason == AboutToExecuteScript)
-        ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+        ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
 
     if (m_frame.document() && m_frame.document()->isSandboxed(SandboxScripts)) {
         // FIXME: This message should be moved off the console once a solution to https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
index d0198a4..f7583f5 100644 (file)
@@ -25,7 +25,6 @@
 
 #include "AttributeChangeInvalidation.h"
 #include "Event.h"
-#include "NoEventDispatchAssertion.h"
 #include "ScopedEventQueue.h"
 #include "StyleProperties.h"
 #include "StyledElement.h"
index 01c05f1..e6ee2ac 100644 (file)
@@ -30,7 +30,6 @@
 #include "MutationEvent.h"
 #include "MutationObserverInterestGroup.h"
 #include "MutationRecord.h"
-#include "NoEventDispatchAssertion.h"
 #include "ProcessingInstruction.h"
 #include "RenderText.h"
 #include "StyleInheritedData.h"
index 4f133ae..1903f7a 100644 (file)
@@ -89,7 +89,7 @@ ALWAYS_INLINE NodeVector ContainerNode::removeAllChildrenWithScriptAssertion(Chi
         }
     } else {
         ASSERT(source == ContainerNode::ChildChangeSource::Parser);
-        NoEventDispatchAssertion assertNoEventDispatch;
+        NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
         if (UNLIKELY(document().hasMutationObserversOfType(MutationObserver::ChildList))) {
             ChildListMutationScope mutation(*this);
             for (auto& child : children)
@@ -100,7 +100,7 @@ ALWAYS_INLINE NodeVector ContainerNode::removeAllChildrenWithScriptAssertion(Chi
     disconnectSubframesIfNeeded(*this, DescendantsOnly);
 
     WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     document().nodeChildrenWillBeRemoved(*this);
 
@@ -120,11 +120,11 @@ ALWAYS_INLINE bool ContainerNode::removeNodeWithScriptAssertion(Node& childToRem
     Ref<Node> protectedChildToRemove(childToRemove);
     ASSERT_WITH_SECURITY_IMPLICATION(childToRemove.parentNode() == this);
     {
-        NoEventDispatchAssertion assertNoEventDispatch;
+        NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
         ChildListMutationScope(*this).willRemoveChild(childToRemove);
     }
 
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(childToRemove));
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree(childToRemove));
     if (source == ContainerNode::ChildChangeSource::API) {
         childToRemove.notifyMutationObserversNodeWillDetach();
         dispatchChildRemovalEvents(protectedChildToRemove);
@@ -144,7 +144,7 @@ ALWAYS_INLINE bool ContainerNode::removeNodeWithScriptAssertion(Node& childToRem
         return false;
 
     WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     document().nodeWillBeRemoved(childToRemove);
 
@@ -174,7 +174,7 @@ static ALWAYS_INLINE void executeNodeInsertionWithScriptAssertion(ContainerNode&
 {
     NodeVector postInsertionNotificationTargets;
     {
-        NoEventDispatchAssertion assertNoEventDispatch;
+        NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
         doNodeInsertion();
         ChildListMutationScope(containerNode).childAdded(child);
         postInsertionNotificationTargets = notifyChildNodeInserted(containerNode, child);
@@ -192,7 +192,7 @@ static ALWAYS_INLINE void executeNodeInsertionWithScriptAssertion(ContainerNode&
         });
     }
 
-    ASSERT(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(child));
+    ASSERT(NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree(child));
     for (auto& target : postInsertionNotificationTargets)
         target->didFinishInsertingNode();
 
@@ -225,6 +225,7 @@ void ContainerNode::removeDetachedChildren()
             child->updateAncestorConnectedSubframeCountForRemoval();
     }
     // FIXME: We should be able to ASSERT(!attached()) here: https://bugs.webkit.org/show_bug.cgi?id=107801
+    NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
     removeDetachedChildrenInContainer(*this);
 }
 
@@ -399,7 +400,7 @@ ExceptionOr<void> ContainerNode::insertBefore(Node& newChild, Node* refChild)
 
 void ContainerNode::insertBeforeCommon(Node& nextChild, Node& newChild)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     ASSERT(!newChild.parentNode()); // Use insertBefore if you need to handle reparenting (and want DOM mutation events).
     ASSERT(!newChild.nextSibling());
@@ -424,7 +425,7 @@ void ContainerNode::insertBeforeCommon(Node& nextChild, Node& newChild)
 
 void ContainerNode::appendChildCommon(Node& child)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     child.setParentNode(this);
 
@@ -563,7 +564,7 @@ void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node& ol
 {
     InspectorInstrumentation::didRemoveDOMNode(oldChild.document(), oldChild);
 
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     ASSERT(oldChild.parentNode() == this);
 
@@ -762,7 +763,7 @@ static void dispatchChildInsertionEvents(Node& child)
     if (child.isInShadowTree())
         return;
 
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(child));
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree(child));
 
     RefPtr<Node> c = &child;
     Ref<Document> document(child.document());
@@ -779,7 +780,7 @@ static void dispatchChildInsertionEvents(Node& child)
 
 static void dispatchChildRemovalEvents(Ref<Node>& child)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(child));
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree(child));
     InspectorInstrumentation::willRemoveDOMNode(child->document(), child.get());
 
     if (child->isInShadowTree())
index 07d8b46..9614d87 100644 (file)
@@ -84,7 +84,7 @@ static void notifyNodeInsertedIntoTree(ContainerNode& parentOfInsertedTree, Node
 
 NodeVector notifyChildNodeInserted(ContainerNode& parentOfInsertedTree, Node& node)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
+    ASSERT(!NoEventDispatchAssertion::InMainThread::isEventAllowed());
 
     InspectorInstrumentation::didInsertDOMNode(node.document(), node);
 
@@ -147,7 +147,8 @@ static void notifyNodeRemovedFromTree(ContainerNode& oldParentOfRemovedTree, Tre
 
 void notifyChildNodeRemoved(ContainerNode& oldParentOfRemovedTree, Node& child)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
+    // Assert that the caller of this function has an instance of NoEventDispatchAssertion.
+    ASSERT(!isMainThread() || !NoEventDispatchAssertion::InMainThread::isEventAllowed());
 
     // Tree scope has changed if the container node from which "node" is removed is in a document or a shadow root.
     auto treeScopeChange = oldParentOfRemovedTree.isInTreeScope() ? TreeScopeChange::Changed : TreeScopeChange::DidNotChange;
index b2c94a5..f19462e 100644 (file)
@@ -1793,7 +1793,7 @@ void Document::resolveStyle(ResolveStyleType type)
     // hits a null-dereference due to security code always assuming the document has a SecurityOrigin.
 
     {
-        NoEventDispatchAssertion noEventDispatchAssertion;
+        NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
         styleScope().flushPendingUpdate();
         frameView.willRecalcStyle();
     }
@@ -1804,7 +1804,7 @@ void Document::resolveStyle(ResolveStyleType type)
     {
         Style::PostResolutionCallbackDisabler disabler(*this);
         WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-        NoEventDispatchAssertion noEventDispatchAssertion;
+        NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
 
         m_inStyleRecalc = true;
 
@@ -1924,7 +1924,7 @@ bool Document::needsStyleRecalc() const
 bool Document::updateStyleIfNeeded()
 {
     {
-        NoEventDispatchAssertion noEventDispatchAssertion;
+        NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
         ASSERT(isMainThread());
         ASSERT(!view() || !view()->isPainting());
 
@@ -4085,7 +4085,7 @@ void Document::updateRangesAfterChildrenChanged(ContainerNode& container)
 
 void Document::nodeChildrenWillBeRemoved(ContainerNode& container)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
+    ASSERT(!NoEventDispatchAssertion::InMainThread::isEventAllowed());
 
     removeFocusedNodeOfSubtree(container, true /* amongChildrenOnly */);
     removeFocusNavigationNodeOfSubtree(container, true /* amongChildrenOnly */);
@@ -4118,7 +4118,7 @@ void Document::nodeChildrenWillBeRemoved(ContainerNode& container)
 
 void Document::nodeWillBeRemoved(Node& node)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
+    ASSERT(!NoEventDispatchAssertion::InMainThread::isEventAllowed());
 
     removeFocusedNodeOfSubtree(node);
     removeFocusNavigationNodeOfSubtree(node);
@@ -4262,7 +4262,7 @@ EventListener* Document::getWindowAttributeEventListener(const AtomicString& eve
 
 void Document::dispatchWindowEvent(Event& event, EventTarget* target)
 {
-    RELEASE_ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
     if (!m_domWindow)
         return;
     m_domWindow->dispatchEvent(event, target);
@@ -4270,7 +4270,7 @@ void Document::dispatchWindowEvent(Event& event, EventTarget* target)
 
 void Document::dispatchWindowLoadEvent()
 {
-    RELEASE_ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
     if (!m_domWindow)
         return;
     m_domWindow->dispatchLoadEvent();
@@ -5100,7 +5100,7 @@ void Document::applyPendingXSLTransformsTimerFired()
         return;
 
     m_hasPendingXSLTransforms = false;
-    ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
     for (auto& processingInstruction : styleScope().collectXSLTransforms()) {
         ASSERT(processingInstruction->isXSL());
 
index 48d49cc..c897e0e 100644 (file)
@@ -1771,28 +1771,31 @@ void Element::addShadowRoot(Ref<ShadowRoot>&& newShadowRoot)
 {
     ASSERT(!newShadowRoot->hasChildNodes());
     ASSERT(!shadowRoot());
-    
-    if (renderer())
-        RenderTreeUpdater::tearDownRenderers(*this);
 
     ShadowRoot& shadowRoot = newShadowRoot;
-    ensureElementRareData().setShadowRoot(WTFMove(newShadowRoot));
+    {
+        NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
+        if (renderer())
+            RenderTreeUpdater::tearDownRenderers(*this);
 
-    shadowRoot.setHost(this);
-    shadowRoot.setParentTreeScope(treeScope());
+        ensureElementRareData().setShadowRoot(WTFMove(newShadowRoot));
+
+        shadowRoot.setHost(this);
+        shadowRoot.setParentTreeScope(treeScope());
 
 #if !ASSERT_DISABLED
-    ASSERT(notifyChildNodeInserted(*this, shadowRoot).isEmpty());
+        ASSERT(notifyChildNodeInserted(*this, shadowRoot).isEmpty());
 #else
-    notifyChildNodeInserted(*this, shadowRoot);
+        notifyChildNodeInserted(*this, shadowRoot);
 #endif
 
-    invalidateStyleAndRenderersForSubtree();
-
-    InspectorInstrumentation::didPushShadowRoot(*this, shadowRoot);
+        invalidateStyleAndRenderersForSubtree();
+    }
 
     if (shadowRoot.mode() == ShadowRootMode::UserAgent)
         didAddUserAgentShadowRoot(shadowRoot);
+
+    InspectorInstrumentation::didPushShadowRoot(*this, shadowRoot);
 }
 
 void Element::removeShadowRoot()
@@ -2146,7 +2149,7 @@ void Element::attachAttributeNodeIfNeeded(Attr& attrNode)
     if (attrNode.ownerElement() == this)
         return;
 
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     attrNode.attachToElement(*this);
     ensureAttrNodeListForElement(*this).append(&attrNode);
@@ -2164,8 +2167,8 @@ ExceptionOr<RefPtr<Attr>> Element::setAttributeNode(Attr& attrNode)
         return Exception { InUseAttributeError };
 
     {
-    NoEventDispatchAssertion assertNoEventDispatch;
-    synchronizeAllAttributes();
+        NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
+        synchronizeAllAttributes();
     }
 
     auto& elementData = ensureUniqueElementData();
@@ -2210,25 +2213,23 @@ ExceptionOr<RefPtr<Attr>> Element::setAttributeNodeNS(Attr& attrNode)
     if (attrNode.ownerElement() && attrNode.ownerElement() != this)
         return Exception { InUseAttributeError };
 
-    unsigned index = 0;
-    
     // Attr::value() will return its 'm_standaloneValue' member any time its Element is set to nullptr. We need to cache this value
     // before making changes to attrNode's Element connections.
     auto attrNodeValue = attrNode.value();
-
+    unsigned index = 0;
     {
-    NoEventDispatchAssertion assertNoEventDispatch;
-    synchronizeAllAttributes();
-    auto& elementData = ensureUniqueElementData();
+        NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
+        synchronizeAllAttributes();
+        auto& elementData = ensureUniqueElementData();
 
-    index = elementData.findAttributeIndexByName(attrNode.qualifiedName());
+        index = elementData.findAttributeIndexByName(attrNode.qualifiedName());
 
-    if (index != ElementData::attributeNotFound) {
-        if (oldAttrNode)
-            detachAttrNodeFromElementWithValue(oldAttrNode.get(), elementData.attributeAt(index).value());
-        else
-            oldAttrNode = Attr::create(document(), attrNode.qualifiedName(), elementData.attributeAt(index).value());
-    }
+        if (index != ElementData::attributeNotFound) {
+            if (oldAttrNode)
+                detachAttrNodeFromElementWithValue(oldAttrNode.get(), elementData.attributeAt(index).value());
+            else
+                oldAttrNode = Attr::create(document(), attrNode.qualifiedName(), elementData.attributeAt(index).value());
+        }
     }
 
     attachAttributeNodeIfNeeded(attrNode);
@@ -2474,14 +2475,14 @@ void Element::blur()
 
 void Element::dispatchFocusInEvent(const AtomicString& eventType, RefPtr<Element>&& oldFocusedElement)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
     ASSERT(eventType == eventNames().focusinEvent || eventType == eventNames().DOMFocusInEvent);
     dispatchScopedEvent(FocusEvent::create(eventType, true, false, document().defaultView(), 0, WTFMove(oldFocusedElement)));
 }
 
 void Element::dispatchFocusOutEvent(const AtomicString& eventType, RefPtr<Element>&& newFocusedElement)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
     ASSERT(eventType == eventNames().focusoutEvent || eventType == eventNames().DOMFocusOutEvent);
     dispatchScopedEvent(FocusEvent::create(eventType, true, false, document().defaultView(), 0, WTFMove(newFocusedElement)));
 }
index a396082..6b785e9 100644 (file)
@@ -130,7 +130,7 @@ static bool shouldSuppressEventDispatchInDOM(Node& node, Event& event)
 
 bool EventDispatcher::dispatchEvent(Node& node, Event& event)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(node));
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree(node));
     Ref<Node> protectedNode(node);
     RefPtr<FrameView> view = node.document().view();
     EventPath eventPath(node, event);
index 1603861..fd23caf 100644 (file)
@@ -63,11 +63,41 @@ public:
 #endif
     }
 
-    static bool isEventDispatchAllowedInSubtree(Node& node)
-    {
-        return isEventAllowedInMainThread() || EventAllowedScope::isAllowedNode(node);
-    }
+    class InMainThread {
+    public:
+        InMainThread()
+        {
+            ASSERT(isMainThread());
+#if !ASSERT_DISABLED
+            ++s_count;
+#endif
+        }
 
+        ~InMainThread()
+        {
+            ASSERT(isMainThread());
+#if !ASSERT_DISABLED
+            ASSERT(s_count);
+            --s_count;
+#endif
+        }
+
+        static bool isEventDispatchAllowedInSubtree(Node& node)
+        {
+            return isEventAllowed() || EventAllowedScope::isAllowedNode(node);
+        }
+
+        static bool isEventAllowed()
+        {
+            ASSERT(isMainThread());
+#if !ASSERT_DISABLED
+            return !s_count;
+#else
+            return true;
+#endif
+        }
+    };
+    
 #if !ASSERT_DISABLED
     class EventAllowedScope {
     public:
index 0f71b80..846acf0 100644 (file)
@@ -2326,7 +2326,7 @@ void Node::dispatchSubtreeModifiedEvent()
     if (isInShadowTree())
         return;
 
-    RELEASE_ASSERT(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(*this));
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree(*this));
 
     if (!document().hasListenerType(Document::DOMSUBTREEMODIFIED_LISTENER))
         return;
@@ -2339,7 +2339,7 @@ void Node::dispatchSubtreeModifiedEvent()
 
 bool Node::dispatchDOMActivateEvent(int detail, Event& underlyingEvent)
 {
-    RELEASE_ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());
     Ref<UIEvent> event = UIEvent::create(eventNames().DOMActivateEvent, true, true, document().defaultView(), detail);
     event->setUnderlyingEvent(&underlyingEvent);
     dispatchScopedEvent(event);
index 5a60fbb..382d2c8 100644 (file)
@@ -190,7 +190,7 @@ bool ScriptExecutionContext::canSuspendActiveDOMObjectsForDocumentSuspension(Vec
     // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
     // An ASSERT_WITH_SECURITY_IMPLICATION or RELEASE_ASSERT will fire if this happens, but it's important to code
     // canSuspend functions so it will not happen!
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
     for (auto* activeDOMObject : m_activeDOMObjects) {
         if (!activeDOMObject->canSuspendForDocumentSuspension()) {
             canSuspend = false;
@@ -232,7 +232,7 @@ void ScriptExecutionContext::suspendActiveDOMObjects(ActiveDOMObject::ReasonForS
     // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
     // An ASSERT_WITH_SECURITY_IMPLICATION or RELEASE_ASSERT will fire if this happens, but it's important to code
     // suspend functions so it will not happen!
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
     for (auto* activeDOMObject : m_activeDOMObjects)
         activeDOMObject->suspend(why);
 
@@ -261,7 +261,7 @@ void ScriptExecutionContext::resumeActiveDOMObjects(ActiveDOMObject::ReasonForSu
     // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
     // An ASSERT_WITH_SECURITY_IMPLICATION or RELEASE_ASSERT will fire if this happens, but it's important to code
     // resume functions so it will not happen!
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
     for (auto* activeDOMObject : m_activeDOMObjects)
         activeDOMObject->resume();
 
index 851c197..20a9c4a 100644 (file)
@@ -108,7 +108,7 @@ void CachedPage::restore(Page& page)
     {
         // Do not dispatch DOM events as their JavaScript listeners could cause the page to be put
         // into the page cache before we have finished restoring it from the page cache.
-        NoEventDispatchAssertion noEventDispatchAssertion;
+        NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
 
         m_cachedMainFrame->open();
     }
index aaac284..326f2e7 100644 (file)
@@ -423,7 +423,7 @@ void PageCache::addIfCacheable(HistoryItem& item, Page* page)
     setPageCacheState(*page, Document::InPageCache);
 
     // Make sure we no longer fire any JS events past this point.
-    NoEventDispatchAssertion assertNoEventDispatch;
+    NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
     item.m_cachedPage = std::make_unique<CachedPage>(*page);
     item.m_pruningReason = PruningReason::None;
index 00b599d..e0002ca 100644 (file)
@@ -211,7 +211,7 @@ void LayoutContext::layout()
     }
     {
         SetForScope<LayoutPhase> layoutPhase(m_layoutPhase, LayoutPhase::InRenderTreeLayout);
-        NoEventDispatchAssertion noEventDispatchAssertion;
+        NoEventDispatchAssertion::InMainThread noEventDispatchAssertion;
         SubtreeLayoutStateMaintainer subtreeLayoutStateMaintainer(subtreeLayoutRoot());
         RenderView::RepaintRegionAccumulator repaintRegionAccumulator(renderView());
 #ifndef NDEBUG
index 9cc1857..0ca130f 100644 (file)
@@ -2526,7 +2526,7 @@ void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const Layo
 
             if (frameElementAndViewPermitScroll(frameElementBase, frameView)) {
                 // If this assertion fires we need to protect the ownerElement from being destroyed.
-                NoEventDispatchAssertion assertNoEventDispatch;
+                NoEventDispatchAssertion::InMainThread assertNoEventDispatch;
 
                 LayoutRect viewRect = frameView.visibleContentRect(LegacyIOSDocumentVisibleRect);
                 LayoutRect exposeRect = getRectToExpose(viewRect, absoluteRect, insideFixed, alignX, alignY);