[Webkit-unassigned] [Bug 174670] WebDriver: wait until navigation is complete before running new commands and after a click

[bugzilla-daemon at webkit.org]

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

Brian Burg <bburg at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #315976|review?                     |review+
              Flags|                            |

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

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

r=me with a few questions. Great work!

> Source/WebDriver/ChangeLog:15
> +        6.3 Processing Model. Step 7. Wait for navigation to complete. If this returns an error return its value and

Nit: step 6

This suggests that we should be waiting for navigations to complete after any command. Should this be extracted out of particular Session methods, then?

> Source/WebDriver/ChangeLog:19
> +        14.1 Element Click. Step 10. If the click causes navigation: 1. Run the post-navigation checks and return its

I don't understand substep 1. How could the post-navigation checks do anything useful if the click just initiated a load? Ostensibly, the load has likely not been sent out yet, or hasn't received a response in the time waited during Step 9.

> Source/WebDriver/Session.cpp:947
> +    if (!m_toplevelBrowsingContext) {

Nit: add quotations from specific sections/steps of the spec, especially for error cases.

> Source/WebDriver/Session.cpp:954
> +    if (m_browsingContext)

Can we rename this local to m_selectedFrameBrowsingContext or something similar? Otherwise it's a bit obscure why there are two browsing context members.

> Source/WebKit/ChangeLog:24
> +        all the frames. Complete page operations of it's a main frame, or frame operations otherwise.

Nit: of->if

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:467
> +        if (auto callback = m_pendingNavigationInBrowsingContextCallbacksPerPage.take(frame.page()->pageID()))

When asked to wait on a page navigation, couldn't we use the main frame as the callback key and remove m_pendingNavigationInBrowsingContextCallbacksPerPage? Or does the frame somehow not survive long enough?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170720/7d71cc22/attachment.html>