ASSERTION FAILED: (!s_current || &m_view != &s_current->m_view) in RenderTreeBuilder...
authorshihchieh_lee@apple.com <shihchieh_lee@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 05:53:52 +0000 (05:53 +0000)
committershihchieh_lee@apple.com <shihchieh_lee@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 05:53:52 +0000 (05:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212163
<rdar://problem/57028096>

Reviewed by Geoffrey Garen.

Source/WebCore:

Calling ~PostResolutionCallbackDisabler() before completing render tree updating and releasing RenderTreeBuilder
triggers this assertion. Therefore we added a utility function "updateRenderTree" in which PostResolutionCallback
is delayed until RenderTreeUpdater is released and m_inRenderTreeUpdate is cleared.

Test: fast/rendering/nested-render-tree-update-crash.html

* Headers.cmake:
* WebCore.xcodeproj/project.pbxproj:
* dom/Document.cpp:
(WebCore::Document::updateRenderTree):
(WebCore::Document::resolveStyle):
(WebCore::Document::updateTextRenderer):
* dom/Document.h:
* rendering/updating/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::RenderTreeUpdater):
(WebCore::RenderTreeUpdater::commit):
* rendering/updating/RenderTreeUpdater.h:

LayoutTests:

Added a regression test for the crash.

* fast/rendering/nested-render-tree-update-crash-expected.txt: Added.
* fast/rendering/nested-render-tree-update-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/rendering/nested-render-tree-update-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/rendering/nested-render-tree-update-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Headers.cmake
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/rendering/updating/RenderTreeUpdater.cpp
Source/WebCore/rendering/updating/RenderTreeUpdater.h

index 9b153df..cc04054 100644 (file)
@@ -1,3 +1,16 @@
+2020-05-22  Jack Lee  <shihchieh_lee@apple.com>
+
+        ASSERTION FAILED: (!s_current || &m_view != &s_current->m_view) in RenderTreeBuilder::RenderTreeBuilder
+        https://bugs.webkit.org/show_bug.cgi?id=212163
+        <rdar://problem/57028096>
+
+        Reviewed by Geoffrey Garen.
+
+        Added a regression test for the crash.
+
+        * fast/rendering/nested-render-tree-update-crash-expected.txt: Added.
+        * fast/rendering/nested-render-tree-update-crash.html: Added.
+
 2020-05-22  Zalan Bujtas  <zalan@apple.com>
 
         Nullptr deref in WebCore::RenderTreeBuilder::Block::attachIgnoringContinuation when parent and beforeChild are siblings
diff --git a/LayoutTests/fast/rendering/nested-render-tree-update-crash-expected.txt b/LayoutTests/fast/rendering/nested-render-tree-update-crash-expected.txt
new file mode 100644 (file)
index 0000000..9d13d0b
--- /dev/null
@@ -0,0 +1 @@
+Tests nested render tree update. The test passes if WebKit doesn't crash or hit an assertion. 
diff --git a/LayoutTests/fast/rendering/nested-render-tree-update-crash.html b/LayoutTests/fast/rendering/nested-render-tree-update-crash.html
new file mode 100644 (file)
index 0000000..a328b8d
--- /dev/null
@@ -0,0 +1,13 @@
+<script>\r
+function run() {\r
+    if (window.testRunner)\r
+        testRunner.dumpAsText();\r
+\r
+    obj = document.createElement("object");\r
+    li.appendChild(obj);\r
+    svg.currentScale = 0.99;\r
+    obj.data = String.fromCharCode(89, 82)\r
+    ff.setAttribute("direction", "rtl");\r
+}\r
+</script>\r
+<body onload=run()><li id=li><svg id=svg><font-face-uri id=ff><tref xlink:href=#ff><span>Tests nested render tree update. The test passes if WebKit doesn't crash or hit an assertion.</span>\r
index 029b6ec..eaa03ec 100644 (file)
@@ -1,3 +1,29 @@
+2020-05-22  Jack Lee  <shihchieh_lee@apple.com>
+
+        ASSERTION FAILED: (!s_current || &m_view != &s_current->m_view) in RenderTreeBuilder::RenderTreeBuilder
+        https://bugs.webkit.org/show_bug.cgi?id=212163
+        <rdar://problem/57028096>
+
+        Reviewed by Geoffrey Garen.
+
+        Calling ~PostResolutionCallbackDisabler() before completing render tree updating and releasing RenderTreeBuilder 
+        triggers this assertion. Therefore we added a utility function "updateRenderTree" in which PostResolutionCallback
+        is delayed until RenderTreeUpdater is released and m_inRenderTreeUpdate is cleared.
+
+        Test: fast/rendering/nested-render-tree-update-crash.html
+
+        * Headers.cmake:
+        * WebCore.xcodeproj/project.pbxproj:
+        * dom/Document.cpp:
+        (WebCore::Document::updateRenderTree):
+        (WebCore::Document::resolveStyle):
+        (WebCore::Document::updateTextRenderer):
+        * dom/Document.h:
+        * rendering/updating/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::RenderTreeUpdater):
+        (WebCore::RenderTreeUpdater::commit):
+        * rendering/updating/RenderTreeUpdater.h:
+
 2020-05-22  Simon Fraser  <simon.fraser@apple.com>
 
         Stuttery overflow scrolling in slow-scrolling regions (facebook messenger, feedly.com)
index b05f941..b589dfc 100644 (file)
@@ -1476,6 +1476,7 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS
 
     style/StyleChange.h
     style/StyleScope.h
+    style/StyleUpdate.h
     style/StyleValidity.h
 
     svg/SVGLengthContext.h
index 0489ae4..262dcda 100644 (file)
                E424A39E1330DF0100CF6DC9 /* LegacyTileGridTile.h in Headers */ = {isa = PBXBuildFile; fileRef = E424A39D1330DF0100CF6DC9 /* LegacyTileGridTile.h */; };
                E425A49A18292B840020CFCF /* CollectionIndexCache.h in Headers */ = {isa = PBXBuildFile; fileRef = E425A49918292B840020CFCF /* CollectionIndexCache.h */; settings = {ATTRIBUTES = (Private, ); }; };
                E4295FA412B0614E00D1ACE0 /* ResourceLoadPriority.h in Headers */ = {isa = PBXBuildFile; fileRef = E4295FA312B0614E00D1ACE0 /* ResourceLoadPriority.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               E42E76DC1C7AF77600E3614D /* StyleUpdate.h in Headers */ = {isa = PBXBuildFile; fileRef = E42E76DB1C7AF77600E3614D /* StyleUpdate.h */; };
+               E42E76DC1C7AF77600E3614D /* StyleUpdate.h in Headers */ = {isa = PBXBuildFile; fileRef = E42E76DB1C7AF77600E3614D /* StyleUpdate.h */; settings = {ATTRIBUTES = (Private, ); }; };
                E43105BB16750F1600DB2FB8 /* NodeTraversal.h in Headers */ = {isa = PBXBuildFile; fileRef = E43105BA16750F1600DB2FB8 /* NodeTraversal.h */; settings = {ATTRIBUTES = (Private, ); }; };
                E4343D232392778400EBBB66 /* LineLayoutTraversalSimplePath.h in Headers */ = {isa = PBXBuildFile; fileRef = E4343D212392778300EBBB66 /* LineLayoutTraversalSimplePath.h */; settings = {ATTRIBUTES = (Private, ); }; };
                E4343D252392779000EBBB66 /* LineLayoutTraversalComplexPath.h in Headers */ = {isa = PBXBuildFile; fileRef = E4343D242392778F00EBBB66 /* LineLayoutTraversalComplexPath.h */; settings = {ATTRIBUTES = (Private, ); }; };
index 513b828..ed98ba9 100644 (file)
@@ -1920,6 +1920,19 @@ bool Document::hasPendingFullStyleRebuild() const
     return hasPendingStyleRecalc() && m_needsFullStyleRebuild;
 }
 
+void Document::updateRenderTree(std::unique_ptr<const Style::Update> styleUpdate)
+{
+    ASSERT(!inRenderTreeUpdate());
+
+    // NOTE: Preserve the order of definitions below so the destructors are called in proper sequence.
+    Style::PostResolutionCallbackDisabler callbackDisabler(*this);
+    SetForScope<bool> inRenderTreeUpdate(m_inRenderTreeUpdate, true);
+    RenderTreeUpdater updater(*this, callbackDisabler);
+    // End of ordered definitions
+
+    updater.commit(WTFMove(styleUpdate));
+}
+
 void Document::resolveStyle(ResolveStyleType type)
 {
     ASSERT(!view() || !view()->isPainting());
@@ -2004,11 +2017,7 @@ void Document::resolveStyle(ResolveStyleType type)
         m_inStyleRecalc = false;
 
         if (styleUpdate) {
-            SetForScope<bool> inRenderTreeUpdate(m_inRenderTreeUpdate, true);
-
-            RenderTreeUpdater updater(*this);
-            updater.commit(WTFMove(styleUpdate));
-
+            updateRenderTree(WTFMove(styleUpdate));
             frameView.styleAndRenderTreeDidChange();
         }
 
@@ -2042,14 +2051,10 @@ void Document::resolveStyle(ResolveStyleType type)
 
 void Document::updateTextRenderer(Text& text, unsigned offsetOfReplacedText, unsigned lengthOfReplacedText)
 {
-    ASSERT(!m_inRenderTreeUpdate);
-    SetForScope<bool> inRenderTreeUpdate(m_inRenderTreeUpdate, true);
-
     auto textUpdate = makeUnique<Style::Update>(*this);
     textUpdate->addText(text, { offsetOfReplacedText, lengthOfReplacedText, WTF::nullopt });
 
-    RenderTreeUpdater renderTreeUpdater(*this);
-    renderTreeUpdater.commit(WTFMove(textUpdate));
+    updateRenderTree(WTFMove(textUpdate));
 }
 
 bool Document::needsStyleRecalc() const
index 9fc37d1..948ae46 100644 (file)
@@ -55,6 +55,7 @@
 #include "SecurityPolicyViolationEvent.h"
 #include "StringWithDirection.h"
 #include "StyleColor.h"
+#include "StyleUpdate.h"
 #include "Supplementable.h"
 #include "TextResourceDecoder.h"
 #include "Timer.h"
@@ -643,6 +644,7 @@ public:
 
     bool renderTreeBeingDestroyed() const { return m_renderTreeBeingDestroyed; }
     bool hasLivingRenderTree() const { return renderView() && !renderTreeBeingDestroyed(); }
+    void updateRenderTree(std::unique_ptr<const Style::Update> styleUpdate);
     
     bool updateLayoutIfDimensionsOutOfDate(Element&, DimensionsCheck = AllDimensionsCheck);
     
index 1e6e1f0..dd8bf8e 100644 (file)
@@ -78,7 +78,7 @@ RenderTreeUpdater::Parent::Parent(Element& element, const Style::ElementUpdates*
 {
 }
 
-RenderTreeUpdater::RenderTreeUpdater(Document& document)
+RenderTreeUpdater::RenderTreeUpdater(Document& document, Style::PostResolutionCallbackDisabler&)
     : m_document(document)
     , m_generatedContent(makeUnique<GeneratedContent>(*this))
     , m_builder(renderView())
@@ -121,8 +121,6 @@ void RenderTreeUpdater::commit(std::unique_ptr<const Style::Update> styleUpdate)
     
     TraceScope scope(RenderTreeBuildStart, RenderTreeBuildEnd);
 
-    Style::PostResolutionCallbackDisabler callbackDisabler(m_document);
-
     m_styleUpdate = WTFMove(styleUpdate);
 
     for (auto* root : findRenderingRoots(*m_styleUpdate))
index 531f2cf..234034c 100644 (file)
@@ -28,6 +28,7 @@
 #include "RenderTreeBuilder.h"
 #include "RenderTreePosition.h"
 #include "StyleChange.h"
+#include "StyleTreeResolver.h"
 #include "StyleUpdate.h"
 #include <wtf/HashSet.h>
 #include <wtf/Vector.h>
@@ -43,7 +44,7 @@ class Text;
 
 class RenderTreeUpdater {
 public:
-    RenderTreeUpdater(Document&);
+    RenderTreeUpdater(Document&, Style::PostResolutionCallbackDisabler&);
     ~RenderTreeUpdater();
 
     void commit(std::unique_ptr<const Style::Update>);