[webkit-changes] [259727] trunk/Source/WebKit

Wed, 08 Apr 2020 10:31:27 -0700

Title: [259727] trunk/Source/WebKit
Revision
259727
Author
[email protected]
Date
2020-04-08 10:30:21 -0700 (Wed, 08 Apr 2020)

Log Message

REGRESSION(r253346): some Automation commands targeted at an iframe do not return
https://bugs.webkit.org/show_bug.cgi?id=210139
<rdar://problem/60561009>

Reviewed by Devin Rousso.

WebAutomationSession / WebAutomationSessionProxy are singletons, and only one
exists at a time in the UIProcess / WebProcess respectively. A recent change was
made to call Connection::sendWithAsyncReply from the WebPageProxy object rather
than WebProcess. The effect of this is that the WebPage's destinationID is
used for the response message rather than the WebProcess.

Because WebAutomationSessionProxy registers itself as a global IPC message receiver,
the page-targeted destinationID cannot find any receivers for the message at the
destinationID, which causes an ASSERT in debug builds and a hang in non-debug builds.

The fix is to continue sending messages via the WebProcess object, whose messages
are tagged with a destinationID of 0 (eg, global message receiver). This could alternatively
be accomplished by passing a destinationID of 0 to every sendWithAsyncReply, but
as is this change is a straightforward partial revert of r253346.

* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::switchToBrowsingContext):
(WebKit::WebAutomationSession::evaluateJavaScriptFunction):
(WebKit::WebAutomationSession::resolveChildFrameHandle):
(WebKit::WebAutomationSession::resolveParentFrameHandle):
(WebKit::WebAutomationSession::computeElementLayout):
(WebKit::WebAutomationSession::selectOptionElement):
(WebKit::WebAutomationSession::setFilesForInputFileUpload):
(WebKit::WebAutomationSession::getAllCookies):
(WebKit::WebAutomationSession::deleteSingleCookie):
(WebKit::WebAutomationSession::takeScreenshot):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (259726 => 259727)


--- trunk/Source/WebKit/ChangeLog	2020-04-08 17:25:25 UTC (rev 259726)
+++ trunk/Source/WebKit/ChangeLog	2020-04-08 17:30:21 UTC (rev 259727)
@@ -1,3 +1,38 @@
+2020-04-08  Brian Burg  <[email protected]>
+
+        REGRESSION(r253346): some Automation commands targeted at an iframe do not return
+        https://bugs.webkit.org/show_bug.cgi?id=210139
+        <rdar://problem/60561009>
+
+        Reviewed by Devin Rousso.
+
+        WebAutomationSession / WebAutomationSessionProxy are singletons, and only one
+        exists at a time in the UIProcess / WebProcess respectively. A recent change was
+        made to call Connection::sendWithAsyncReply from the WebPageProxy object rather
+        than WebProcess. The effect of this is that the WebPage's destinationID is
+        used for the response message rather than the WebProcess.
+
+        Because WebAutomationSessionProxy registers itself as a global IPC message receiver,
+        the page-targeted destinationID cannot find any receivers for the message at the
+        destinationID, which causes an ASSERT in debug builds and a hang in non-debug builds.
+
+        The fix is to continue sending messages via the WebProcess object, whose messages
+        are tagged with a destinationID of 0 (eg, global message receiver). This could alternatively
+        be accomplished by passing a destinationID of 0 to every sendWithAsyncReply, but
+        as is this change is a straightforward partial revert of r253346.
+
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::WebAutomationSession::switchToBrowsingContext):
+        (WebKit::WebAutomationSession::evaluateJavaScriptFunction):
+        (WebKit::WebAutomationSession::resolveChildFrameHandle):
+        (WebKit::WebAutomationSession::resolveParentFrameHandle):
+        (WebKit::WebAutomationSession::computeElementLayout):
+        (WebKit::WebAutomationSession::selectOptionElement):
+        (WebKit::WebAutomationSession::setFilesForInputFileUpload):
+        (WebKit::WebAutomationSession::getAllCookies):
+        (WebKit::WebAutomationSession::deleteSingleCookie):
+        (WebKit::WebAutomationSession::takeScreenshot):
+
 2020-04-08  Truitt Savell  <[email protected]>
 
         Unreviewed, reverting r259708.

Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp (259726 => 259727)


--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp	2020-04-08 17:25:25 UTC (rev 259726)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp	2020-04-08 17:30:21 UTC (rev 259727)
@@ -375,7 +375,7 @@
 
     m_client->requestSwitchToPage(*this, *page, [frameID, page = makeRef(*page), callback = WTFMove(callback)]() {
         page->setFocus(true);
-        page->send(Messages::WebAutomationSessionProxy::FocusFrame(page->webPageID(), frameID), 0);
+        page->process().send(Messages::WebAutomationSessionProxy::FocusFrame(page->webPageID(), frameID), 0);
 
         callback->sendSuccess();
     });
@@ -947,7 +947,7 @@
     uint64_t callbackID = m_nextEvaluateJavaScriptCallbackID++;
     m_evaluateJavaScriptFunctionCallbacks.set(callbackID, WTFMove(callback));
 
-    page->send(Messages::WebAutomationSessionProxy::EvaluateJavaScriptFunction(page->webPageID(), frameID, function, argumentsVector, expectsImplicitCallbackArgument, callbackTimeout, callbackID), 0);
+    page->process().send(Messages::WebAutomationSessionProxy::EvaluateJavaScriptFunction(page->webPageID(), frameID, function, argumentsVector, expectsImplicitCallbackArgument, callbackTimeout, callbackID), 0);
 }
 
 void WebAutomationSession::didEvaluateJavaScriptFunction(uint64_t callbackID, const String& result, const String& errorType)
@@ -986,17 +986,17 @@
     };
 
     if (optionalNodeHandle) {
-        page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithNodeHandle(page->webPageID(), frameID, *optionalNodeHandle), WTFMove(completionHandler));
+        page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithNodeHandle(page->webPageID(), frameID, *optionalNodeHandle), WTFMove(completionHandler));
         return;
     }
 
     if (optionalName) {
-        page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithName(page->webPageID(), frameID, *optionalName), WTFMove(completionHandler));
+        page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithName(page->webPageID(), frameID, *optionalName), WTFMove(completionHandler));
         return;
     }
 
     if (optionalOrdinal) {
-        page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithOrdinal(page->webPageID(), frameID, *optionalOrdinal), WTFMove(completionHandler));
+        page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithOrdinal(page->webPageID(), frameID, *optionalOrdinal), WTFMove(completionHandler));
         return;
     }
 
@@ -1023,7 +1023,7 @@
         callback->sendSuccess(handleForWebFrameID(frameID));
     };
 
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveParentFrame(page->webPageID(), frameID), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveParentFrame(page->webPageID(), frameID), WTFMove(completionHandler));
 }
 
 static Optional<CoordinateSystem> protocolStringToCoordinateSystem(const String& coordinateSystemString)
@@ -1085,7 +1085,7 @@
     };
 
     bool scrollIntoViewIfNeeded = optionalScrollIntoViewIfNeeded ? *optionalScrollIntoViewIfNeeded : false;
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::ComputeElementLayout(page->webPageID(), frameID, nodeHandle, scrollIntoViewIfNeeded, coordinateSystem.value()), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ComputeElementLayout(page->webPageID(), frameID, nodeHandle, scrollIntoViewIfNeeded, coordinateSystem.value()), WTFMove(completionHandler));
 }
 
 void WebAutomationSession::selectOptionElement(const String& browsingContextHandle, const String& frameHandle, const String& nodeHandle, Ref<SelectOptionElementCallback>&& callback)
@@ -1108,7 +1108,7 @@
         callback->sendSuccess();
     };
 
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::SelectOptionElement(page->webPageID(), frameID, nodeHandle), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::SelectOptionElement(page->webPageID(), frameID, nodeHandle), WTFMove(completionHandler));
 }
 
 void WebAutomationSession::isShowingJavaScriptDialog(Inspector::ErrorString& errorString, const String& browsingContextHandle, bool* result)
@@ -1268,7 +1268,7 @@
         callback->sendSuccess();
     };
 
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::SetFilesForInputFileUpload(page->webPageID(), frameID, nodeHandle, WTFMove(newFileList)), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::SetFilesForInputFileUpload(page->webPageID(), frameID, nodeHandle, WTFMove(newFileList)), WTFMove(completionHandler));
 }
 
 static Ref<Inspector::Protocol::Automation::Cookie> buildObjectForCookie(const WebCore::Cookie& cookie)
@@ -1311,7 +1311,7 @@
         callback->sendSuccess(buildArrayForCookies(cookies));
     };
 
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::GetCookiesForFrame(page->webPageID(), WTF::nullopt), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::GetCookiesForFrame(page->webPageID(), WTF::nullopt), WTFMove(completionHandler));
 }
 
 void WebAutomationSession::deleteSingleCookie(const String& browsingContextHandle, const String& cookieName, Ref<DeleteSingleCookieCallback>&& callback)
@@ -1329,7 +1329,7 @@
         callback->sendSuccess();
     };
 
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::DeleteCookie(page->webPageID(), WTF::nullopt, cookieName), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::DeleteCookie(page->webPageID(), WTF::nullopt, cookieName), WTFMove(completionHandler));
 }
 
 static String domainByAddingDotPrefixIfNeeded(String domain)
@@ -2036,12 +2036,12 @@
         takeViewSnapsot(page.get(), WTFMove(rect), WTFMove(callback));
     };
 
-    page->sendWithAsyncReply(Messages::WebAutomationSessionProxy::SnapshotRectForScreenshot(page->webPageID(), frameID, nodeHandle, scrollIntoViewIfNeeded, clipToViewport), WTFMove(completionHandler));
+    page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::SnapshotRectForScreenshot(page->webPageID(), frameID, nodeHandle, scrollIntoViewIfNeeded, clipToViewport), WTFMove(completionHandler));
 #else
     uint64_t callbackID = m_nextScreenshotCallbackID++;
     m_screenshotCallbacks.set(callbackID, WTFMove(callback));
 
-    page->send(Messages::WebAutomationSessionProxy::TakeScreenshot(page->webPageID(), frameID, nodeHandle, scrollIntoViewIfNeeded, clipToViewport, callbackID), 0);
+    page->process().send(Messages::WebAutomationSessionProxy::TakeScreenshot(page->webPageID(), frameID, nodeHandle, scrollIntoViewIfNeeded, clipToViewport, callbackID), 0);
 #endif
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to