From d86ad2fcfa17867e812b9f79d2d05d1e69b12c4a Mon Sep 17 00:00:00 2001
From: Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
Date: Wed, 10 Apr 2024 19:10:10 +0200
Subject: [PATCH] LibWeb: Process all task source while waiting for document
 population

"apply the history step" initiated by reloading or back/forward
navigation might require doing fetching while populating a document,
so it is not possible to restrict spin_until() to process only
NavigationAndTraversal task source.

"apply the history step" initiated by synchronous navigation keeps
processing only NavigationAndTraversal task source because it will
never have to populate a document. Another reason to keep synchronous
navigation blocking other task sources is that we crash if active SHE
changes in the middle of "apply the history step" initiated by sync
navigation. The new test is added to makes sure we don't regress that.
---
 .../history-replace-push-state-race-3.txt     |  1 +
 .../history-replace-push-state-race-3.html    | 22 +++++++++++++++++++
 .../LibWeb/HTML/TraversableNavigable.cpp      | 10 ++++++++-
 3 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 Tests/LibWeb/Text/expected/navigation/history-replace-push-state-race-3.txt
 create mode 100644 Tests/LibWeb/Text/input/navigation/history-replace-push-state-race-3.html

diff --git a/Tests/LibWeb/Text/expected/navigation/history-replace-push-state-race-3.txt b/Tests/LibWeb/Text/expected/navigation/history-replace-push-state-race-3.txt
new file mode 100644
index 00000000000..173c92ab806
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/navigation/history-replace-push-state-race-3.txt
@@ -0,0 +1 @@
+test done!
diff --git a/Tests/LibWeb/Text/input/navigation/history-replace-push-state-race-3.html b/Tests/LibWeb/Text/input/navigation/history-replace-push-state-race-3.html
new file mode 100644
index 00000000000..b1102c77f10
--- /dev/null
+++ b/Tests/LibWeb/Text/input/navigation/history-replace-push-state-race-3.html
@@ -0,0 +1,22 @@
+<script src="../include.js"></script>
+<script>
+    asyncTest(done => {
+        let counter = 0;
+        setTimeout(() => {
+            history.replaceState({}, "test", "history-replace-push-state-race-3.html");
+            counter++;
+            if (counter === 2) {
+                println("test done!");
+                done();
+            }
+        }, 0);
+        setTimeout(() => {
+            history.replaceState({}, "test", "history-replace-push-state-race-3.html");
+            counter++;
+            if (counter === 2) {
+                println("test done!");
+                done();
+            }
+        }, 0);
+    });
+</script>
diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp
index ad6a9b30f60..f6b987169d1 100644
--- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp
+++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp
@@ -587,10 +587,18 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
         });
     }
 
-    main_thread_event_loop().spin_processing_tasks_with_source_until(Task::Source::NavigationAndTraversal, [&] {
+    auto check_if_document_population_tasks_completed = JS::SafeFunction<bool()>([&] {
         return changing_navigable_continuations.size() + completed_change_jobs == total_change_jobs;
     });
 
+    if (synchronous_navigation == SynchronousNavigation::Yes) {
+        // NOTE: Synchronous navigation should never require document population, so it is safe to process only NavigationAndTraversal source.
+        main_thread_event_loop().spin_processing_tasks_with_source_until(Task::Source::NavigationAndTraversal, move(check_if_document_population_tasks_completed));
+    } else {
+        // NOTE: Process all task sources while waiting because reloading or back/forward navigation might require fetching to populate a document.
+        main_thread_event_loop().spin_until(move(check_if_document_population_tasks_completed));
+    }
+
     // 13. Let navigablesThatMustWaitBeforeHandlingSyncNavigation be an empty set.
     Vector<JS::GCPtr<Navigable>> navigables_that_must_wait_before_handling_sync_navigation;