Make WebFrameLoaderClient more robust against null pointer dereferencing
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Nov 2017 21:41:59 +0000 (21:41 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Nov 2017 21:41:59 +0000 (21:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180157
<rdar://problem/34895616>

Reviewed by Tim Horton.

There has always been rare null pointer crashes in this code, but they have become more common
now that we are waiting for completion handlers for redirects, which makes it more likely that
we are hitting this code after we have detached from the core frame.

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::page const):

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

Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit/WebProcess/WebPage/WebFrame.cpp

index 3b2cec3..1f6a23b 100644 (file)
@@ -1,5 +1,24 @@
 2017-11-29  Alex Christensen  <achristensen@webkit.org>
 
+        Make WebFrameLoaderClient more robust against null pointer dereferencing
+        https://bugs.webkit.org/show_bug.cgi?id=180157
+        <rdar://problem/34895616>
+
+        Reviewed by Tim Horton.
+
+        There has always been rare null pointer crashes in this code, but they have become more common
+        now that we are waiting for completion handlers for redirects, which makes it more likely that
+        we are hitting this code after we have detached from the core frame.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        * WebProcess/WebPage/WebFrame.cpp:
+        (WebKit::WebFrame::page const):
+
+2017-11-29  Alex Christensen  <achristensen@webkit.org>
+
         Fix Mac CMake build.
 
         * PlatformMac.cmake:
index 33e6712..7866a77 100644 (file)
@@ -692,7 +692,7 @@ void WebFrameLoaderClient::dispatchShow()
 
 void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceResponse& response, const ResourceRequest& request, FramePolicyFunction&& function)
 {
-    WebPage* webPage = m_frame->page();
+    WebPage* webPage = m_frame ? m_frame->page() : nullptr;
     if (!webPage) {
         function(PolicyAction::Ignore);
         return;
@@ -721,6 +721,8 @@ void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceRespons
 
     Ref<WebFrame> protect(*m_frame);
     WebCore::Frame* coreFrame = m_frame->coreFrame();
+    if (!coreFrame)
+        return function(PolicyAction::Ignore);
     auto navigationID = static_cast<WebDocumentLoader&>(*coreFrame->loader().provisionalDocumentLoader()).navigationID();
     if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForResponseSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), navigationID, response, request, canShowMIMEType, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForResponseSync::Reply(receivedPolicyAction, policyAction, downloadID), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend)) {
         m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
@@ -734,7 +736,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceRespons
 
 void WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction(const NavigationAction& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, FramePolicyFunction&& function)
 {
-    WebPage* webPage = m_frame->page();
+    WebPage* webPage = m_frame ? m_frame->page() : nullptr;
     if (!webPage) {
         function(PolicyAction::Ignore);
         return;
@@ -821,7 +823,7 @@ void WebFrameLoaderClient::applyToDocumentLoader(WebsitePolicies&& websitePolici
 
 void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, bool didReceiveRedirectResponse, FormState* formState, FramePolicyFunction&& function)
 {
-    WebPage* webPage = m_frame->page();
+    WebPage* webPage = m_frame ? m_frame->page() : nullptr;
     if (!webPage) {
         function(PolicyAction::Ignore);
         return;
@@ -835,10 +837,10 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
 
     RefPtr<API::Object> userData;
 
-    RefPtr<InjectedBundleNavigationAction> action = InjectedBundleNavigationAction::create(m_frame, navigationAction, formState);
+    Ref<InjectedBundleNavigationAction> action = InjectedBundleNavigationAction::create(m_frame, navigationAction, formState);
 
     // Notify the bundle client.
-    WKBundlePagePolicyAction policy = webPage->injectedBundlePolicyClient().decidePolicyForNavigationAction(webPage, m_frame, action.get(), request, userData);
+    WKBundlePagePolicyAction policy = webPage->injectedBundlePolicyClient().decidePolicyForNavigationAction(webPage, m_frame, action.ptr(), request, userData);
     if (policy == WKBundlePagePolicyActionUse) {
         function(PolicyAction::Use);
         return;
@@ -874,6 +876,8 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
     navigationActionData.isRedirect = didReceiveRedirectResponse;
 
     WebCore::Frame* coreFrame = m_frame->coreFrame();
+    if (!coreFrame)
+        return function(PolicyAction::Ignore);
     WebDocumentLoader* documentLoader = static_cast<WebDocumentLoader*>(coreFrame->loader().policyDocumentLoader());
     if (!documentLoader) {
         // FIXME: When we receive a redirect after the navigation policy has been decided for the initial request,
index acbbfad..c3c1549 100644 (file)
@@ -171,14 +171,14 @@ WebFrame::~WebFrame()
 }
 
 WebPage* WebFrame::page() const
-{ 
+{
     if (!m_coreFrame)
-        return 0;
+        return nullptr;
     
     if (Page* page = m_coreFrame->page())
         return WebPage::fromCorePage(page);
 
-    return 0;
+    return nullptr;
 }
 
 WebFrame* WebFrame::fromCoreFrame(Frame& frame)