Skip to content

Commit

Permalink
Web Automation: clean up some WebAutomationSession methods to use mod…
Browse files Browse the repository at this point in the history 
…ern async IPC

https://bugs.webkit.org/show_bug.cgi?id=196168

Reviewed by Devin Rousso.

Modern WebKit IPC is capable of providing completion handlers and can track callback IDs.
So, most messages between WebAutomationSession and its proxy can use this facility and stop
keeping track of callback IDs manually. This makes most code easier to read on both the
sender and receiver side.

There are two cases that could not be converted:
- For evaluateJavaScript, we cannot use async IPC because WebAutomationSession expects to
be able to cancel all pending replies when a page navigates away, the web process crashes,
or when handling an alert.
- For takeScreenshot, there is not currently support in the modern async IPC code paths for
sending the result back. ShareableBitmap and friends lack a modern decoder implementation.

* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::resolveChildFrameHandle):
(WebKit::WebAutomationSession::resolveParentFrameHandle):
(WebKit::WebAutomationSession::computeElementLayout):
(WebKit::WebAutomationSession::selectOptionElement):
(WebKit::WebAutomationSession::getAllCookies):
(WebKit::WebAutomationSession::deleteSingleCookie):
(WebKit::WebAutomationSession::viewportInViewCenterPointOfElement):
(WebKit::WebAutomationSession::didResolveChildFrame): Deleted.
(WebKit::WebAutomationSession::didResolveParentFrame): Deleted.
(WebKit::WebAutomationSession::didComputeElementLayout): Deleted.
(WebKit::WebAutomationSession::didSelectOptionElement): Deleted.
(WebKit::WebAutomationSession::didGetCookiesForFrame): Deleted.
(WebKit::WebAutomationSession::didDeleteCookie): Deleted.
* UIProcess/Automation/WebAutomationSession.h:
* UIProcess/Automation/WebAutomationSession.messages.in:
* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::WebAutomationSessionProxy::resolveChildFrameWithOrdinal):
(WebKit::WebAutomationSessionProxy::resolveChildFrameWithNodeHandle):
(WebKit::WebAutomationSessionProxy::resolveChildFrameWithName):
(WebKit::WebAutomationSessionProxy::resolveParentFrame):
(WebKit::WebAutomationSessionProxy::computeElementLayout):
(WebKit::WebAutomationSessionProxy::selectOptionElement):
(WebKit::WebAutomationSessionProxy::getCookiesForFrame):
(WebKit::WebAutomationSessionProxy::deleteCookie):
* WebProcess/Automation/WebAutomationSessionProxy.h:
* WebProcess/Automation/WebAutomationSessionProxy.messages.in:

Canonical link: https://commits.webkit.org/210972@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244033 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@burg
burg committed Apr 8, 2019
1 parent 71504f5 commit 8525cc0
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 250 deletions.
47 changes: 47 additions & 0 deletions Source/WebKit/ChangeLog
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,50 @@
2019-04-05 Brian Burg <bburg@apple.com>

Web Automation: clean up some WebAutomationSession methods to use modern async IPC
https://bugs.webkit.org/show_bug.cgi?id=196168

Reviewed by Devin Rousso.

Modern WebKit IPC is capable of providing completion handlers and can track callback IDs.
So, most messages between WebAutomationSession and its proxy can use this facility and stop
keeping track of callback IDs manually. This makes most code easier to read on both the
sender and receiver side.

There are two cases that could not be converted:
- For evaluateJavaScript, we cannot use async IPC because WebAutomationSession expects to
be able to cancel all pending replies when a page navigates away, the web process crashes,
or when handling an alert.
- For takeScreenshot, there is not currently support in the modern async IPC code paths for
sending the result back. ShareableBitmap and friends lack a modern decoder implementation.

* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::resolveChildFrameHandle):
(WebKit::WebAutomationSession::resolveParentFrameHandle):
(WebKit::WebAutomationSession::computeElementLayout):
(WebKit::WebAutomationSession::selectOptionElement):
(WebKit::WebAutomationSession::getAllCookies):
(WebKit::WebAutomationSession::deleteSingleCookie):
(WebKit::WebAutomationSession::viewportInViewCenterPointOfElement):
(WebKit::WebAutomationSession::didResolveChildFrame): Deleted.
(WebKit::WebAutomationSession::didResolveParentFrame): Deleted.
(WebKit::WebAutomationSession::didComputeElementLayout): Deleted.
(WebKit::WebAutomationSession::didSelectOptionElement): Deleted.
(WebKit::WebAutomationSession::didGetCookiesForFrame): Deleted.
(WebKit::WebAutomationSession::didDeleteCookie): Deleted.
* UIProcess/Automation/WebAutomationSession.h:
* UIProcess/Automation/WebAutomationSession.messages.in:
* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::WebAutomationSessionProxy::resolveChildFrameWithOrdinal):
(WebKit::WebAutomationSessionProxy::resolveChildFrameWithNodeHandle):
(WebKit::WebAutomationSessionProxy::resolveChildFrameWithName):
(WebKit::WebAutomationSessionProxy::resolveParentFrame):
(WebKit::WebAutomationSessionProxy::computeElementLayout):
(WebKit::WebAutomationSessionProxy::selectOptionElement):
(WebKit::WebAutomationSessionProxy::getCookiesForFrame):
(WebKit::WebAutomationSessionProxy::deleteCookie):
* WebProcess/Automation/WebAutomationSessionProxy.h:
* WebProcess/Automation/WebAutomationSessionProxy.messages.in:

2019-04-08 Alex Christensen <achristensen@webkit.org>

REGRESSION(236463) DownloadManager can call a null CompletionHandler
Expand Down
235 changes: 91 additions & 144 deletions Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -970,39 +970,33 @@ void WebAutomationSession::resolveChildFrameHandle(const String& browsingContext
if (!frameID)
ASYNC_FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);

uint64_t callbackID = m_nextResolveFrameCallbackID++;
m_resolveChildFrameHandleCallbacks.set(callbackID, WTFMove(callback));
WTF::CompletionHandler<void(Optional<String>, uint64_t)> completionHandler = [this, protectedThis = makeRef(*this), callback = callback.copyRef()](Optional<String> errorType, uint64_t frameID) mutable {
if (errorType) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(*errorType));
return;
}

callback->sendSuccess(handleForWebFrameID(frameID));
};

if (optionalNodeHandle) {
page->process().send(Messages::WebAutomationSessionProxy::ResolveChildFrameWithNodeHandle(page->pageID(), frameID.value(), *optionalNodeHandle, callbackID), 0);
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithNodeHandle(page->pageID(), frameID.value(), *optionalNodeHandle), WTFMove(completionHandler));
return;
}

if (optionalName) {
page->process().send(Messages::WebAutomationSessionProxy::ResolveChildFrameWithName(page->pageID(), frameID.value(), *optionalName, callbackID), 0);
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithName(page->pageID(), frameID.value(), *optionalName), WTFMove(completionHandler));
return;
}

if (optionalOrdinal) {
page->process().send(Messages::WebAutomationSessionProxy::ResolveChildFrameWithOrdinal(page->pageID(), frameID.value(), *optionalOrdinal, callbackID), 0);
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveChildFrameWithOrdinal(page->pageID(), frameID.value(), *optionalOrdinal), WTFMove(completionHandler));
return;
}

ASSERT_NOT_REACHED();
}

void WebAutomationSession::didResolveChildFrame(uint64_t callbackID, uint64_t frameID, const String& errorType)
{
auto callback = m_resolveChildFrameHandleCallbacks.take(callbackID);
if (!callback)
return;

if (!errorType.isEmpty())
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
else
callback->sendSuccess(handleForWebFrameID(frameID));
}

void WebAutomationSession::resolveParentFrameHandle(const String& browsingContextHandle, const String& frameHandle, Ref<ResolveParentFrameHandleCallback>&& callback)
{
WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
Expand All @@ -1013,22 +1007,16 @@ void WebAutomationSession::resolveParentFrameHandle(const String& browsingContex
if (!frameID)
ASYNC_FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);

uint64_t callbackID = m_nextResolveParentFrameCallbackID++;
m_resolveParentFrameHandleCallbacks.set(callbackID, WTFMove(callback));

page->process().send(Messages::WebAutomationSessionProxy::ResolveParentFrame(page->pageID(), frameID.value(), callbackID), 0);
}

void WebAutomationSession::didResolveParentFrame(uint64_t callbackID, uint64_t frameID, const String& errorType)
{
auto callback = m_resolveParentFrameHandleCallbacks.take(callbackID);
if (!callback)
return;
WTF::CompletionHandler<void(Optional<String>, uint64_t)> completionHandler = [this, protectedThis = makeRef(*this), callback = callback.copyRef()](Optional<String> errorType, uint64_t frameID) mutable {
if (errorType) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(*errorType));
return;
}

if (!errorType.isEmpty())
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
else
callback->sendSuccess(handleForWebFrameID(frameID));
};

page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ResolveParentFrame(page->pageID(), frameID.value()), WTFMove(completionHandler));
}

static Optional<CoordinateSystem> protocolStringToCoordinateSystem(const String& coordinateSystemString)
Expand All @@ -1054,62 +1042,42 @@ void WebAutomationSession::computeElementLayout(const String& browsingContextHan
if (!coordinateSystem)
ASYNC_FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'coordinateSystem' is invalid.");

// Start at 2 and use only even numbers to not conflict with m_nextViewportInViewCenterPointOfElementCallbackID.
uint64_t callbackID = m_nextComputeElementLayoutCallbackID += 2;
m_computeElementLayoutCallbacks.set(callbackID, WTFMove(callback));

bool scrollIntoViewIfNeeded = optionalScrollIntoViewIfNeeded ? *optionalScrollIntoViewIfNeeded : false;
page->process().send(Messages::WebAutomationSessionProxy::ComputeElementLayout(page->pageID(), frameID.value(), nodeHandle, scrollIntoViewIfNeeded, coordinateSystem.value(), callbackID), 0);
}

void WebAutomationSession::didComputeElementLayout(uint64_t callbackID, WebCore::IntRect rect, Optional<WebCore::IntPoint> inViewCenterPoint, bool isObscured, const String& errorType)
{
if (callbackID % 2 == 1) {
ASSERT(inViewCenterPoint);
if (auto callback = m_viewportInViewCenterPointOfElementCallbacks.take(callbackID)) {
Optional<AutomationCommandError> error;
if (!errorType.isEmpty())
error = AUTOMATION_COMMAND_ERROR_WITH_MESSAGE(errorType);
callback(inViewCenterPoint, error);
WTF::CompletionHandler<void(Optional<String>, WebCore::IntRect, Optional<WebCore::IntPoint>, bool)> completionHandler = [callback = callback.copyRef()](Optional<String> errorType, WebCore::IntRect rect, Optional<WebCore::IntPoint> inViewCenterPoint, bool isObscured) mutable {
if (errorType) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(*errorType));
return;
}
return;
}

auto callback = m_computeElementLayoutCallbacks.take(callbackID);
if (!callback)
return;

if (!errorType.isEmpty()) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
return;
}
auto originObject = Inspector::Protocol::Automation::Point::create()
.setX(rect.x())
.setY(rect.y())
.release();

auto originObject = Inspector::Protocol::Automation::Point::create()
.setX(rect.x())
.setY(rect.y())
.release();
auto sizeObject = Inspector::Protocol::Automation::Size::create()
.setWidth(rect.width())
.setHeight(rect.height())
.release();

auto sizeObject = Inspector::Protocol::Automation::Size::create()
.setWidth(rect.width())
.setHeight(rect.height())
.release();
auto rectObject = Inspector::Protocol::Automation::Rect::create()
.setOrigin(WTFMove(originObject))
.setSize(WTFMove(sizeObject))
.release();

auto rectObject = Inspector::Protocol::Automation::Rect::create()
.setOrigin(WTFMove(originObject))
.setSize(WTFMove(sizeObject))
.release();
if (!inViewCenterPoint) {
callback->sendSuccess(WTFMove(rectObject), nullptr, isObscured);
return;
}

if (!inViewCenterPoint) {
callback->sendSuccess(WTFMove(rectObject), nullptr, isObscured);
return;
}
auto inViewCenterPointObject = Inspector::Protocol::Automation::Point::create()
.setX(inViewCenterPoint.value().x())
.setY(inViewCenterPoint.value().y())
.release();

auto inViewCenterPointObject = Inspector::Protocol::Automation::Point::create()
.setX(inViewCenterPoint.value().x())
.setY(inViewCenterPoint.value().y())
.release();
callback->sendSuccess(WTFMove(rectObject), WTFMove(inViewCenterPointObject), isObscured);
};

callback->sendSuccess(WTFMove(rectObject), WTFMove(inViewCenterPointObject), isObscured);
bool scrollIntoViewIfNeeded = optionalScrollIntoViewIfNeeded ? *optionalScrollIntoViewIfNeeded : false;
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ComputeElementLayout(page->pageID(), frameID.value(), nodeHandle, scrollIntoViewIfNeeded, coordinateSystem.value()), WTFMove(completionHandler));
}

void WebAutomationSession::selectOptionElement(const String& browsingContextHandle, const String& frameHandle, const String& nodeHandle, Ref<SelectOptionElementCallback>&& callback)
Expand All @@ -1122,27 +1090,18 @@ void WebAutomationSession::selectOptionElement(const String& browsingContextHand
if (!frameID)
ASYNC_FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);

uint64_t callbackID = m_nextSelectOptionElementCallbackID++;
m_selectOptionElementCallbacks.set(callbackID, WTFMove(callback));

page->process().send(Messages::WebAutomationSessionProxy::SelectOptionElement(page->pageID(), frameID.value(), nodeHandle, callbackID), 0);
}

void WebAutomationSession::didSelectOptionElement(uint64_t callbackID, const String& errorType)
{
auto callback = m_selectOptionElementCallbacks.take(callbackID);
if (!callback)
return;

if (!errorType.isEmpty()) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
return;
}
WTF::CompletionHandler<void(Optional<String>)> completionHandler = [callback = callback.copyRef()](Optional<String> errorType) mutable {
if (errorType) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(*errorType));
return;
}

callback->sendSuccess();
};

callback->sendSuccess();
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::SelectOptionElement(page->pageID(), frameID.value(), nodeHandle), WTFMove(completionHandler));
}


void WebAutomationSession::isShowingJavaScriptDialog(Inspector::ErrorString& errorString, const String& browsingContextHandle, bool* result)
{
ASSERT(m_client);
Expand Down Expand Up @@ -1270,21 +1229,6 @@ void WebAutomationSession::setFilesToSelectForFileUpload(ErrorString& errorStrin
m_filesToSelectForFileUpload.swap(newFileList);
}

void WebAutomationSession::getAllCookies(const String& browsingContextHandle, Ref<GetAllCookiesCallback>&& callback)
{
WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
if (!page)
ASYNC_FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);

// Always send the main frame ID as 0 so it is resolved on the WebProcess side. This avoids a race when page->mainFrame() is null still.
const uint64_t mainFrameID = 0;

uint64_t callbackID = m_nextGetCookiesCallbackID++;
m_getCookieCallbacks.set(callbackID, WTFMove(callback));

page->process().send(Messages::WebAutomationSessionProxy::GetCookiesForFrame(page->pageID(), mainFrameID, callbackID), 0);
}

static Ref<Inspector::Protocol::Automation::Cookie> buildObjectForCookie(const WebCore::Cookie& cookie)
{
return Inspector::Protocol::Automation::Cookie::create()
Expand All @@ -1310,18 +1254,24 @@ static Ref<JSON::ArrayOf<Inspector::Protocol::Automation::Cookie>> buildArrayFor
return cookies;
}

void WebAutomationSession::didGetCookiesForFrame(uint64_t callbackID, Vector<WebCore::Cookie> cookies, const String& errorType)
void WebAutomationSession::getAllCookies(const String& browsingContextHandle, Ref<GetAllCookiesCallback>&& callback)
{
auto callback = m_getCookieCallbacks.take(callbackID);
if (!callback)
return;
WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
if (!page)
ASYNC_FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);

if (!errorType.isEmpty()) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
return;
}
WTF::CompletionHandler<void(Optional<String>, Vector<WebCore::Cookie>)> completionHandler = [callback = callback.copyRef()](Optional<String> errorType, Vector<WebCore::Cookie> cookies) mutable {
if (errorType) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(*errorType));
return;
}

callback->sendSuccess(buildArrayForCookies(cookies));
};

callback->sendSuccess(buildArrayForCookies(cookies));
// Always send the main frame ID as 0 so it is resolved on the WebProcess side. This avoids a race when page->mainFrame() is null still.
const uint64_t mainFrameID = 0;
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::GetCookiesForFrame(page->pageID(), mainFrameID), WTFMove(completionHandler));
}

void WebAutomationSession::deleteSingleCookie(const String& browsingContextHandle, const String& cookieName, Ref<DeleteSingleCookieCallback>&& callback)
Expand All @@ -1330,27 +1280,18 @@ void WebAutomationSession::deleteSingleCookie(const String& browsingContextHandl
if (!page)
ASYNC_FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);

// Always send the main frame ID as 0 so it is resolved on the WebProcess side. This avoids a race when page->mainFrame() is null still.
const uint64_t mainFrameID = 0;

uint64_t callbackID = m_nextDeleteCookieCallbackID++;
m_deleteCookieCallbacks.set(callbackID, WTFMove(callback));

page->process().send(Messages::WebAutomationSessionProxy::DeleteCookie(page->pageID(), mainFrameID, cookieName, callbackID), 0);
}

void WebAutomationSession::didDeleteCookie(uint64_t callbackID, const String& errorType)
{
auto callback = m_deleteCookieCallbacks.take(callbackID);
if (!callback)
return;
WTF::CompletionHandler<void(Optional<String>)> completionHandler = [callback = callback.copyRef()](Optional<String> errorType) mutable {
if (errorType) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(*errorType));
return;
}

if (!errorType.isEmpty()) {
callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
return;
}
callback->sendSuccess();
};

callback->sendSuccess();
// Always send the main frame ID as 0 so it is resolved on the WebProcess side. This avoids a race when page->mainFrame() is null still.
const uint64_t mainFrameID = 0;
page->process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::DeleteCookie(page->pageID(), mainFrameID, cookieName), WTFMove(completionHandler));
}

static String domainByAddingDotPrefixIfNeeded(String domain)
Expand Down Expand Up @@ -1518,11 +1459,17 @@ SimulatedInputSource* WebAutomationSession::inputSourceForType(SimulatedInputSou
// MARK: SimulatedInputDispatcher::Client API
void WebAutomationSession::viewportInViewCenterPointOfElement(WebPageProxy& page, uint64_t frameID, const String& nodeHandle, Function<void (Optional<WebCore::IntPoint>, Optional<AutomationCommandError>)>&& completionHandler)
{
// Start at 3 and use only odd numbers to not conflict with m_nextComputeElementLayoutCallbackID.
uint64_t callbackID = m_nextViewportInViewCenterPointOfElementCallbackID += 2;
m_viewportInViewCenterPointOfElementCallbacks.set(callbackID, WTFMove(completionHandler));
WTF::CompletionHandler<void(Optional<String>, WebCore::IntRect, Optional<WebCore::IntPoint>, bool)> didComputeElementLayoutHandler = [completionHandler = WTFMove(completionHandler)](Optional<String> errorType, WebCore::IntRect, Optional<WebCore::IntPoint> inViewCenterPoint, bool) mutable {
if (errorType) {
completionHandler(WTF::nullopt, AUTOMATION_COMMAND_ERROR_WITH_MESSAGE(*errorType));
return;
}

ASSERT(inViewCenterPoint);
completionHandler(inViewCenterPoint, WTF::nullopt);
};

page.process().send(Messages::WebAutomationSessionProxy::ComputeElementLayout(page.pageID(), frameID, nodeHandle, false, CoordinateSystem::LayoutViewport, callbackID), 0);
page.process().sendWithAsyncReply(Messages::WebAutomationSessionProxy::ComputeElementLayout(page.pageID(), frameID, nodeHandle, false, CoordinateSystem::LayoutViewport), WTFMove(didComputeElementLayoutHandler));
}

#if ENABLE(WEBDRIVER_MOUSE_INTERACTIONS)
Expand Down
Loading

0 comments on commit 8525cc0

Please sign in to comment.