[webkit-changes] [WebKit/WebKit] 9ae6f7: Make sure we don't leak API::Navigation objects

Chris Dumez noreply at github.com
Tue May 23 19:17:58 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 9ae6f745cc3d47fd6804e26af87ecedf62bd4109
      https://github.com/WebKit/WebKit/commit/9ae6f745cc3d47fd6804e26af87ecedf62bd4109
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-05-23 (Tue, 23 May 2023)

  Changed paths:
    M Source/WebKit/UIProcess/API/APINavigation.cpp
    M Source/WebKit/UIProcess/API/APINavigation.h
    M Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
    M Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
    M Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
    M Source/WebKit/UIProcess/ProvisionalPageProxy.h
    M Source/WebKit/UIProcess/SuspendedPageProxy.cpp
    M Source/WebKit/UIProcess/SuspendedPageProxy.h
    M Source/WebKit/UIProcess/WebNavigationState.cpp
    M Source/WebKit/UIProcess/WebNavigationState.h
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/WebProcessProxy.cpp
    M Source/WebKit/UIProcess/WebProcessProxy.h
    M Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp
    M Source/WebKit/WebProcess/WebPage/WebDocumentLoader.h
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm

  Log Message:
  -----------
  Make sure we don't leak API::Navigation objects
https://bugs.webkit.org/show_bug.cgi?id=256900
rdar://109430054

Reviewed by Ben Nham.

The lifetime of the API::Navigation objects in the UIProcess is tied to the one
of the WebDocumentLoaders in the WebProcess. The API::Navigation object get
stored on the WebNavigationState::m_navigations HashMap upon creation, keeping
them alive. When the WebDocumentLoader gets detached from its frame, we send a
`DidDestroyNavigation()` IPC to the UIProcess so that we remove the
corresponding navigation from WebNavigationState::m_navigations and stop
extending the API::Navigation lifetime.

The reason the lifetime of navigations is tied to WebDocumentLoaders is that
the original API::Navigation object is reused for some follow-up navigations,
such as same-document navigations.

Since we've added support for process-swapping, this has become leak-prone.
The reason is that a WebPageProxy (and thus a WebNavigationState) is no longer
tied to a single WebProcess. Also, A navigation may start in process A and
then finish in process B.

To avoid leaking, I am adding a ProcessIdentifier data member to API navigation
which is kept up-to-date so that we know which WebProcess currently owns this
object. When a process gets disassociated with a page, we also clear all the
page's navigations that are owned by this process.

* Source/WebKit/UIProcess/API/APINavigation.cpp:
(API::Navigation::Navigation):
* Source/WebKit/UIProcess/API/APINavigation.h:
(API::Navigation::create):
(API::Navigation::processID const):
(API::Navigation::setProcessID):
* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
(WebKit::ProvisionalPageProxy::didDestroyNavigation):
(WebKit::ProvisionalPageProxy::didReceiveMessage):
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::messageNamesToIgnoreWhileSuspended):
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
(WebKit::SuspendedPageProxy::~SuspendedPageProxy):
(WebKit::SuspendedPageProxy::didDestroyNavigation):
(WebKit::SuspendedPageProxy::didReceiveMessage):
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebNavigationState.cpp:
(WebKit::WebNavigationState::createLoadRequestNavigation):
(WebKit::WebNavigationState::createBackForwardNavigation):
(WebKit::WebNavigationState::createReloadNavigation):
(WebKit::WebNavigationState::createLoadDataNavigation):
(WebKit::WebNavigationState::createSimulatedLoadWithDataNavigation):
(WebKit::WebNavigationState::didDestroyNavigation):
(WebKit::WebNavigationState::clearNavigationsFromProcess):
* Source/WebKit/UIProcess/WebNavigationState.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::launchProcessForReload):
(WebKit::WebPageProxy::loadRequest):
(WebKit::WebPageProxy::loadFile):
(WebKit::WebPageProxy::loadData):
(WebKit::WebPageProxy::loadSimulatedRequest):
(WebKit::WebPageProxy::reload):
(WebKit::WebPageProxy::goToBackForwardItem):
(WebKit::WebPageProxy::continueNavigationInNewProcess):
(WebKit::WebPageProxy::didDestroyNavigation):
(WebKit::WebPageProxy::didDestroyNavigationShared):
(WebKit::WebPageProxy::didSameDocumentNavigationForFrameViaJSHistoryAPI):
(WebKit::WebPageProxy::processIsNoLongerAssociatedWithPage):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::setIsInProcessCache):
(WebKit::WebProcessProxy::removeProvisionalPageProxy):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
(WebKit::WebProcessProxy::addSuspendedPageProxy):
(WebKit::WebProcessProxy::removeSuspendedPageProxy):
(WebKit::WebProcessProxy::reportProcessDisassociatedWithPageIfNecessary):
(WebKit::WebProcessProxy::isAssociatedWithPage const):
(WebKit::WebProcessProxy::incrementSuspendedPageCount): Deleted.
(WebKit::WebProcessProxy::decrementSuspendedPageCount): Deleted.
* Source/WebKit/UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::suspendedPageCount const):
* Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp:
(WebKit::WebDocumentLoader::WebDocumentLoader):
(WebKit::WebDocumentLoader::~WebDocumentLoader):
* Source/WebKit/WebProcess/WebPage/WebDocumentLoader.h:

Canonical link: https://commits.webkit.org/264449@main




More information about the webkit-changes mailing list