Attachment 357927 Details for Bug 192859 – Fixes the bug

[patch] Fixes the bug

bug-192859-20181220220000.patch (text/plain), 4.76 KB, created by Ryosuke Niwa on 2018-12-20 22:00:01 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-12-20 22:00:01 PST

Size: 4.76 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 239493)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,31 @@
>+2018-12-20  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        REGRESSION(r239353): iOS WK1 Assertion failure in notifyChildNodeRemoved while running
>+        TestWebKitAPI.QuickLook.LegacyQuickLookContent
>+        https://bugs.webkit.org/show_bug.cgi?id=192859
>+        <rdar://problem/46887237>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        After r239353, ScriptDisallowedScope::InMainThread::isScriptAllowed() may return false when the web thread
>+        is inside a delegate callback even when there is a ScriptDisallowedScope defined.
>+
>+        Replace the existign debug assertions which assert !ScriptDisallowedScope::InMainThread::isScriptAllowed()
>+        by a newly added ScriptDisallowedScope::InMainThread::hasDisallowedScope to avoid hitting this assertion.
>+
>+        Tests: TestWebKitAPI.QuickLook.LegacyQuickLookContent
>+
>+        * dom/ContainerNodeAlgorithms.cpp:
>+        (WebCore::notifyChildNodeInserted):
>+        (WebCore::notifyChildNodeRemoved):
>+        * dom/Document.cpp:
>+        (WebCore::Document::nodeChildrenWillBeRemoved):
>+        (WebCore::Document::nodeWillBeRemoved):
>+        * dom/ScriptDisallowedScope.h:
>+        (WebCore::ScriptDisallowedScope::InMainThread::hasDisallowedScope):
>+        * html/HTMLFormElement.cpp:
>+        (WebCore:: const):
>+
> 2018-12-20  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Unreviewed, remove stray #pragma once added to .cpp file
>Index: Source/WebCore/dom/ContainerNodeAlgorithms.cpp
>===================================================================
>--- Source/WebCore/dom/ContainerNodeAlgorithms.cpp	(revision 239491)
>+++ Source/WebCore/dom/ContainerNodeAlgorithms.cpp	(working copy)
>@@ -88,7 +88,7 @@ static void notifyNodeInsertedIntoTree(C
> 
> NodeVector notifyChildNodeInserted(ContainerNode& parentOfInsertedTree, Node& node)
> {
>-    ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
>+    ASSERT(ScriptDisallowedScope::InMainThread::hasDisallowedScope());
> 
>     InspectorInstrumentation::didInsertDOMNode(node.document(), node);
> 
>@@ -152,7 +152,7 @@ static void notifyNodeRemovedFromTree(Co
> void notifyChildNodeRemoved(ContainerNode& oldParentOfRemovedTree, Node& child)
> {
>     // Assert that the caller of this function has an instance of ScriptDisallowedScope.
>-    ASSERT(!isMainThread() || !ScriptDisallowedScope::InMainThread::isScriptAllowed());
>+    ASSERT(!isMainThread() || ScriptDisallowedScope::InMainThread::hasDisallowedScope());
>     ContainerChildRemovalScope removalScope(oldParentOfRemovedTree, child);
> 
>     // Tree scope has changed if the container node from which "node" is removed is in a document or a shadow root.
>Index: Source/WebCore/dom/Document.cpp
>===================================================================
>--- Source/WebCore/dom/Document.cpp	(revision 239491)
>+++ Source/WebCore/dom/Document.cpp	(working copy)
>@@ -4358,7 +4358,7 @@ void Document::updateRangesAfterChildren
> 
> void Document::nodeChildrenWillBeRemoved(ContainerNode& container)
> {
>-    ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
>+    ASSERT(ScriptDisallowedScope::InMainThread::hasDisallowedScope());
> 
>     adjustFocusedNodeOnNodeRemoval(container, NodeRemoval::ChildrenOfNode);
>     adjustFocusNavigationNodeOnNodeRemoval(container, NodeRemoval::ChildrenOfNode);
>@@ -4391,7 +4391,7 @@ void Document::nodeChildrenWillBeRemoved
> 
> void Document::nodeWillBeRemoved(Node& node)
> {
>-    ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
>+    ASSERT(ScriptDisallowedScope::InMainThread::hasDisallowedScope());
> 
>     adjustFocusedNodeOnNodeRemoval(node);
>     adjustFocusNavigationNodeOnNodeRemoval(node);
>Index: Source/WebCore/dom/ScriptDisallowedScope.h
>===================================================================
>--- Source/WebCore/dom/ScriptDisallowedScope.h	(revision 239491)
>+++ Source/WebCore/dom/ScriptDisallowedScope.h	(working copy)
>@@ -87,6 +87,8 @@ public:
> #endif
>         }
> 
>+        static bool hasDisallowedScope() { return s_count; }
>+
>         static bool isScriptAllowed()
>         {
>             ASSERT(isMainThread());
>Index: Source/WebCore/html/HTMLFormElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLFormElement.cpp	(revision 239491)
>+++ Source/WebCore/html/HTMLFormElement.cpp	(working copy)
>@@ -863,7 +863,7 @@ void HTMLFormElement::finishParsingChild
> 
> const Vector<FormAssociatedElement*>& HTMLFormElement::unsafeAssociatedElements() const
> {
>-    ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
>+    ASSERT(ScriptDisallowedScope::InMainThread::hasDisallowedScope());
>     return m_associatedElements;
> }
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192859: 357927