LibWeb: Wait until child navigable's SHE is ready before navigating

This change fixes a bug that can be reproduced with the following steps:
```js
const iframe = document.createElement("iframe");
document.body.appendChild(iframe);
iframe.contentWindow.location.href = ("http://localhost:8080/demo.html");
```

These steps are executed in the following order:
1. Create iframe and schedule session history traversal task that adds
   session history entry for the iframe.
2. Generate navigation id for scheduled navigation to
   `http://localhost:8080/demo.html`.
3. Execute the scheduled session history traversal task, which adds
   session history entry for the iframe.
4. Ooops, navigation to `http://localhost:8080/demo.html` is aborted
   because addings SHE for the iframe resets the navigation id.

This change fixes this by delaying all navigations until SHE for a
navigable is created.

Libraries/LibWeb/DOM/Document.cpp

@@ -684,9 +684,9 @@ WebIDL::ExceptionOr<Document*> Document::open(Optional<String> const&, Optional<
     // If document belongs to a child navigable, we need to make sure its initial navigation is done,
     // because subsequent steps will modify "initial about:blank" to false, which would cause
     // initial navigation to fail in case it was "about:blank".
-    if (auto navigable = this->navigable(); navigable && navigable->container() && !navigable->container()->content_navigable_initialized()) {
+    if (auto navigable = this->navigable(); navigable && navigable->container() && !navigable->container()->content_navigable_has_session_history_entry_and_ready_for_navigation()) {
         HTML::main_thread_event_loop().spin_processing_tasks_with_source_until(HTML::Task::Source::NavigationAndTraversal, GC::create_function(heap(), [navigable_container = navigable->container()] {
-            return navigable_container->content_navigable_initialized();
+            return navigable_container->content_navigable_has_session_history_entry_and_ready_for_navigation();
         }));
     }
 

Libraries/LibWeb/HTML/HTMLFrameElement.cpp

@@ -52,7 +52,7 @@ void HTMLFrameElement::inserted()
     MUST(create_new_child_navigable(GC::create_function(realm().heap(), [this] {
         // 4. Process the frame attributes for insertedNode, with initialInsertion set to true.
         process_the_frame_attributes(true);
-        set_content_navigable_initialized();
+        set_content_navigable_has_session_history_entry_and_ready_for_navigation();
     })));
 }
 

Libraries/LibWeb/HTML/HTMLIFrameElement.cpp

@@ -92,7 +92,8 @@ void HTMLIFrameElement::post_connection()
 
         // 3. Process the iframe attributes for insertedNode, with initialInsertion set to true.
         process_the_iframe_attributes(true);
-        set_content_navigable_initialized();
+
+        set_content_navigable_has_session_history_entry_and_ready_for_navigation();
     })));
 }
 

Libraries/LibWeb/HTML/HTMLObjectElement.cpp

@@ -446,7 +446,7 @@ void HTMLObjectElement::run_object_representation_handler_steps(Fetch::Infrastru
         // If the object element's content navigable is null, then create a new child navigable for the element.
         if (!m_content_navigable && in_a_document_tree()) {
             MUST(create_new_child_navigable());
-            set_content_navigable_initialized();
+            set_content_navigable_has_session_history_entry_and_ready_for_navigation();
         }
 
         // NOTE: Creating a new nested browsing context can fail if the document is not attached to a browsing context

Libraries/LibWeb/HTML/Navigable.cpp

@@ -134,6 +134,17 @@ void Navigable::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_container);
     visitor.visit(m_navigation_observers);
     m_event_handler.visit_edges(visitor);
+
+    for (auto& navigation_params : m_pending_navigations) {
+        navigation_params.visit_edges(visitor);
+    }
+}
+
+void Navigable::NavigateParams::visit_edges(Cell::Visitor& visitor)
+{
+    visitor.visit(response);
+    visitor.visit(source_document);
+    visitor.visit(source_element);
 }
 
 // https://html.spec.whatwg.org/multipage/nav-history-apis.html#script-closable
@@ -1329,40 +1340,12 @@ WebIDL::ExceptionOr<void> Navigable::populate_session_history_entry_document(
     return {};
 }
 
-// To navigate a navigable navigable to a URL url using a Document sourceDocument,
-// with an optional POST resource, string, or null documentResource (default null),
-// an optional response-or-null response (default null), an optional boolean exceptionsEnabled (default false),
-// an optional NavigationHistoryBehavior historyHandling (default "auto"),
-// an optional serialized state-or-null navigationAPIState (default null),
-// an optional entry list or null formDataEntryList (default null),
-// an optional referrer policy referrerPolicy (default the empty string),
-// an optional user navigation involvement userInvolvement (default "none"),
-// and an optional Element sourceElement (default null):
-
-// https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate
 WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
 {
-    // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window.
-    if (!active_window())
-        return {};
-
-    auto const& url = params.url;
     auto source_document = params.source_document;
-    auto const& document_resource = params.document_resource;
-    auto response = params.response;
     auto exceptions_enabled = params.exceptions_enabled;
-    auto history_handling = params.history_handling;
-    auto const& navigation_api_state = params.navigation_api_state;
-    auto const& form_data_entry_list = params.form_data_entry_list;
-    auto referrer_policy = params.referrer_policy;
-    auto user_involvement = params.user_involvement;
-    auto source_element = params.source_element;
     auto& active_document = *this->active_document();
     auto& realm = active_document.realm();
-    auto& vm = this->vm();
-
-    // 1. Let cspNavigationType be "form-submission" if formDataEntryList is non-null; otherwise "other".
-    auto csp_navigation_type = form_data_entry_list.has_value() ? CSPNavigationType::FormSubmission : CSPNavigationType::Other;
 
     // 2. Let sourceSnapshotParams be the result of snapshotting source snapshot params given sourceDocument.
     auto source_snapshot_params = source_document->snapshot_source_snapshot_params();
@@ -1384,8 +1367,67 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
         return {};
     }
 
+    if (m_pending_navigations.size() == 0 && params.url.equals(URL::about_blank())) {
+        begin_navigation(move(params));
+        return {};
+    }
+
+    if (!m_has_session_history_entry_and_ready_for_navigation) {
+        m_pending_navigations.append(move(params));
+        return {};
+    }
+
+    begin_navigation(move(params));
+    return {};
+}
+
+// To navigate a navigable navigable to a URL url using a Document sourceDocument,
+// with an optional POST resource, string, or null documentResource (default null),
+// an optional response-or-null response (default null), an optional boolean exceptionsEnabled (default false),
+// an optional NavigationHistoryBehavior historyHandling (default "auto"),
+// an optional serialized state-or-null navigationAPIState (default null),
+// an optional entry list or null formDataEntryList (default null),
+// an optional referrer policy referrerPolicy (default the empty string),
+// an optional user navigation involvement userInvolvement (default "none"),
+// and an optional Element sourceElement (default null):
+
+// https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate
+void Navigable::begin_navigation(NavigateParams params)
+{
+    // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window.
+    if (!active_window())
+        return;
+
+    auto const& url = params.url;
+    auto source_document = params.source_document;
+    auto const& document_resource = params.document_resource;
+    auto response = params.response;
+    auto history_handling = params.history_handling;
+    auto const& navigation_api_state = params.navigation_api_state;
+    auto const& form_data_entry_list = params.form_data_entry_list;
+    auto referrer_policy = params.referrer_policy;
+    auto user_involvement = params.user_involvement;
+    auto source_element = params.source_element;
+    auto& active_document = *this->active_document();
+    auto& vm = this->vm();
+
+    // 1. Let cspNavigationType be "form-submission" if formDataEntryList is non-null; otherwise "other".
+    auto csp_navigation_type = form_data_entry_list.has_value() ? CSPNavigationType::FormSubmission : CSPNavigationType::Other;
+
+    // 2. Let sourceSnapshotParams be the result of snapshotting source snapshot params given sourceDocument.
+    auto source_snapshot_params = source_document->snapshot_source_snapshot_params();
+
+    // 3. Let initiatorOriginSnapshot be sourceDocument's origin.
+    auto initiator_origin_snapshot = source_document->origin();
+
+    // 4. Let initiatorBaseURLSnapshot be sourceDocument's document base URL.
+    auto initiator_base_url_snapshot = source_document->base_url();
+
+    // 5. If sourceDocument's node navigable is not allowed by sandboxing to navigate navigable given sourceSnapshotParams, then:
+    // NOTE: This step is handled in Navigable::navigate()
+
     // 6. Let navigationId be the result of generating a random UUID.
-    String navigation_id = TRY_OR_THROW_OOM(vm, Crypto::generate_random_uuid());
+    String navigation_id = MUST(Crypto::generate_random_uuid());
 
     // FIXME: 7. If the surrounding agent is equal to navigable's active document's relevant agent, then continue these steps.
     //           Otherwise, queue a global task on the navigation and traversal task source given navigable's active window to continue these steps.
@@ -1396,7 +1438,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
     if (active_document.unload_counter() > 0) {
         // FIXME: invoke WebDriver BiDi navigation failed with navigable and a WebDriver BiDi navigation status whose id
         //        is navigationId, status is "canceled", and url is url
-        return {};
+        return;
     }
 
     // 9. Let container be navigable's container.
@@ -1441,10 +1483,10 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
         && url.equals(active_session_history_entry()->url(), URL::ExcludeFragment::Yes)
         && url.fragment().has_value()) {
         // 1. Navigate to a fragment given navigable, url, historyHandling, userInvolvement, sourceElement, navigationAPIState, and navigationId.
-        TRY(navigate_to_a_fragment(url, to_history_handling_behavior(history_handling), user_involvement, source_element, navigation_api_state, navigation_id));
+        navigate_to_a_fragment(url, to_history_handling_behavior(history_handling), user_involvement, source_element, navigation_api_state, navigation_id);
 
         // 2. Return.
-        return {};
+        return;
     }
 
     // 14. If navigable's parent is non-null, then set navigable's is delaying load events to true.
@@ -1461,7 +1503,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
         // FIXME: 1. Invoke WebDriver BiDi navigation failed with navigable and a new WebDriver BiDi navigation status whose id is navigationId, status is "canceled", and url is url.
 
         // 2. Return.
-        return {};
+        return;
     }
 
     // 18. Set the ongoing navigation for navigable to navigationId.
@@ -1476,7 +1518,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
         }));
 
         // 2. Return.
-        return {};
+        return;
     }
 
     // 20. If all of the following are true:
@@ -1491,7 +1533,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
         auto navigation = active_window()->navigation();
 
         // 2. Let entryListForFiring be formDataEntryList if documentResource is a POST resource; otherwise, null.
-        auto entry_list_for_firing = [&]() -> Optional<Vector<XHR::FormDataEntry>&> {
+        auto entry_list_for_firing = [&]() -> Optional<Vector<XHR::FormDataEntry>> {
             if (document_resource.has<POSTResource>())
                 return form_data_entry_list;
             return {};
@@ -1520,7 +1562,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
 
         // 5. If continue is false, then return.
         if (!continue_)
-            return {};
+            return;
     }
 
     // AD-HOC: Tell the UI that we started loading.
@@ -1619,14 +1661,12 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
         })).release_value_but_fixme_should_propagate_errors();
     }));
 
-    return {};
+    return;
 }
 
 // https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate-fragid
-WebIDL::ExceptionOr<void> Navigable::navigate_to_a_fragment(URL::URL const& url, HistoryHandlingBehavior history_handling, UserNavigationInvolvement user_involvement, GC::Ptr<DOM::Element> source_element, Optional<SerializationRecord> navigation_api_state, String navigation_id)
+void Navigable::navigate_to_a_fragment(URL::URL const& url, HistoryHandlingBehavior history_handling, UserNavigationInvolvement user_involvement, GC::Ptr<DOM::Element> source_element, Optional<SerializationRecord> navigation_api_state, String navigation_id)
 {
-    (void)navigation_id;
-
     // 1. Let navigation be navigable's active window's navigation API.
     VERIFY(active_window());
     auto navigation = active_window()->navigation();
@@ -1643,7 +1683,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate_to_a_fragment(URL::URL const& url,
 
     // 5. If continue is false, then return.
     if (!continue_)
-        return {};
+        return;
 
     // 6. Let historyEntry be a new session history entry, with
     //      URL: url
@@ -1707,8 +1747,6 @@ WebIDL::ExceptionOr<void> Navigable::navigate_to_a_fragment(URL::URL const& url,
         //            navigation status whose id is navigationId, url is url, and status is "complete".
         (void)navigation_id;
     }));
-
-    return {};
 }
 
 // https://html.spec.whatwg.org/multipage/browsing-the-web.html#evaluate-a-javascript:-url
@@ -2366,4 +2404,13 @@ void Navigable::stop_loading()
     document->abort_a_document_and_its_descendants();
 }
 
+void Navigable::set_has_session_history_entry_and_ready_for_navigation()
+{
+    m_has_session_history_entry_and_ready_for_navigation = true;
+    while (!m_pending_navigations.is_empty()) {
+        auto navigation_params = m_pending_navigations.take_first();
+        begin_navigation(navigation_params);
+    }
+}
+
 }

Libraries/LibWeb/HTML/Navigable.h

@@ -135,25 +135,24 @@ public:
         GC::Ptr<GC::Function<void()>> completion_steps = {});
 
     struct NavigateParams {
-        URL::URL const& url;
+        URL::URL url;
         GC::Ref<DOM::Document> source_document;
         Variant<Empty, String, POSTResource> document_resource = Empty {};
         GC::Ptr<Fetch::Infrastructure::Response> response = nullptr;
         bool exceptions_enabled = false;
         Bindings::NavigationHistoryBehavior history_handling = Bindings::NavigationHistoryBehavior::Auto;
         Optional<SerializationRecord> navigation_api_state = {};
-        Optional<Vector<XHR::FormDataEntry>&> form_data_entry_list = {};
+        Optional<Vector<XHR::FormDataEntry>> form_data_entry_list = {};
         ReferrerPolicy::ReferrerPolicy referrer_policy = ReferrerPolicy::ReferrerPolicy::EmptyString;
         UserNavigationInvolvement user_involvement = UserNavigationInvolvement::None;
         GC::Ptr<DOM::Element> source_element = nullptr;
+
+        void visit_edges(Cell::Visitor& visitor);
     };
 
     WebIDL::ExceptionOr<void> navigate(NavigateParams);
 
-    WebIDL::ExceptionOr<void> navigate_to_a_fragment(URL::URL const&, HistoryHandlingBehavior, UserNavigationInvolvement, GC::Ptr<DOM::Element> source_element, Optional<SerializationRecord> navigation_api_state, String navigation_id);
-
     GC::Ptr<DOM::Document> evaluate_javascript_url(URL::URL const&, URL::Origin const& new_document_origin, UserNavigationInvolvement, String navigation_id);
-    void navigate_to_a_javascript_url(URL::URL const&, HistoryHandlingBehavior, GC::Ref<SourceSnapshotParams>, URL::Origin const& initiator_origin, UserNavigationInvolvement, CSPNavigationType csp_navigation_type, String navigation_id);
 
     bool allowed_by_sandboxing_to_navigate(Navigable const& target, SourceSnapshotParams const&);
 
@@ -188,6 +187,11 @@ public:
     Web::EventHandler& event_handler() { return m_event_handler; }
     Web::EventHandler const& event_handler() const { return m_event_handler; }
 
+    bool has_session_history_entry_and_ready_for_navigation() const { return m_has_session_history_entry_and_ready_for_navigation; }
+    void set_has_session_history_entry_and_ready_for_navigation();
+
+    bool has_pending_navigations() const { return !m_pending_navigations.is_empty(); }
+
 protected:
     explicit Navigable(GC::Ref<Page>);
 
@@ -198,6 +202,10 @@ protected:
     Variant<Empty, Traversal, String> m_ongoing_navigation;
 
 private:
+    void begin_navigation(NavigateParams);
+    void navigate_to_a_fragment(URL::URL const&, HistoryHandlingBehavior, UserNavigationInvolvement, GC::Ptr<DOM::Element> source_element, Optional<SerializationRecord> navigation_api_state, String navigation_id);
+    void navigate_to_a_javascript_url(URL::URL const&, HistoryHandlingBehavior, GC::Ref<SourceSnapshotParams>, URL::Origin const& initiator_origin, UserNavigationInvolvement, CSPNavigationType csp_navigation_type, String navigation_id);
+
     void reset_cursor_blink_cycle();
 
     void scroll_offset_did_change();
@@ -235,6 +243,10 @@ private:
     CSSPixelPoint m_viewport_scroll_offset;
 
     Web::EventHandler m_event_handler;
+
+    bool m_has_session_history_entry_and_ready_for_navigation { false };
+
+    Vector<NavigateParams> m_pending_navigations;
 };
 
 HashTable<GC::RawRef<Navigable>>& all_navigables();

Libraries/LibWeb/HTML/NavigableContainer.cpp

@@ -202,6 +202,10 @@ Optional<URL::URL> NavigableContainer::shared_attribute_processing_steps_for_ifr
     if (!m_content_navigable)
         return {};
 
+    if (initial_insertion && m_content_navigable->has_pending_navigations()) {
+        return {};
+    }
+
     // 1. Let url be the URL record about:blank.
     auto url = URL::about_blank();
 
@@ -310,7 +314,7 @@ void NavigableContainer::destroy_the_child_navigable()
 // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#potentially-delays-the-load-event
 bool NavigableContainer::currently_delays_the_load_event() const
 {
-    if (!m_content_navigable_initialized)
+    if (!content_navigable_has_session_history_entry_and_ready_for_navigation())
         return true;
 
     if (!m_potentially_delays_the_load_event)

Libraries/LibWeb/HTML/NavigableContainer.h

@@ -38,7 +38,12 @@ public:
     // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#potentially-delays-the-load-event
     bool currently_delays_the_load_event() const;
 
-    bool content_navigable_initialized() const { return m_content_navigable_initialized; }
+    bool content_navigable_has_session_history_entry_and_ready_for_navigation() const
+    {
+        if (!content_navigable())
+            return false;
+        return m_content_navigable->has_session_history_entry_and_ready_for_navigation();
+    }
 
 protected:
     NavigableContainer(DOM::Document&, DOM::QualifiedName);
@@ -58,12 +63,16 @@ protected:
 
     void set_potentially_delays_the_load_event(bool value) { m_potentially_delays_the_load_event = value; }
 
-    void set_content_navigable_initialized() { m_content_navigable_initialized = true; }
+    void set_content_navigable_has_session_history_entry_and_ready_for_navigation()
+    {
+        if (!content_navigable())
+            return;
+        content_navigable()->set_has_session_history_entry_and_ready_for_navigation();
+    }
 
 private:
     virtual bool is_navigable_container() const override { return true; }
     bool m_potentially_delays_the_load_event { true };
-    bool m_content_navigable_initialized { false };
 };
 
 }

Libraries/LibWeb/HTML/TraversableNavigable.cpp

@@ -137,6 +137,7 @@ WebIDL::ExceptionOr<GC::Ref<TraversableNavigable>> TraversableNavigable::create_
 
     // 9. Append initialHistoryEntry to traversable's session history entries.
     traversable->m_session_history_entries.append(*initial_history_entry);
+    traversable->set_has_session_history_entry_and_ready_for_navigation();
 
     // FIXME: 10. If opener is non-null, then legacy-clone a traversable storage shed given opener's top-level traversable and traversable. [STORAGE]
 

Tests/LibWeb/Text/data/test-iframe-content.html

@@ -0,0 +1,6 @@
+<body>
+    hello!
+</body>
+<script>
+    window.parent.postMessage("hello!", "*");
+</script>

Tests/LibWeb/Text/expected/navigation/navigate-iframe-right-after-creation.txt

@@ -0,0 +1 @@
+hello!

Tests/LibWeb/Text/input/navigation/navigate-iframe-right-after-creation.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html lang="en">
+<script src="../include.js"></script>
+<body>
+    <script>
+        asyncTest(done => {
+            const iframe = document.createElement("iframe");
+            document.body.appendChild(iframe);
+            window.addEventListener("message", (e) => {
+                println(e.data);
+                done();
+            });
+            iframe.contentWindow.location.href = "../../data/test-iframe-content.html";
+        });
+    </script>
+</body>
+</html>