WebContent+WebDriver: Asynchronously wait for navigations to complete

Similar to commit c2cf65adac, we should
avoid spinning the event loop from the WebContent-side of the WebDriver
connection. This can result in deadlocks if another component in LibWeb
also spins the event loop.

The AO to await navigations has two event loop spinners - waiting for
the navigation to complete and for the document to reach the target
readiness state. We now use NavigationObserver and DocumentObserver to
be notified when these conditions are met. And we use the same async IPC
mechanism as script execution to notify the WebDriver process when all
conditions are met (or timed out).

Userland/Libraries/LibWeb/WebDriver/HeapTimer.cpp

@@ -42,6 +42,15 @@ void HeapTimer::start(u64 timeout_ms, JS::NonnullGCPtr<JS::HeapFunction<void()>>
     m_timer->start();
 }
 
+void HeapTimer::stop_and_fire_timeout_handler()
+{
+    auto on_timeout = m_on_timeout;
+    stop();
+
+    if (on_timeout)
+        on_timeout->function()();
+}
+
 void HeapTimer::stop()
 {
     m_on_timeout = nullptr;

Userland/Libraries/LibWeb/WebDriver/HeapTimer.h

@@ -21,6 +21,7 @@ public:
     virtual ~HeapTimer() override;
 
     void start(u64 timeout_ms, JS::NonnullGCPtr<JS::HeapFunction<void()>> on_timeout);
+    void stop_and_fire_timeout_handler();
     void stop();
 
     bool is_timed_out() const { return m_timed_out; }

Userland/Services/WebContent/PageClient.cpp

@@ -389,6 +389,9 @@ void PageClient::page_did_request_media_context_menu(Web::CSSPixelPoint content_
 void PageClient::page_did_request_alert(String const& message)
 {
     client().async_did_request_alert(m_id, message);
+
+    if (m_webdriver)
+        m_webdriver->page_did_open_dialog({});
 }
 
 void PageClient::alert_closed()
@@ -399,6 +402,9 @@ void PageClient::alert_closed()
 void PageClient::page_did_request_confirm(String const& message)
 {
     client().async_did_request_confirm(m_id, message);
+
+    if (m_webdriver)
+        m_webdriver->page_did_open_dialog({});
 }
 
 void PageClient::confirm_closed(bool accepted)
@@ -409,6 +415,9 @@ void PageClient::confirm_closed(bool accepted)
 void PageClient::page_did_request_prompt(String const& message, String const& default_)
 {
     client().async_did_request_prompt(m_id, message, default_);
+
+    if (m_webdriver)
+        m_webdriver->page_did_open_dialog({});
 }
 
 void PageClient::page_did_request_set_prompt_text(String const& text)

Userland/Services/WebContent/WebDriverConnection.cpp

@@ -23,6 +23,7 @@
 #include <LibWeb/Cookie/ParsedCookie.h>
 #include <LibWeb/Crypto/Crypto.h>
 #include <LibWeb/DOM/Document.h>
+#include <LibWeb/DOM/DocumentObserver.h>
 #include <LibWeb/DOM/Element.h>
 #include <LibWeb/DOM/Event.h>
 #include <LibWeb/DOM/Position.h>
@@ -40,6 +41,7 @@
 #include <LibWeb/HTML/HTMLOptionElement.h>
 #include <LibWeb/HTML/HTMLSelectElement.h>
 #include <LibWeb/HTML/HTMLTextAreaElement.h>
+#include <LibWeb/HTML/NavigationObserver.h>
 #include <LibWeb/HTML/Scripting/TemporaryExecutionContext.h>
 #include <LibWeb/HTML/SelectedFile.h>
 #include <LibWeb/HTML/TraversableNavigable.h>
@@ -52,6 +54,7 @@
 #include <LibWeb/WebDriver/Actions.h>
 #include <LibWeb/WebDriver/ElementReference.h>
 #include <LibWeb/WebDriver/ExecuteScript.h>
+#include <LibWeb/WebDriver/HeapTimer.h>
 #include <LibWeb/WebDriver/InputState.h>
 #include <LibWeb/WebDriver/Properties.h>
 #include <LibWeb/WebDriver/Screenshot.h>
@@ -201,6 +204,9 @@ void WebDriverConnection::visit_edges(JS::Cell::Visitor& visitor)
     visitor.visit(m_current_parent_browsing_context);
     visitor.visit(m_current_top_level_browsing_context);
     visitor.visit(m_action_executor);
+    visitor.visit(m_document_observer);
+    visitor.visit(m_navigation_observer);
+    visitor.visit(m_navigation_timer);
 }
 
 // https://w3c.github.io/webdriver/#dfn-close-the-session
@@ -288,22 +294,28 @@ Messages::WebDriverClient::NavigateToResponse WebDriverConnection::navigate_to(J
     // 7. Navigate the current top-level browsing context to url.
     current_top_level_browsing_context()->page().load(url);
 
+    auto navigation_complete = JS::create_heap_function(current_top_level_browsing_context()->heap(), [this](Web::WebDriver::Response result) {
+        // 9. Set the current browsing context with the current top-level browsing context.
+        set_current_browsing_context(*current_top_level_browsing_context());
+
+        // FIXME: 10. If the current top-level browsing context contains a refresh state pragma directive of time 1 second or less, wait until the refresh timeout has elapsed, a new navigate has begun, and return to the first step of this algorithm.
+
+        async_navigation_complete(move(result));
+    });
+
     // 8. If url is special except for file and current URL and URL do not have the same absolute URL:
     // AD-HOC: We wait for the navigation to complete regardless of whether the current URL differs from the provided
     //         URL. Even if they're the same, the navigation queues a tasks that we must await, otherwise subsequent
     //         endpoint invocations will attempt to operate on the wrong page.
     if (url.is_special() && url.scheme() != "file"sv) {
         // a. Try to wait for navigation to complete.
-        TRY(wait_for_navigation_to_complete());
+        wait_for_navigation_to_complete(navigation_complete);
 
         // FIXME: b. Try to run the post-navigation checks.
+    } else {
+        navigation_complete->function()(JsonValue {});
     }
 
-    // 9. Set the current browsing context with the current top-level browsing context.
-    set_current_browsing_context(*current_top_level_browsing_context());
-
-    // FIXME: 10. If the current top-level browsing context contains a refresh state pragma directive of time 1 second or less, wait until the refresh timeout has elapsed, a new navigate has begun, and return to the first step of this algorithm.
-
     // 11. Return success with data null.
     return JsonValue {};
 }
@@ -1365,14 +1377,14 @@ Messages::WebDriverClient::ElementClickResponse WebDriverConnection::element_cli
         // FIXME: 10. Perform implementation-defined steps to allow any navigations triggered by the click to start.
 
         // 11. Try to wait for navigation to complete.
-        if (auto navigation_result = wait_for_navigation_to_complete(); navigation_result.is_error()) {
+        wait_for_navigation_to_complete(JS::create_heap_function(current_browsing_context().heap(), [this, result = move(result)](Web::WebDriver::Response navigation_result) mutable {
-            async_actions_performed(navigation_result.release_error());
-            return;
-        }
-
             // FIXME: 12. Try to run the post-navigation checks.
 
+            if (navigation_result.is_error())
+                async_actions_performed(move(navigation_result));
+            else
                 async_actions_performed(move(result));
+        }));
     });
 
     // 8. Matching on element:
@@ -2394,34 +2406,54 @@ ErrorOr<void, Web::WebDriver::Error> WebDriverConnection::handle_any_user_prompt
     return {};
 }
 
-// https://w3c.github.io/webdriver/#dfn-waiting-for-the-navigation-to-complete
+// https://w3c.github.io/webdriver/#dfn-wait-for-navigation-to-complete
 // FIXME: Update this AO to the latest spec steps.
-ErrorOr<void, Web::WebDriver::Error> WebDriverConnection::wait_for_navigation_to_complete()
+void WebDriverConnection::wait_for_navigation_to_complete(OnNavigationComplete on_complete)
 {
     // 1. If the current session has a page loading strategy of none, return success with data null.
-    if (m_page_load_strategy == Web::WebDriver::PageLoadStrategy::None)
+    if (m_page_load_strategy == Web::WebDriver::PageLoadStrategy::None) {
-        return {};
+        on_complete->function()(JsonValue {});
+        return;
+    }
 
     // 2. If the current browsing context is no longer open, return success with data null.
-    if (ensure_browsing_context_is_open(current_browsing_context()).is_error())
+    if (ensure_browsing_context_is_open(current_browsing_context()).is_error()) {
-        return {};
+        on_complete->function()(JsonValue {});
+        return;
+    }
 
+    auto& realm = current_browsing_context().active_document()->realm();
     auto navigable = current_browsing_context().active_document()->navigable();
-    if (!navigable || navigable->ongoing_navigation().has<Empty>())
-        return {};
 
-    // 3. Start a timer. If this algorithm has not completed before timer reaches the session’s session page load timeout in milliseconds, return an error with error code timeout.
+    if (!navigable || navigable->ongoing_navigation().has<Empty>()) {
-    auto page_load_timeout_fired = false;
+        on_complete->function()(JsonValue {});
-    auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout.value_or(300'000), [&] {
+        return;
-        page_load_timeout_fired = true;
+    }
-    });
-    timer->start();
-    // 4. If there is an ongoing attempt to navigate the current browsing context that has not yet matured, wait for navigation to mature.
-    Web::Platform::EventLoopPlugin::the().spin_until([&] {
-        return page_load_timeout_fired || navigable->ongoing_navigation().has<Empty>();
-    });
 
-    // 5. Let readiness target be the document readiness state associated with the current session’s page loading strategy, which can be found in the table of page load strategies.
+    auto reset_observers = [](auto& self) {
+        if (self.m_navigation_observer) {
+            self.m_navigation_observer->set_navigation_complete({});
+            self.m_navigation_observer = nullptr;
+        }
+        if (self.m_document_observer) {
+            self.m_document_observer->set_document_readiness_observer({});
+            self.m_document_observer = nullptr;
+        }
+    };
+
+    // 3. Start a timer. If this algorithm has not completed before timer reaches the session’s session page load timeout
+    //    in milliseconds, return an error with error code timeout.
+    m_navigation_timer = realm.heap().allocate<Web::WebDriver::HeapTimer>(realm);
+
+    // 4. If there is an ongoing attempt to navigate the current browsing context that has not yet matured, wait for
+    //    navigation to mature.
+    m_navigation_observer = realm.heap().allocate<Web::HTML::NavigationObserver>(realm, realm, *navigable);
+
+    m_navigation_observer->set_navigation_complete([this, &realm, reset_observers]() {
+        reset_observers(*this);
+
+        // 5. Let readiness target be the document readiness state associated with the current session’s page loading
+        //    strategy, which can be found in the table of page load strategies.
         auto readiness_target = [this]() {
             switch (m_page_load_strategy) {
             case Web::WebDriver::PageLoadStrategy::Normal:
@@ -2435,16 +2467,44 @@ ErrorOr<void, Web::WebDriver::Error> WebDriverConnection::wait_for_navigation_to
 
         // 6. Wait for the current browsing context’s document readiness state to reach readiness target,
         //    or for the session page load timeout to pass, whichever occurs sooner.
-    Web::Platform::EventLoopPlugin::the().spin_until([&]() {
+        if (auto* document = current_browsing_context().active_document(); document->readiness() != readiness_target) {
-        return page_load_timeout_fired || current_browsing_context().active_document()->readiness() == readiness_target;
+            m_document_observer = realm.heap().allocate<Web::DOM::DocumentObserver>(realm, realm, *document);
+
+            m_document_observer->set_document_readiness_observer([this, readiness_target](Web::HTML::DocumentReadyState readiness) {
+                if (readiness == readiness_target)
+                    m_navigation_timer->stop_and_fire_timeout_handler();
+            });
+        } else {
+            m_navigation_timer->stop_and_fire_timeout_handler();
+        }
     });
 
-    // 7. If the previous step completed by the session page load timeout being reached and the browser does not have an active user prompt, return error with error code timeout.
+    m_navigation_timer->start(m_timeouts_configuration.page_load_timeout.value_or(300'000), JS::create_heap_function(realm.heap(), [this, on_complete, reset_observers]() {
-    if (page_load_timeout_fired && !current_browsing_context().page().has_pending_dialog())
+        reset_observers(*this);
-        return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::Timeout, "Navigation timed out"sv);
+
+        auto did_time_out = m_navigation_timer->is_timed_out();
+        m_navigation_timer = nullptr;
+
+        // 7. If the previous step completed by the session page load timeout being reached and the browser does
+        //    not have an active user prompt, return error with error code timeout.
+        if (did_time_out && !current_browsing_context().active_document()->page().has_pending_dialog()) {
+            on_complete->function()(Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::Timeout, "Navigation timed out"sv));
+            return;
+        }
 
         // 8. Return success with data null.
-    return {};
+        on_complete->function()(JsonValue {});
+    }));
+}
+
+void WebDriverConnection::page_did_open_dialog(Badge<PageClient>)
+{
+    // OPTMIZATION: If a dialog is opened while we are awaiting a specific document readiness state, that state will
+    //              never be reached, as the dialog will block the HTML event loop from any further processing. Instead
+    //              of waiting for the session's page load timeout to expire, unblock the waiter immediately. This also
+    //              seems to match how other browsers behave.
+    if (m_navigation_timer)
+        m_navigation_timer->stop_and_fire_timeout_handler();
 }
 
 // https://w3c.github.io/webdriver/#dfn-restore-the-window

Userland/Services/WebContent/WebDriverConnection.h

@@ -37,6 +37,8 @@ public:
 
     void visit_edges(JS::Cell::Visitor&);
 
+    void page_did_open_dialog(Badge<PageClient>);
+
 private:
     WebDriverConnection(IPC::Transport transport, Web::PageClient& page_client);
 
@@ -122,7 +124,8 @@ private:
     Gfx::IntRect maximize_the_window();
     Gfx::IntRect iconify_the_window();
 
-    ErrorOr<void, Web::WebDriver::Error> wait_for_navigation_to_complete();
+    using OnNavigationComplete = JS::NonnullGCPtr<JS::HeapFunction<void(Web::WebDriver::Response)>>;
+    void wait_for_navigation_to_complete(OnNavigationComplete);
 
     Gfx::IntPoint calculate_absolute_position_of_element(JS::NonnullGCPtr<Web::Geometry::DOMRect> rect);
     Gfx::IntRect calculate_absolute_rect_of_element(Web::DOM::Element const& element);
@@ -159,6 +162,10 @@ private:
     JS::GCPtr<Web::HTML::BrowsingContext> m_current_top_level_browsing_context;
 
     JS::GCPtr<JS::Cell> m_action_executor;
+
+    JS::GCPtr<Web::DOM::DocumentObserver> m_document_observer;
+    JS::GCPtr<Web::HTML::NavigationObserver> m_navigation_observer;
+    JS::GCPtr<Web::WebDriver::HeapTimer> m_navigation_timer;
 };
 
 }

Userland/Services/WebContent/WebDriverServer.ipc

@@ -1,6 +1,7 @@
 #include <LibWeb/WebDriver/Response.h>
 
 endpoint WebDriverServer {
+    navigation_complete(Web::WebDriver::Response response) =|
     script_executed(Web::WebDriver::Response response) =|
     actions_performed(Web::WebDriver::Response response) =|
 }

Userland/Services/WebDriver/Client.cpp

@@ -241,7 +241,7 @@ Web::WebDriver::Response Client::navigate_to(Web::WebDriver::Parameters paramete
 {
     dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session/<session_id>/url");
     auto session = TRY(find_session_with_id(parameters[0]));
-    return session->web_content_connection().navigate_to(payload);
+    return session->navigate_to(payload);
 }
 
 // 10.2 Get Current URL, https://w3c.github.io/webdriver/#dfn-get-current-url

Userland/Services/WebDriver/Session.cpp

@@ -202,6 +202,13 @@ static Web::WebDriver::Response perform_async_action(Handler& handler, Action&&
     return response.release_value();
 }
 
+Web::WebDriver::Response Session::navigate_to(JsonValue payload) const
+{
+    return perform_async_action(web_content_connection().on_navigation_complete, [&]() {
+        return web_content_connection().navigate_to(move(payload));
+    });
+}
+
 Web::WebDriver::Response Session::execute_script(JsonValue payload, ScriptMode mode) const
 {
     return perform_async_action(web_content_connection().on_script_executed, [&]() {

Userland/Services/WebDriver/Session.h

@@ -57,6 +57,8 @@ public:
     Web::WebDriver::Response get_window_handles() const;
     ErrorOr<void, Web::WebDriver::Error> ensure_current_window_handle_is_valid() const;
 
+    Web::WebDriver::Response navigate_to(JsonValue) const;
+
     enum class ScriptMode {
         Sync,
         Async,

Userland/Services/WebDriver/WebContentConnection.cpp

@@ -20,6 +20,12 @@ void WebContentConnection::die()
         on_close();
 }
 
+void WebContentConnection::navigation_complete(Web::WebDriver::Response const& response)
+{
+    if (on_navigation_complete)
+        on_navigation_complete(response);
+}
+
 void WebContentConnection::script_executed(Web::WebDriver::Response const& response)
 {
     if (on_script_executed)

Userland/Services/WebDriver/WebContentConnection.h

@@ -22,12 +22,14 @@ public:
     explicit WebContentConnection(IPC::Transport transport);
 
     Function<void()> on_close;
+    Function<void(Web::WebDriver::Response)> on_navigation_complete;
     Function<void(Web::WebDriver::Response)> on_script_executed;
     Function<void(Web::WebDriver::Response)> on_actions_performed;
 
 private:
     virtual void die() override;
 
+    virtual void navigation_complete(Web::WebDriver::Response const&) override;
     virtual void script_executed(Web::WebDriver::Response const&) override;
     virtual void actions_performed(Web::WebDriver::Response const&) override;
 };