Add assertions to CachedFrame to help figure out crash in CachedFrame constructor
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 May 2019 20:25:27 +0000 (20:25 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 May 2019 20:25:27 +0000 (20:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197621

Reviewed by Geoffrey Garen.

Add release assertions to try and figure out who is sometimes detaching the document from its
frame while constructing CachedFrames for its descendants.

* dom/Document.cpp:
(WebCore::Document::detachFromFrame):
* dom/Document.h:
(WebCore::Document::setMayBeDetachedFromFrame):
* history/CachedFrame.cpp:
(WebCore::CachedFrame::CachedFrame):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/history/CachedFrame.cpp

index c6c1f76..22b67eb 100644 (file)
@@ -1,3 +1,20 @@
+2019-05-06  Chris Dumez  <cdumez@apple.com>
+
+        Add assertions to CachedFrame to help figure out crash in CachedFrame constructor
+        https://bugs.webkit.org/show_bug.cgi?id=197621
+
+        Reviewed by Geoffrey Garen.
+
+        Add release assertions to try and figure out who is sometimes detaching the document from its
+        frame while constructing CachedFrames for its descendants.
+
+        * dom/Document.cpp:
+        (WebCore::Document::detachFromFrame):
+        * dom/Document.h:
+        (WebCore::Document::setMayBeDetachedFromFrame):
+        * history/CachedFrame.cpp:
+        (WebCore::CachedFrame::CachedFrame):
+
 2019-05-06  Zan Dobersek  <zdobersek@igalia.com>
 
         [GLib] WebCore::MainThreadSharedTimer should use the appropriate GSource priority, name
index 9136d9c..966179d 100644 (file)
@@ -8130,6 +8130,10 @@ bool Document::registerCSSProperty(CSSRegisteredCustomProperty&& prop)
 
 void Document::detachFromFrame()
 {
+    // Assertion to help pinpint rdar://problem/49877867. If this hits, the crash trace should tell us
+    // which piece of code is detaching the document from its frame while constructing the CachedFrames.
+    RELEASE_ASSERT(m_mayBeDetachedFromFrame);
+
     observeFrame(nullptr);
 }
 
index 941d5f5..dddc83d 100644 (file)
@@ -1462,6 +1462,9 @@ public:
     TextAutoSizing& textAutoSizing();
 #endif
 
+    // For debugging rdar://problem/49877867.
+    void setMayBeDetachedFromFrame(bool mayBeDetachedFromFrame) { m_mayBeDetachedFromFrame = mayBeDetachedFromFrame; }
+
     Logger& logger();
 
     void hasStorageAccess(Ref<DeferredPromise>&& passedPromise);
@@ -2059,6 +2062,7 @@ private:
 
     bool m_hasEvaluatedUserAgentScripts { false };
     bool m_isRunningUserScripts { false };
+    bool m_mayBeDetachedFromFrame { true };
 #if ENABLE(APPLE_PAY)
     bool m_hasStartedApplePaySession { false };
 #endif
index e461649..6aba928 100644 (file)
@@ -143,10 +143,20 @@ CachedFrame::CachedFrame(Frame& frame)
     ASSERT(m_view);
     ASSERT(m_document->pageCacheState() == Document::InPageCache);
 
+    RELEASE_ASSERT(m_document->domWindow());
+    RELEASE_ASSERT(m_document->frame());
+    RELEASE_ASSERT(m_document->domWindow()->frame());
+
+    // FIXME: We have evidence that constructing CachedFrames for descendant frames may detach the document from its frame (rdar://problem/49877867).
+    // This sets the flag to help find the guilty code.
+    m_document->setMayBeDetachedFromFrame(false);
+
     // Create the CachedFrames for all Frames in the FrameTree.
     for (Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
         m_childFrames.append(std::make_unique<CachedFrame>(*child));
 
+    RELEASE_ASSERT(m_document->domWindow());
+    RELEASE_ASSERT(m_document->frame());
     RELEASE_ASSERT(m_document->domWindow()->frame());
 
     // Active DOM objects must be suspended before we cache the frame script data.
@@ -193,6 +203,7 @@ CachedFrame::CachedFrame(Frame& frame)
     }
 #endif
 
+    m_document->setMayBeDetachedFromFrame(true);
     m_document->detachFromCachedFrame(*this);
 
     ASSERT_WITH_SECURITY_IMPLICATION(!m_documentLoader->isLoading());