[webkit-reviews] review granted: [Bug 176583] Modernize and make API::UIClient more asynchronous : [Attachment 320271] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 176583: Modernize and make API::UIClient more asynchronous
https://bugs.webkit.org/show_bug.cgi?id=176583

Attachment 320271: Patch

https://bugs.webkit.org/attachment.cgi?id=320271&action=review




--- Comment #9 from Brian Burg <bburg at apple.com> ---
Comment on attachment 320271
  --> https://bugs.webkit.org/attachment.cgi?id=320271
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320271&action=review

r=me with small request

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:212
> +    page.getWindowFrameWithCompletionHandler([this, protectedThis =
WTFMove(protectedThis), activeURL = page.pageLoadState().activeURL(), handle =
handleForWebPageProxy(page), completionHandler =
WTFMove(completionHandler)](WebCore::FloatRect windowFrame) mutable {

Err, this is weird. The async part should not be in the factory method, which
just translates WK types to JSON payloads. So the new signature for this
builder should be adjusted to take the page and its windowFrame, and stay
synchronous.

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:235
> +void WebAutomationSession::getNextContext(Ref<WebAutomationSession>&&
protectedThis, Vector<Ref<WebPageProxy>>&& pages,
Ref<Inspector::Protocol::Array<Inspector::Protocol::Automation::BrowsingContext
>> contexts, Ref<WebAutomationSession::GetBrowsingContextsCallback>&& callback)

This diff is hard to read.

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:242
> +    buildBrowsingContextForPage(WTFMove(protectedThis), page.get(), [this,
pages = WTFMove(pages), contexts = WTFMove(contexts), callback =
WTFMove(callback)](Ref<WebAutomationSession>&& protectedThis,
Ref<Inspector::Protocol::Automation::BrowsingContext>&& context) mutable {

... so the windowFrame async call would be here, and call the builder inside
the completion handler.

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1288
> +

parsedButton and parsedInteraction don't need to be in the completion handler.