document should not be null after navigation with site isolation enabled

achristensen07 · achristensen07 · commit 3c907c552d9c · 2025-06-09T17:31:14.000-07:00
https://bugs.webkit.org/show_bug.cgi?id=294228 rdar://152872994 Reviewed by Charlie Wolfe. https://commits.webkit.org/292437@main introduced a case where we take the WindowProxy from one page to another when navigating the main frame from one domain to another. If the WindowProxy that we are taking had its JSWindowProxy initialized by an iframe on the previous page accessing its parent's properties, then JSDOMWindowBase::initStaticGlobals has already been called to initialize document to null, but ScriptController::initScriptForWindowProxy won't call JSDOMWindowBase::updateDocument to set document to the document instead of null because WindowProxy::jsWindowProxy will find an already existing JSWindowProxy initialized where WindowProxy::createJSWindowProxyWithInitializedScript has skipped the call to initScriptForWindowProxy because the Frame was a RemoteFrame when it was initialized. To fix this issue, we add another call to didBecomeCurrentDocumentInFrame after swapping from a RemoteFrame to a LocalFrame. That will call updateDocument as well as do possibly other initialization needed. I also restore an assertion that was removed in 292437@main, but I added a condition to make it pass when taking a WindowProxy from one page to another. This fixes one of the issues I found in rdar://152460976 but not the initial one, so that still needs more investigation. * LayoutTests/http/tests/site-isolation/document-access-expected.txt: Added. * LayoutTests/http/tests/site-isolation/document-access.html: Added. * LayoutTests/http/tests/site-isolation/resources/access-document.html: Added. * LayoutTests/http/tests/site-isolation/resources/access-parent.html: Added. * Source/WebCore/dom/Document.h: * Source/WebCore/page/Frame.cpp: (WebCore::Frame::takeWindowProxyAndOpenerFrom): * Source/WebKit/WebProcess/WebPage/WebFrame.cpp: (WebKit::WebFrame::commitProvisionalFrame): Canonical link: https://commits.webkit.org/296022@main
diff --git a/LayoutTests/http/tests/site-isolation/document-access-expected.txt b/LayoutTests/http/tests/site-isolation/document-access-expected.txt
@@ -0,0 +1,2 @@
+ALERT: document is null? false
+
diff --git a/LayoutTests/http/tests/site-isolation/document-access.html b/LayoutTests/http/tests/site-isolation/document-access.html
@@ -0,0 +1,8 @@
+<!-- webkit-test-runner [ SiteIsolationEnabled=true ] -->
+<body>
+<script>
+    if (window.testRunner) { testRunner.dumpAsText(); testRunner.waitUntilDone() }
+    onload = ()=>{ window.location = 'http://localhost:8000/site-isolation/resources/access-document.html' }
+</script>
+<iframe src="http://localhost:8000/site-isolation/resources/access-parent.html" frameborder=0></iframe>
+</body>
diff --git a/LayoutTests/http/tests/site-isolation/resources/access-document.html b/LayoutTests/http/tests/site-isolation/resources/access-document.html
@@ -0,0 +1,4 @@
+<script>
+    alert('document is null? ' + (document == null))
+    if (window.testRunner) { testRunner.notifyDone() }
+</script>
diff --git a/LayoutTests/http/tests/site-isolation/resources/access-parent.html b/LayoutTests/http/tests/site-isolation/resources/access-parent.html
@@ -0,0 +1,3 @@
+<script>
+    window.parent.frames
+</script>
diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
@@ -754,7 +754,7 @@ class Document
     inline CachedResourceLoader& cachedResourceLoader();
     inline Ref<CachedResourceLoader> protectedCachedResourceLoader() const;
 
-    void didBecomeCurrentDocumentInFrame();
+    WEBCORE_EXPORT void didBecomeCurrentDocumentInFrame();
     void destroyRenderTree();
     WEBCORE_EXPORT void willBeRemovedFromFrame();
 
diff --git a/Source/WebCore/page/Frame.cpp b/Source/WebCore/page/Frame.cpp
@@ -173,6 +173,7 @@ void Frame::disconnectOwnerElement()
 
 void Frame::takeWindowProxyAndOpenerFrom(Frame& frame)
 {
+    ASSERT(is<LocalDOMWindow>(window()) != is<LocalDOMWindow>(frame.window()) || page() != frame.page());
     ASSERT(m_windowProxy->frame() == this);
     m_windowProxy->detachFromFrame();
     m_windowProxy = frame.windowProxy();
diff --git a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
@@ -518,6 +518,8 @@ void WebFrame::commitProvisionalFrame()
     if (remoteFrame->isMainFrame())
         corePage->setMainFrame(*localFrame);
     localFrame->takeWindowProxyAndOpenerFrom(*remoteFrame);
+    if (RefPtr document = localFrame->document())
+        document->didBecomeCurrentDocumentInFrame();
 
     if (corePage->focusController().focusedFrame() == remoteFrame.get())
         corePage->focusController().setFocusedFrame(localFrame.get(), FocusController::BroadcastFocusedFrame::No);

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+<script>`
	`2`	`+ window.parent.frames`
	`3`	`+</script>`
Original file line number	Diff line number	Diff line change
`@@ -173,6 +173,7 @@ void Frame::disconnectOwnerElement()`
`173`	`173`
`174`	`174`	`void Frame::takeWindowProxyAndOpenerFrom(Frame& frame)`
`175`	`175`	`{`
	`176`	`+ ASSERT(is<LocalDOMWindow>(window()) != is<LocalDOMWindow>(frame.window()) \|\| page() != frame.page());`
`176`	`177`	`ASSERT(m_windowProxy->frame() == this);`
`177`	`178`	`m_windowProxy->detachFromFrame();`
`178`	`179`	`m_windowProxy = frame.windowProxy();`