Bug 223017 – Web Automation: Get Title command sometimes hangs inside evaluateJavaScriptFunction

• Component: WebDriver
• Status: NEW

[Blaze Burg]

.

[Blaze Burg]

<rdar://75239013>

[Blaze Burg]

Created attachment 422805 [details]
Patch

[Blaze Burg]

Created attachment 422806 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 422806 [details]
Patch v1.0

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

r=me, nice fix :)

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:771
> +        // would no longer be valid and the command would also fail also with WindowNotFound, just earlier.

"also fail also"

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:774
> +            for (auto key : copyToVector(m_evaluateJavaScriptFunctionCallbacks.keys())) {
> +                auto callback = m_evaluateJavaScriptFunctionCallbacks.take(key);

Rather than `copyToVector`, can you `std::exchange`?
```
    for (auto&& [key, callback] : std::exchange(m_evaluateJavaScriptFunctionCallbacks, { }))
        ASYNC_FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
```
I think you could also drop the `isEmpty` check if you do this too.

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:775
> +                ASYNC_FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);

I believe this will `return`.  Based on your comment it sounds like we want that, but I just want to make sure.

[Blaze Burg]

Looking at this more, I think the bug is actually that WebAutomationSessionProxy is not seeing these main frame navigations. It already has cancellation logic. I'm going to look into this with a debugger to see if it ever gets triggered.