Release assert in PolicyCheckIdentifier::isValidFor via WebFrameLoaderClient::dispatc...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Feb 2019 09:01:49 +0000 (09:01 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Feb 2019 09:01:49 +0000 (09:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194582

Reviewed by Antti Koivisto.

Source/WebCore:

Check the zero-ness of m_policyCheck first so that we can differentiate process ID being wrong
from the non-generated identifier being sent to us as it was the case in this failure.

* loader/PolicyChecker.cpp:
(WebCore::PolicyCheckIdentifier::isValidFor):

Source/WebKit:

The bug was caused by WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction invoking the callback
with responseIdentifier even when we had failed to send the policy check IPC. Clearly, responseIdentifier
is invalid in that case, and we should be using requestIdentifier instead.

Unfortunately no new tests since I'm not aware of a way to make sendSync fail in this case.

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

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

Source/WebCore/ChangeLog
Source/WebCore/loader/PolicyChecker.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

index 63878be..e29e6ac 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Release assert in PolicyCheckIdentifier::isValidFor via WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction
+        https://bugs.webkit.org/show_bug.cgi?id=194582
+
+        Reviewed by Antti Koivisto.
+
+        Check the zero-ness of m_policyCheck first so that we can differentiate process ID being wrong
+        from the non-generated identifier being sent to us as it was the case in this failure.
+
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyCheckIdentifier::isValidFor):
+
 2019-02-13  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r241273.
index 2ee54f5..d591e01 100644 (file)
@@ -80,8 +80,8 @@ PolicyCheckIdentifier PolicyCheckIdentifier::create()
 
 bool PolicyCheckIdentifier::isValidFor(PolicyCheckIdentifier expectedIdentifier)
 {
-    RELEASE_ASSERT_WITH_MESSAGE(m_process == expectedIdentifier.m_process, "Received a policy check response for a wrong process");
     RELEASE_ASSERT_WITH_MESSAGE(m_policyCheck, "Received 0 as the policy check identifier");
+    RELEASE_ASSERT_WITH_MESSAGE(m_process == expectedIdentifier.m_process, "Received a policy check response for a wrong process");
     RELEASE_ASSERT_WITH_MESSAGE(m_policyCheck <= expectedIdentifier.m_policyCheck, "Received a policy check response from the future");
     return m_policyCheck == expectedIdentifier.m_policyCheck;
 }
index a691dda..0f75ae4 100644 (file)
@@ -1,3 +1,19 @@
+2019-02-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Release assert in PolicyCheckIdentifier::isValidFor via WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction
+        https://bugs.webkit.org/show_bug.cgi?id=194582
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction invoking the callback
+        with responseIdentifier even when we had failed to send the policy check IPC. Clearly, responseIdentifier
+        is invalid in that case, and we should be using requestIdentifier instead.
+
+        Unfortunately no new tests since I'm not aware of a way to make sendSync fail in this case.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+
 2019-02-13  Benjamin Poulain  <benjamin@webkit.org>
 
         Responsiveness timers are too expensive for frequent events
index 938cadd..9a02318 100644 (file)
@@ -911,7 +911,7 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
             requestIdentifier, documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request,
             IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())),
             Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(responseIdentifier, policyAction, newNavigationID, downloadID, websitePolicies))) {
-            m_frame->didReceivePolicyDecision(listenerID, responseIdentifier, PolicyAction::Ignore, 0, { }, { });
+            m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyAction::Ignore, 0, { }, { });
             return;
         }