Add radar links for site isolation FIXME comments

https://bugs.webkit.org/show_bug.cgi?id=262332 rdar://116204383 Reviewed by Pascoe. A keen observer will notice I've been taking a lot of shortcuts in the last few months and adding comments indicating the need for future cleanup. This adds tracking to those comments. * Source/WebCore/page/FocusController.cpp: (WebCore::FocusController::focusedOrMainFrame const): * Source/WebCore/page/RemoteDOMWindow.cpp: (WebCore::RemoteDOMWindow::location const): (WebCore::RemoteDOMWindow::close): (WebCore::RemoteDOMWindow::closed const): (WebCore::RemoteDOMWindow::focus): (WebCore::RemoteDOMWindow::blur): * Source/WebCore/page/RemoteDOMWindow.idl: * Source/WebKit/UIProcess/BrowsingContextGroup.cpp: * Source/WebKit/UIProcess/Cocoa/UIDelegate.mm: (WebKit::UIDelegate::UIClient::createNewPage): * Source/WebKit/UIProcess/RemotePageProxy.cpp: (WebKit::RemotePageProxy::injectPageIntoNewProcess): (WebKit::RemotePageProxy::didReceiveMessage): * Source/WebKit/UIProcess/WebFrameProxy.cpp: (WebKit::WebFrameProxy::prepareForProvisionalNavigationInProcess): * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::receivedNavigationPolicyDecision): (WebKit::WebPageProxy::continueNavigationInNewProcess): * Source/WebKit/UIProcess/WebProcessPool.cpp: (WebKit::WebProcessPool::processForNavigationInternal): * Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp: (WebKit::WebChromeClient::attachViewOverlayGraphicsLayer): * Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp: (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): * Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp: (WebKit::WebRemoteFrameClient::changeLocation): * Source/WebKit/WebProcess/WebPage/DrawingArea.h: * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm: (WebKit::RemoteLayerTreeDrawingArea::updateRendering): * Source/WebKit/WebProcess/WebPage/WebFrame.cpp: (WebKit::WebFrame::createRemoteSubframe): (WebKit::WebFrame::didCommitLoadInAnotherProcess): * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::m_historyItemClient): * Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm: (WebKit::WebPage::viewportConfigurationChanged): (WebKit::WebPage::platformUserAgent const): Canonical link: https://commits.webkit.org/268618@main
WebKit · Sep 28, 2023 · 57435dd · 57435dd
1 parent c9652ad
commit 57435dd
Show file tree

Hide file tree

Showing 17 changed files with 34 additions and 29 deletions.
diff --git a/Source/WebCore/page/FocusController.cpp b/Source/WebCore/page/FocusController.cpp
@@ -492,7 +492,7 @@ LocalFrame& FocusController::focusedOrMainFrame() const
     if (auto* localMainFrame = dynamicDowncast<LocalFrame>(m_page.mainFrame()))
         return *localMainFrame;
 
-    // FIXME: Do something better here in the site isolated case.
+    // FIXME: Do something better here in the site isolated case. <rdar://116201648>
     ASSERT(m_page.settings().siteIsolationEnabled());
     return *m_page.rootFrames().begin();
 }

diff --git a/Source/WebCore/page/RemoteDOMWindow.cpp b/Source/WebCore/page/RemoteDOMWindow.cpp
@@ -57,29 +57,30 @@ WindowProxy* RemoteDOMWindow::self() const
 
 Location* RemoteDOMWindow::location() const
 {
-    // FIXME: Implemented this.
+    // FIXME: Implemented this. <rdar://116203970>
     return nullptr;
 }
 
 void RemoteDOMWindow::close(Document&)
 {
-    // FIXME: Implemented this.
+    // FIXME: Implemented this. <rdar://116203970>
 }
 
 bool RemoteDOMWindow::closed() const
 {
+    // FIXME: This is probably not completely correct. <rdar://116203970>
     return !m_frame;
 }
 
 void RemoteDOMWindow::focus(LocalDOMWindow& incumbentWindow)
 {
     UNUSED_PARAM(incumbentWindow);
-    // FIXME: Implemented this.
+    // FIXME: Implemented this. <rdar://116203970>
 }
 
 void RemoteDOMWindow::blur()
 {
-    // FIXME: Implemented this.
+    // FIXME: Implemented this. <rdar://116203970>
 }
 
 unsigned RemoteDOMWindow::length() const

diff --git a/Source/WebCore/page/RemoteDOMWindow.idl b/Source/WebCore/page/RemoteDOMWindow.idl
@@ -43,7 +43,7 @@
 ] interface RemoteDOMWindow {
     [LegacyUnforgeable, ImplementedAs=self] readonly attribute WindowProxy window;
     [Replaceable] readonly attribute WindowProxy self;
-    [PutForwards=href, LegacyUnforgeable] readonly attribute Location? location; // FIXME: Should not be nullable.
+    [PutForwards=href, LegacyUnforgeable] readonly attribute Location? location; // FIXME: Should not be nullable. <rdar://116203895>
     [CallWith=IncumbentDocument] undefined close();
     readonly attribute boolean closed;
     [CallWith=IncumbentWindow] undefined focus();

diff --git a/Source/WebKit/UIProcess/BrowsingContextGroup.cpp b/Source/WebKit/UIProcess/BrowsingContextGroup.cpp
@@ -40,6 +40,7 @@ WebProcessProxy* BrowsingContextGroup::processForDomain(const WebCore::Registrab
     return process.get();
 }
 
+// FIXME: This needs a corresponding remove call when a process terminates. <rdar://116202371>
 void BrowsingContextGroup::addProcessForDomain(const WebCore::RegistrableDomain& domain, WebProcessProxy& process)
 {
     ASSERT(!m_processMap.get(domain) || m_processMap.get(domain)->state() == WebProcessProxy::State::Terminated || m_processMap.get(domain) == &process);

diff --git a/Source/WebKit/UIProcess/Cocoa/UIDelegate.mm b/Source/WebKit/UIProcess/Cocoa/UIDelegate.mm
@@ -310,7 +310,7 @@
     ASSERT(delegate);
 
     auto configuration = adoptNS([m_uiDelegate->m_webView.get()->_configuration copy]);
-    // FIXME: after calling triggerBrowsingContextGroupSwitchForNavigation this configuration's BrowsingContextGroup is incorrect.
+    // FIXME: after calling triggerBrowsingContextGroupSwitchForNavigation this configuration's BrowsingContextGroup is incorrect. <rdar://116203642>
     // The fix should go into platform-independent code, though. Needs some refactoring. And also needs switching browsing context group.
     [configuration _setRelatedWebView:m_uiDelegate->m_webView.get().get()];
 

diff --git a/Source/WebKit/UIProcess/RemotePageProxy.cpp b/Source/WebKit/UIProcess/RemotePageProxy.cpp
@@ -77,7 +77,7 @@ void RemotePageProxy::injectPageIntoNewProcess()
 
     auto parameters = page->creationParameters(m_process, *drawingArea);
     parameters.subframeProcessFrameTreeCreationParameters = page->frameTreeCreationParameters();
-    parameters.isProcessSwap = true; // FIXME: This should be a parameter to creationParameters rather than doctoring up the parameters afterwards.
+    parameters.isProcessSwap = true; // FIXME: This should be a parameter to creationParameters rather than doctoring up the parameters afterwards. <rdar://116201784>
     parameters.topContentInset = 0;
     m_process->send(Messages::WebProcess::CreateWebPage(m_webPageID, parameters), 0);
 }
@@ -92,12 +92,12 @@ RemotePageProxy::~RemotePageProxy()
 void RemotePageProxy::didReceiveMessage(IPC::Connection& connection, IPC::Decoder& decoder)
 {
 #if HAVE(VISIBILITY_PROPAGATION_VIEW)
-    // FIXME: This needs to be handled correctly in a way that doesn't cause assertions or crashes..
+    // FIXME: This needs to be handled correctly in a way that doesn't cause assertions or crashes. <rdar://116202187>
     if (decoder.messageName() == Messages::WebPageProxy::DidCreateContextInWebProcessForVisibilityPropagation::name())
         return;
 #endif
 
-    // FIXME: Removing this will be necessary to getting layout tests to work with site isolation.
+    // FIXME: Removing this will be necessary to getting layout tests to work with site isolation. <rdar://116202187>
     if (decoder.messageName() == Messages::WebPageProxy::HandleMessage::name())
         return;
 

diff --git a/Source/WebKit/UIProcess/WebFrameProxy.cpp b/Source/WebKit/UIProcess/WebFrameProxy.cpp
@@ -400,7 +400,7 @@ void WebFrameProxy::prepareForProvisionalNavigationInProcess(WebProcessProxy& pr
     }
 
     if (!m_provisionalFrame || navigation.currentRequestIsCrossSiteRedirect()) {
-        // FIXME: Main resource (of main or subframe) request redirects should go straight from the network to UI process so we don't need to make the processes for each domain in a redirect chain.
+        // FIXME: Main resource (of main or subframe) request redirects should go straight from the network to UI process so we don't need to make the processes for each domain in a redirect chain. <rdar://116202119>
         RegistrableDomain navigationDomain(navigation.currentRequest().url());
         RefPtr remotePageProxy = m_page->remotePageProxyForRegistrableDomain(navigationDomain);
         if (remotePageProxy)
@@ -411,7 +411,7 @@ void WebFrameProxy::prepareForProvisionalNavigationInProcess(WebProcessProxy& pr
         }
 
         m_provisionalFrame = makeUnique<ProvisionalFrameProxy>(*this, process, WTFMove(remotePageProxy));
-        // FIXME: This gives too much cookie access. This should be removed when a RemoteFrame is given a topOrigin member.
+        // FIXME: This gives too much cookie access. This should be removed when a RemoteFrame is given a topOrigin member. <rdar://116201929>
         auto giveAllCookieAccess = LoadedWebArchive::Yes;
         WebCore::RegistrableDomain domain { };
         page()->websiteDataStore().networkProcess().sendWithAsyncReply(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(process.coreProcessIdentifier(), domain, giveAllCookieAccess), WTFMove(completionHandler));

diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -4126,6 +4126,7 @@ void WebPageProxy::receivedNavigationPolicyDecision(WebProcessProxy& processNavi
         }
 
         if (loadContinuingInNonInitiatingProcess) {
+            // FIXME: Add more parameters as appropriate. <rdar://116200985>
             LoadParameters loadParameters;
             loadParameters.request = navigation->currentRequest();
             loadParameters.shouldTreatAsContinuingLoad = WebCore::ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;
@@ -4270,6 +4271,7 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, W
     bool isProcessSwappingOnNavigationResponse = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted;
     if (!frame.isMainFrame() && preferences().siteIsolationEnabled()) {
 
+        // FIXME: Add more parameters as appropriate. <rdar://116200985>
         LoadParameters loadParameters;
         loadParameters.request = navigation.currentRequest();
         loadParameters.shouldTreatAsContinuingLoad = navigation.currentRequestIsRedirect() ? WebCore::ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted : WebCore::ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;
@@ -12810,7 +12812,7 @@ void WebPageProxy::setRemotePageProxyInOpenerProcess(Ref<RemotePageProxy>&& page
     internals().remotePageProxyInOpenerProcess = WTFMove(page);
 }
 
-// FIXME: Add a corresponding remove call if the opened page is closed or destroyed.
+// FIXME: Add a corresponding remove call if the opened page is closed or destroyed. <rdar://111064432>
 void WebPageProxy::addOpenedRemotePageProxy(Ref<RemotePageProxy>&& page)
 {
     internals().openedRemotePageProxies.add(WTFMove(page));

diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
@@ -1941,7 +1941,7 @@ std::tuple<Ref<WebProcessProxy>, SuspendedPageProxy*, ASCIILiteral> WebProcessPo
     if (m_automationSession)
         return { WTFMove(sourceProcess), nullptr, "An automation session is active"_s };
 
-    // FIXME: We ought to be able to re-use processes that haven't committed anything with site isolation enabled, but cross-site redirects are tricky.
+    // FIXME: We ought to be able to re-use processes that haven't committed anything with site isolation enabled, but cross-site redirects are tricky. <rdar://116203552>
     if (!sourceProcess->hasCommittedAnyProvisionalLoads() && !page.preferences().siteIsolationEnabled()) {
         tryPrewarmWithDomainInformation(sourceProcess, targetRegistrableDomain);
         return { WTFMove(sourceProcess), nullptr, "Process has not yet committed any provisional loads"_s };

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
@@ -1059,7 +1059,7 @@ void WebChromeClient::attachViewOverlayGraphicsLayer(GraphicsLayer* graphicsLaye
     if (!drawingArea)
         return;
 
-    // FIXME: Support view overlays in iframe processes if needed.
+    // FIXME: Support view overlays in iframe processes if needed. <rdar://116202544>
     drawingArea->attachViewOverlayGraphicsLayer(page->mainWebFrame().frameID(), graphicsLayer);
 }
 

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
@@ -111,7 +111,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
     if (auto* webPage = requester.pageID ? WebProcess::singleton().webPage(*requester.pageID) : nullptr)
         originatingPageID = webPage->webPageProxyIdentifier();
 
-    // FIXME: Move all this DocumentLoader stuff to the caller, pass in the results.
+    // FIXME: Move all this DocumentLoader stuff to the caller, pass in the results. <rdar://116202776>
     RefPtr coreFrame = m_frame->coreLocalFrame();
 
     // FIXME: When we receive a redirect after the navigation policy has been decided for the initial request,

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp
@@ -71,14 +71,15 @@ void WebRemoteFrameClient::postMessageToRemote(WebCore::FrameIdentifier identifi
 
 void WebRemoteFrameClient::changeLocation(WebCore::FrameLoadRequest&& request)
 {
-    // FIXME: FrameLoadRequest and NavigationAction can probably be refactored to share more.
+    // FIXME: FrameLoadRequest and NavigationAction can probably be refactored to share more. <rdar://116202911>
     WebCore::NavigationAction action(request.requester(), request.resourceRequest(), request.initiatedByMainFrame());
-    // FIXME: action.request and request are probably duplicate information.
-    // FIXME: PolicyCheckIdentifier should probably be pushed to another layer.
-    // FIXME: Get more parameters correct and add tests for each one.
+    // FIXME: action.request and request are probably duplicate information. <rdar://116203126>
+    // FIXME: PolicyCheckIdentifier should probably be pushed to another layer. <rdar://116203008>
+    // FIXME: Get more parameters correct and add tests for each one. <rdar://116203354>
     dispatchDecidePolicyForNavigationAction(action, action.resourceRequest(), WebCore::ResourceResponse(), nullptr, WebCore::PolicyDecisionMode::Asynchronous, WebCore::PolicyCheckIdentifier::generate(), [protectedFrame = Ref { m_frame }, request = WTFMove(request)] (WebCore::PolicyAction policyAction, WebCore::PolicyCheckIdentifier responseIdentifier) mutable {
-        // FIXME: Check responseIdentifier.
+        // FIXME: Check responseIdentifier. <rdar://116203008>
         // WebPage::loadRequest will make this load happen if needed.
+        // FIXME: What if PolicyAction::Ignore is sent. Is everything in the right state? We probably need to make sure the load event still happens on the parent frame. <rdar://116203453>
     });
 }
 

diff --git a/Source/WebKit/WebProcess/WebPage/DrawingArea.h b/Source/WebKit/WebProcess/WebPage/DrawingArea.h
@@ -124,7 +124,7 @@ class DrawingArea : public IPC::MessageReceiver, public WebCore::DisplayRefreshM
     virtual WebCore::GraphicsLayerFactory* graphicsLayerFactory() { return nullptr; }
     virtual void setRootCompositingLayer(WebCore::Frame&, WebCore::GraphicsLayer*) = 0;
     virtual void addRootFrame(WebCore::FrameIdentifier) { }
-    // FIXME: Add a corresponding removeRootFrame.
+    // FIXME: Add a corresponding removeRootFrame. <rdar://116202445>
 
     // Cause a rendering update to happen as soon as possible.
     virtual void triggerRenderingUpdate() = 0;

diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
@@ -369,7 +369,7 @@
 
         backingStoreCollection.willCommitLayerTree(layerTransaction);
 
-        // FIXME: Investigate whether this needs to be done multiple times in a page with multiple root frames.
+        // FIXME: Investigate whether this needs to be done multiple times in a page with multiple root frames. <rdar://116202678>
         webPage->willCommitLayerTree(layerTransaction, rootLayer.frameID);
 
         layerTransaction.setNewlyReachedPaintingMilestones(std::exchange(m_pendingNewlyReachedPaintingMilestones, { }));

diff --git a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
@@ -121,7 +121,7 @@ static uint64_t generateListenerID()
     return uniqueListenerID++;
 }
 
-// FIXME: Remove receivedMainFrameIdentifierFromUIProcess in favor of a more correct way of sending frame tree deltas to each process.
+// FIXME: Remove receivedMainFrameIdentifierFromUIProcess in favor of a more correct way of sending frame tree deltas to each process. <rdar://116201135>
 void WebFrame::initWithCoreMainFrame(WebPage& page, Frame& coreFrame, bool receivedMainFrameIdentifierFromUIProcess)
 {
     if (!receivedMainFrameIdentifierFromUIProcess)
@@ -159,7 +159,7 @@ Ref<WebFrame> WebFrame::createRemoteSubframe(WebPage& page, WebFrame& parent, We
     auto coreFrame = RemoteFrame::createSubframe(*page.corePage(), WTFMove(client), frameID, *parent.coreFrame());
     frame->m_coreFrame = coreFrame.get();
 
-    // FIXME: Pass in a name and call FrameTree::setName here.
+    // FIXME: Pass in a name and call FrameTree::setName here. <rdar://116201307>
     return frame;
 }
 
@@ -367,7 +367,7 @@ void WebFrame::didCommitLoadInAnotherProcess(std::optional<WebCore::LayerHosting
     if (!parent)
         newFrame->takeWindowProxyFrom(*localFrame);
     auto remoteFrameView = WebCore::RemoteFrameView::create(newFrame);
-    // FIXME: We need a corresponding setView(nullptr) during teardown to break the ref cycle.
+    // FIXME: We need a corresponding setView(nullptr) during teardown to break the ref cycle. <rdar://116200737>
     newFrame->setView(remoteFrameView.ptr());
     if (ownerRenderer)
         ownerRenderer->setWidget(remoteFrameView.ptr());

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
@@ -796,7 +796,7 @@ WebPage::WebPage(PageIdentifier pageID, WebPageCreationParameters&& parameters)
     if (m_drawingArea->enterAcceleratedCompositingModeIfNeeded() && !parameters.isProcessSwap)
         m_drawingArea->sendEnterAcceleratedCompositingModeIfNeeded();
 #endif
-    // FIXME: Refactor frame construction and remove receivedMainFrameIdentifierFromUIProcess.
+    // FIXME: Refactor frame construction and remove receivedMainFrameIdentifierFromUIProcess. <rdar://116201135>
     if (!receivedMainFrameIdentifierFromUIProcess || parameters.openerFrameIdentifier)
         m_drawingArea->addRootFrame(m_mainFrame->frameID());
     m_drawingArea->setShouldScaleViewToFitDocument(parameters.shouldScaleViewToFitDocument);

diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
@@ -4209,7 +4209,7 @@ static void handleAnimationActions(Element& element, uint32_t action)
 
     auto* mainFrameView = this->localMainFrameView();
     if (!mainFrameView) {
-        // FIXME: This is hit in some site isolation tests on iOS. Investigate and fix.
+        // FIXME: This is hit in some site isolation tests on iOS. Investigate and fix. <rdar://116201382>
         return;
     }
 
@@ -4652,7 +4652,7 @@ static bool selectionIsInsideFixedPositionContainer(LocalFrame& frame)
 
     auto* mainFrame = m_mainFrame->coreLocalFrame();
     if (!mainFrame) {
-        // FIXME: Add a user agent for loads from iframe processes.
+        // FIXME: Add a user agent for loads from iframe processes. <rdar://116201535>
         return { };
     }