Attachment 350149 Details for Bug 189765 – Patch

[patch] Patch

bug-189765-20180919140550.patch (text/plain), 10.24 KB, created by Alex Christensen on 2018-09-19 14:05:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-09-19 14:05:51 PDT

Size: 10.24 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 236223)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,32 @@
>+2018-09-19  Alex Christensen  <achristensen@webkit.org>
>+
>+        Don't crash if we get a null navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=189765
>+        <rdar://problem/19373509>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        It happens sometimes.  That's unfortunate, but we shouldn't crash.
>+
>+        * UIProcess/WebNavigationState.cpp:
>+        (WebKit::WebNavigationState::createLoadRequestNavigation):
>+        (WebKit::WebNavigationState::createBackForwardNavigation):
>+        (WebKit::WebNavigationState::createReloadNavigation):
>+        (WebKit::WebNavigationState::createLoadDataNavigation):
>+        (WebKit::WebNavigationState::navigation):
>+        (WebKit::WebNavigationState::takeNavigation):
>+        * UIProcess/WebNavigationState.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::didStartProvisionalLoadForFrame):
>+        (WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
>+        (WebKit::WebPageProxy::didCommitLoadForFrame):
>+        (WebKit::WebPageProxy::didFinishDocumentLoadForFrame):
>+        (WebKit::WebPageProxy::didFinishLoadForFrame):
>+        (WebKit::WebPageProxy::didFailLoadForFrame):
>+        (WebKit::WebPageProxy::didSameDocumentNavigationForFrame):
>+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
>+        (WebKit::WebPageProxy::decidePolicyForResponse):
>+
> 2018-09-19  John Wilander  <wilander@apple.com>
> 
>         Resource Load Statistics: Add optional cap on partitioned cache max age
>Index: Source/WebKit/UIProcess/WebNavigationState.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebNavigationState.cpp	(revision 236150)
>+++ Source/WebKit/UIProcess/WebNavigationState.cpp	(working copy)
>@@ -45,7 +45,7 @@ Ref<API::Navigation> WebNavigationState:
> {
>     auto navigation = API::Navigation::create(*this, WTFMove(request), currentItem);
> 
>-    m_navigations.set(navigation->navigationID(), navigation.ptr());
>+    m_navigations.set(navigation->navigationID(), WTFMove(navigation));
> 
>     return navigation;
> }
>@@ -54,7 +54,7 @@ Ref<API::Navigation> WebNavigationState:
> {
>     auto navigation = API::Navigation::create(*this, targetItem, currentItem, frameLoadType);
> 
>-    m_navigations.set(navigation->navigationID(), navigation.ptr());
>+    m_navigations.set(navigation->navigationID(), WTFMove(navigation));
> 
>     return navigation;
> }
>@@ -63,7 +63,7 @@ Ref<API::Navigation> WebNavigationState:
> {
>     auto navigation = API::Navigation::create(*this);
> 
>-    m_navigations.set(navigation->navigationID(), navigation.ptr());
>+    m_navigations.set(navigation->navigationID(), WTFMove(navigation));
> 
>     return navigation;
> }
>@@ -72,25 +72,25 @@ Ref<API::Navigation> WebNavigationState:
> {
>     auto navigation = API::Navigation::create(*this);
> 
>-    m_navigations.set(navigation->navigationID(), navigation.ptr());
>+    m_navigations.set(navigation->navigationID(), WTFMove(navigation));
> 
>     return navigation;
> }
> 
>-API::Navigation& WebNavigationState::navigation(uint64_t navigationID)
>+API::Navigation* WebNavigationState::navigation(uint64_t navigationID)
> {
>     ASSERT(navigationID);
>     ASSERT(m_navigations.contains(navigationID));
> 
>-    return *m_navigations.get(navigationID);
>+    return m_navigations.get(navigationID);
> }
> 
>-Ref<API::Navigation> WebNavigationState::takeNavigation(uint64_t navigationID)
>+RefPtr<API::Navigation> WebNavigationState::takeNavigation(uint64_t navigationID)
> {
>     ASSERT(navigationID);
>     ASSERT(m_navigations.contains(navigationID));
>     
>-    return m_navigations.take(navigationID).releaseNonNull();
>+    return m_navigations.take(navigationID);
> }
> 
> void WebNavigationState::didDestroyNavigation(uint64_t navigationID)
>Index: Source/WebKit/UIProcess/WebNavigationState.h
>===================================================================
>--- Source/WebKit/UIProcess/WebNavigationState.h	(revision 236150)
>+++ Source/WebKit/UIProcess/WebNavigationState.h	(working copy)
>@@ -53,8 +53,8 @@ public:
>     Ref<API::Navigation> createReloadNavigation();
>     Ref<API::Navigation> createLoadDataNavigation();
> 
>-    API::Navigation& navigation(uint64_t navigationID);
>-    Ref<API::Navigation> takeNavigation(uint64_t navigationID);
>+    API::Navigation* navigation(uint64_t navigationID);
>+    RefPtr<API::Navigation> takeNavigation(uint64_t navigationID);
>     void didDestroyNavigation(uint64_t navigationID);
>     void clearAllNavigations();
> 
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 236154)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -3460,7 +3460,7 @@ void WebPageProxy::didStartProvisionalLo
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (frame->isMainFrame() && navigationID)
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
> 
>     // If this seemingly new load is actually continuing a server redirect for a previous navigation in a new process,
>     // then we ignore this notification.
>@@ -3507,7 +3507,7 @@ void WebPageProxy::didReceiveServerRedir
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (navigationID) {
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
>         navigation->appendRedirectionURL(request.url());
>     }
> 
>@@ -3634,7 +3634,7 @@ void WebPageProxy::didCommitLoadForFrame
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (frame->isMainFrame() && navigationID)
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
> 
>     m_hasCommittedAnyProvisionalLoads = true;
>     m_process->didCommitProvisionalLoad();
>@@ -3726,7 +3726,7 @@ void WebPageProxy::didFinishDocumentLoad
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (frame->isMainFrame() && navigationID)
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
> 
>     if (frame->isMainFrame())
>         m_navigationClient->didFinishDocumentLoad(*this, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
>@@ -3744,7 +3744,7 @@ void WebPageProxy::didFinishLoadForFrame
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (frame->isMainFrame() && navigationID)
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
> 
>     auto transaction = m_pageLoadState.transaction();
> 
>@@ -3787,7 +3787,7 @@ void WebPageProxy::didFailLoadForFrame(u
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (frame->isMainFrame() && navigationID)
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
> 
>     clearLoadDependentCallbacks();
> 
>@@ -3828,7 +3828,7 @@ void WebPageProxy::didSameDocumentNaviga
>     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
>     RefPtr<API::Navigation> navigation;
>     if (frame->isMainFrame() && navigationID)
>-        navigation = &navigationState().navigation(navigationID);
>+        navigation = navigationState().navigation(navigationID);
> 
>     auto transaction = m_pageLoadState.transaction();
> 
>@@ -4002,7 +4002,7 @@ void WebPageProxy::decidePolicyForNaviga
> 
>     RefPtr<API::Navigation> navigation;
>     if (navigationID)
>-        navigation = makeRef(m_navigationState->navigation(navigationID));
>+        navigation = m_navigationState->navigation(navigationID);
> 
>     if (auto targetBackForwardItemIdentifier = navigationActionData.targetBackForwardItemIdentifier) {
>         if (auto* item = m_backForwardList->itemForID(*navigationActionData.targetBackForwardItemIdentifier)) {
>@@ -4034,7 +4034,7 @@ void WebPageProxy::decidePolicyForNaviga
> 
> #if ENABLE(CONTENT_FILTERING)
>     if (frame->didHandleContentFilterUnblockNavigation(request))
>-        return receivedPolicyDecision(PolicyAction::Ignore, &m_navigationState->navigation(newNavigationID), std::nullopt, WTFMove(sender));
>+        return receivedPolicyDecision(PolicyAction::Ignore, m_navigationState->navigation(newNavigationID), std::nullopt, WTFMove(sender));
> #else
>     UNUSED_PARAM(newNavigationID);
> #endif
>@@ -4128,7 +4128,7 @@ void WebPageProxy::decidePolicyForRespon
>     MESSAGE_CHECK_URL(request.url());
>     MESSAGE_CHECK_URL(response.url());
> 
>-    RefPtr<API::Navigation> navigation = navigationID ? &m_navigationState->navigation(navigationID) : nullptr;
>+    RefPtr<API::Navigation> navigation = navigationID ? m_navigationState->navigation(navigationID) : nullptr;
>     auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frameID, listenerID, navigation = WTFMove(navigation)] (WebCore::PolicyAction policyAction, API::WebsitePolicies*, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&& safeBrowsingResults) mutable {
>         // FIXME: Assert the API::WebsitePolicies* is nullptr here once clients of WKFramePolicyListenerUseWithPolicies go away.
>         RELEASE_ASSERT(processSwapRequestedByClient == ProcessSwapRequestedByClient::No);

Actions: View | Formatted Diff | Diff

Attachments on bug 189765: 350149