Summary: | Web Automation: Get Title command sometimes hangs inside evaluateJavaScriptFunction | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||
Component: | WebDriver | Assignee: | BJ Burg <bburg> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | bburg, hi, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
BJ Burg
2021-03-09 23:39:41 PST
Created attachment 422805 [details]
Patch
Created attachment 422806 [details]
Patch v1.0
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. 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. |