Webdriver GoBack and GoForward commands wait for navigation complete (#37950)

After sending `GoBack` or `GoForward` command, webdriver wait for the
navigation complete.
It can be achieved by waiting for
`WebViewDelegate::notify_history_changed`

Testing: 
`tests/wpt/meta/webdriver/tests/classic/back/back.py`
`tests/wpt/meta/webdriver/tests/classic/forward/forward.py`

---------

Signed-off-by: batu_hoang <longvatrong111@gmail.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Signed-off-by: batu_hoang <hoang.binh.trong@huawei.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>

Cargo.lock

@@ -2261,6 +2261,7 @@ dependencies = [
  "stylo",
  "stylo_traits",
  "url",
+ "uuid",
  "webdriver",
  "webrender_api",
 ]

components/constellation/constellation.rs

@@ -1378,8 +1378,17 @@ where
                 self.embedder_proxy.send(EmbedderMsg::WebViewBlurred);
             },
             // Handle a forward or back request
-            EmbedderToConstellationMessage::TraverseHistory(webview_id, direction) => {
+            EmbedderToConstellationMessage::TraverseHistory(
+                webview_id,
+                direction,
+                traversal_id,
+            ) => {
                 self.handle_traverse_history_msg(webview_id, direction);
+                self.embedder_proxy
+                    .send(EmbedderMsg::HistoryTraversalComplete(
+                        webview_id,
+                        traversal_id,
+                    ));
             },
             EmbedderToConstellationMessage::ChangeViewportDetails(
                 webview_id,

components/constellation/tracing.rs

@@ -214,6 +214,7 @@ mod from_script {
                 Self::SetCursor(..) => target_variant!("SetCursor"),
                 Self::NewFavicon(..) => target_variant!("NewFavicon"),
                 Self::HistoryChanged(..) => target_variant!("HistoryChanged"),
+                Self::HistoryTraversalComplete(..) => target_variant!("HistoryTraversalComplete"),
                 Self::GetWindowRect(..) => target_variant!("GetWindowRect"),
                 Self::GetScreenMetrics(..) => target_variant!("GetScreenMetrics"),
                 Self::NotifyFullscreenStateChanged(..) => {

components/servo/lib.rs

@@ -801,6 +801,13 @@ impl Servo {
                     webview.set_load_status(load_status);
                 }
             },
+            EmbedderMsg::HistoryTraversalComplete(webview_id, traversal_id) => {
+                if let Some(webview) = self.get_webview_handle(webview_id) {
+                    webview
+                        .delegate()
+                        .notify_traversal_complete(webview.clone(), traversal_id);
+                }
+            },
             EmbedderMsg::HistoryChanged(webview_id, urls, current_index) => {
                 if let Some(webview) = self.get_webview_handle(webview_id) {
                     let urls: Vec<_> = urls.into_iter().map(ServoUrl::into_url).collect();

components/servo/webview.rs

@@ -14,7 +14,7 @@ use constellation_traits::{EmbedderToConstellationMessage, TraversalDirection};
 use dpi::PhysicalSize;
 use embedder_traits::{
     Cursor, InputEvent, JSValue, JavaScriptEvaluationError, LoadStatus, MediaSessionActionType,
-    ScreenGeometry, Theme, ViewportDetails,
+    ScreenGeometry, Theme, TraversalId, ViewportDetails,
 };
 use euclid::{Point2D, Scale, Size2D};
 use servo_geometry::DeviceIndependentPixel;
@@ -416,22 +416,28 @@ impl WebView {
             .send(EmbedderToConstellationMessage::Reload(self.id()))
     }
 
-    pub fn go_back(&self, amount: usize) {
+    pub fn go_back(&self, amount: usize) -> TraversalId {
+        let traversal_id = TraversalId::new();
         self.inner()
             .constellation_proxy
             .send(EmbedderToConstellationMessage::TraverseHistory(
                 self.id(),
                 TraversalDirection::Back(amount),
-            ))
+                traversal_id.clone(),
+            ));
+        traversal_id
     }
 
-    pub fn go_forward(&self, amount: usize) {
+    pub fn go_forward(&self, amount: usize) -> TraversalId {
+        let traversal_id = TraversalId::new();
         self.inner()
             .constellation_proxy
             .send(EmbedderToConstellationMessage::TraverseHistory(
                 self.id(),
                 TraversalDirection::Forward(amount),
-            ))
+                traversal_id.clone(),
+            ));
+        traversal_id
     }
 
     /// Ask the [`WebView`] to scroll web content. Note that positive scroll offsets reveal more

components/servo/webview_delegate.rs

@@ -10,7 +10,7 @@ use embedder_traits::{
     AllowOrDeny, AuthenticationResponse, ContextMenuResult, Cursor, FilterPattern,
     GamepadHapticEffectType, InputMethodType, KeyboardEvent, LoadStatus, MediaSessionEvent,
     Notification, PermissionFeature, RgbColor, ScreenGeometry, SelectElementOptionOrOptgroup,
-    SimpleDialog, WebResourceRequest, WebResourceResponse, WebResourceResponseMsg,
+    SimpleDialog, TraversalId, WebResourceRequest, WebResourceResponse, WebResourceResponseMsg,
 };
 use ipc_channel::ipc::IpcSender;
 use serde::Serialize;
@@ -438,6 +438,8 @@ pub trait WebViewDelegate {
     /// The history state has changed.
     // changed pattern; maybe wasteful if embedder doesn’t care?
     fn notify_history_changed(&self, _webview: WebView, _: Vec<Url>, _: usize) {}
+    /// A history traversal operation is complete.
+    fn notify_traversal_complete(&self, _webview: WebView, _: TraversalId) {}
     /// Page content has closed this [`WebView`] via `window.close()`. It's the embedder's
     /// responsibility to remove the [`WebView`] from the interface when this notification
     /// occurs.

components/shared/constellation/lib.rs

@@ -20,7 +20,7 @@ use base::cross_process_instant::CrossProcessInstant;
 use base::id::{MessagePortId, PipelineId, WebViewId};
 use embedder_traits::{
     CompositorHitTestResult, Cursor, InputEvent, JavaScriptEvaluationId, MediaSessionActionType,
-    Theme, ViewportDetails, WebDriverCommandMsg, WebDriverCommandResponse,
+    Theme, TraversalId, ViewportDetails, WebDriverCommandMsg, WebDriverCommandResponse,
 };
 pub use from_script_message::*;
 use ipc_channel::ipc::IpcSender;
@@ -48,7 +48,7 @@ pub enum EmbedderToConstellationMessage {
     /// Clear the network cache.
     ClearCache,
     /// Request to traverse the joint session history of the provided browsing context.
-    TraverseHistory(WebViewId, TraversalDirection),
+    TraverseHistory(WebViewId, TraversalDirection, TraversalId),
     /// Inform the Constellation that a `WebView`'s [`ViewportDetails`] have changed.
     ChangeViewportDetails(WebViewId, ViewportDetails, WindowSizeType),
     /// Inform the constellation of a theme change.

components/shared/embedder/Cargo.toml

@@ -37,6 +37,7 @@ strum_macros = { workspace = true }
 stylo_traits = { workspace = true }
 stylo = { workspace = true }
 url = { workspace = true }
+uuid = { workspace = true }
 webdriver = { workspace = true }
 webrender_api = { workspace = true }
 servo_geometry = { path = "../../geometry" }

components/shared/embedder/lib.rs

@@ -37,6 +37,7 @@ use strum_macros::IntoStaticStr;
 use style::queries::values::PrefersColorScheme;
 use style_traits::CSSPixel;
 use url::Url;
+use uuid::Uuid;
 use webrender_api::units::{DeviceIntPoint, DeviceIntRect, DeviceIntSize, DevicePixel};
 
 pub use crate::input_events::*;
@@ -318,6 +319,17 @@ pub struct ScreenMetrics {
     pub available_size: DeviceIndependentIntSize,
 }
 
+/// An opaque identifier for a single history traversal operation.
+#[derive(Clone, Deserialize, PartialEq, Serialize)]
+pub struct TraversalId(String);
+
+impl TraversalId {
+    #[allow(clippy::new_without_default)]
+    pub fn new() -> Self {
+        Self(Uuid::new_v4().to_string())
+    }
+}
+
 #[derive(Deserialize, IntoStaticStr, Serialize)]
 pub enum EmbedderMsg {
     /// A status message to be displayed by the browser chrome.
@@ -372,6 +384,8 @@ pub enum EmbedderMsg {
     NewFavicon(WebViewId, ServoUrl),
     /// The history state has changed.
     HistoryChanged(WebViewId, Vec<ServoUrl>, usize),
+    /// A history traversal operation completed.
+    HistoryTraversalComplete(WebViewId, TraversalId),
     /// Get the device independent window rectangle.
     GetWindowRect(WebViewId, IpcSender<DeviceIndependentIntRect>),
     /// Get the device independent screen size and available size.

components/shared/embedder/webdriver.rs

@@ -49,9 +49,9 @@ pub enum WebDriverCommandMsg {
     /// Refresh the top-level browsing context with the given ID.
     Refresh(WebViewId, IpcSender<WebDriverLoadStatus>),
     /// Navigate the webview with the given ID to the previous page in the browsing context's history.
-    GoBack(WebViewId),
+    GoBack(WebViewId, IpcSender<WebDriverLoadStatus>),
     /// Navigate the webview with the given ID to the next page in the browsing context's history.
-    GoForward(WebViewId),
+    GoForward(WebViewId, IpcSender<WebDriverLoadStatus>),
     /// Pass a webdriver command to the script thread of the current pipeline
     /// of a browsing context.
     ScriptCommand(BrowsingContextId, WebDriverScriptCommand),

components/webdriver_server/lib.rs

@@ -946,8 +946,11 @@ impl Handler {
         // return error with error code no such window.
         self.verify_top_level_browsing_context_is_open(webview_id)?;
 
-        self.send_message_to_embedder(WebDriverCommandMsg::GoBack(webview_id))?;
-        Ok(WebDriverResponse::Void)
+        self.send_message_to_embedder(WebDriverCommandMsg::GoBack(
+            webview_id,
+            self.load_status_sender.clone(),
+        ))?;
+        self.wait_for_load()
     }
 
     fn handle_go_forward(&self) -> WebDriverResult<WebDriverResponse> {
@@ -956,8 +959,11 @@ impl Handler {
         // return error with error code no such window.
         self.verify_top_level_browsing_context_is_open(webview_id)?;
 
-        self.send_message_to_embedder(WebDriverCommandMsg::GoForward(webview_id))?;
-        Ok(WebDriverResponse::Void)
+        self.send_message_to_embedder(WebDriverCommandMsg::GoForward(
+            webview_id,
+            self.load_status_sender.clone(),
+        ))?;
+        self.wait_for_load()
     }
 
     fn handle_refresh(&self) -> WebDriverResult<WebDriverResponse> {

ports/servoshell/desktop/app.rs

@@ -462,24 +462,34 @@ impl App {
                 },
                 WebDriverCommandMsg::LoadUrl(webview_id, url, load_status_sender) => {
                     if let Some(webview) = running_state.webview_by_id(webview_id) {
-                        webview.load(url.into_url());
                         running_state.set_load_status_sender(webview_id, load_status_sender);
+                        webview.load(url.into_url());
                     }
                 },
                 WebDriverCommandMsg::Refresh(webview_id, load_status_sender) => {
                     if let Some(webview) = running_state.webview_by_id(webview_id) {
-                        webview.reload();
                         running_state.set_load_status_sender(webview_id, load_status_sender);
+                        webview.reload();
                     }
                 },
-                WebDriverCommandMsg::GoBack(webview_id) => {
+                WebDriverCommandMsg::GoBack(webview_id, load_status_sender) => {
                     if let Some(webview) = running_state.webview_by_id(webview_id) {
-                        webview.go_back(1);
+                        let traversal_id = webview.go_back(1);
+                        running_state.set_pending_traversal(
+                            webview_id,
+                            traversal_id,
+                            load_status_sender,
+                        );
                     }
                 },
-                WebDriverCommandMsg::GoForward(webview_id) => {
+                WebDriverCommandMsg::GoForward(webview_id, load_status_sender) => {
                     if let Some(webview) = running_state.webview_by_id(webview_id) {
-                        webview.go_forward(1);
+                        let traversal_id = webview.go_forward(1);
+                        running_state.set_pending_traversal(
+                            webview_id,
+                            traversal_id,
+                            load_status_sender,
+                        );
                     }
                 },
                 // Key events don't need hit test so can be forwarded to constellation for now

ports/servoshell/desktop/app_state.rs

@@ -4,6 +4,7 @@
 
 use std::cell::{Ref, RefCell, RefMut};
 use std::collections::HashMap;
+use std::collections::hash_map::Entry;
 use std::path::PathBuf;
 use std::rc::Rc;
 
@@ -19,8 +20,8 @@ use servo::webrender_api::units::{DeviceIntPoint, DeviceIntSize};
 use servo::{
     AllowOrDenyRequest, AuthenticationRequest, FilterPattern, FormControl, GamepadHapticEffectType,
     KeyboardEvent, LoadStatus, PermissionRequest, Servo, ServoDelegate, ServoError, SimpleDialog,
-    WebDriverCommandMsg, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, WebView,
-    WebViewBuilder, WebViewDelegate,
+    TraversalId, WebDriverCommandMsg, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus,
+    WebView, WebViewBuilder, WebViewDelegate,
 };
 use url::Url;
 
@@ -44,6 +45,7 @@ pub(crate) enum AppState {
 struct WebDriverSenders {
     pub load_status_senders: HashMap<WebViewId, IpcSender<WebDriverLoadStatus>>,
     pub script_evaluation_interrupt_sender: Option<IpcSender<WebDriverJSResult>>,
+    pub pending_traversals: HashMap<WebViewId, (TraversalId, IpcSender<WebDriverLoadStatus>)>,
 }
 
 pub(crate) struct RunningAppState {
@@ -430,6 +432,18 @@ impl RunningAppState {
             });
     }
 
+    pub(crate) fn set_pending_traversal(
+        &self,
+        webview_id: WebViewId,
+        traversal_id: TraversalId,
+        sender: IpcSender<WebDriverLoadStatus>,
+    ) {
+        self.webdriver_senders
+            .borrow_mut()
+            .pending_traversals
+            .insert(webview_id, (traversal_id, sender));
+    }
+
     pub(crate) fn set_load_status_sender(
         &self,
         webview_id: WebViewId,
@@ -511,6 +525,16 @@ impl WebViewDelegate for RunningAppState {
         }
     }
 
+    fn notify_traversal_complete(&self, webview: servo::WebView, traversal_id: TraversalId) {
+        let mut webdriver_state = self.webdriver_senders.borrow_mut();
+        if let Entry::Occupied(entry) = webdriver_state.pending_traversals.entry(webview.id()) {
+            if entry.get().0 == traversal_id {
+                let (_, sender) = entry.remove();
+                let _ = sender.send(WebDriverLoadStatus::Complete);
+            }
+        }
+    }
+
     fn request_move_to(&self, _: servo::WebView, new_position: DeviceIntPoint) {
         self.inner().window.set_position(new_position);
     }

tests/wpt/meta/preload/preload-resource-match.https.html.ini

@@ -73,16 +73,16 @@
     expected: FAIL
 
   [Loading script (anonymous) with link (no-cors) should discard the preloaded response]
-    expected: FAIL
+    expected: TIMEOUT
 
   [Loading script (anonymous) with link (use-credentials) should discard the preloaded response]
-    expected: FAIL
+    expected: NOTRUN
 
   [Loading script (use-credentials) with link (no-cors) should discard the preloaded response]
-    expected: FAIL
+    expected: NOTRUN
 
   [Loading script (use-credentials) with link (anonymous) should discard the preloaded response]
-    expected: TIMEOUT
+    expected: NOTRUN
 
   [Loading script (use-credentials) with link (use-credentials) should reuse the preloaded response]
     expected: NOTRUN
@@ -146,3 +146,6 @@
 
   [Loading style (use-credentials) with link (use-credentials) should reuse the preloaded response]
     expected: NOTRUN
+
+  [Loading script (anonymous) with link (anonymous) should reuse the preloaded response]
+    expected: NOTRUN

tests/wpt/meta/webdriver/tests/classic/back/back.py.ini

@@ -1,11 +1,4 @@
 [back.py]
-  disabled: consistent panic
-  [test_no_top_browsing_context]
-    expected: FAIL
-
-  [test_no_browsing_context]
-    expected: ERROR
-
   [test_seen_nodes[http\]]
     expected: FAIL
 
@@ -14,6 +7,3 @@
 
   [test_seen_nodes[https coop\]]
     expected: FAIL
-
-  [test_history_pushstate]
-    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/forward/forward.py.ini

@@ -1,7 +1,4 @@
 [forward.py]
-  [test_basic]
-    expected: FAIL
-
   [test_seen_nodes[http\]]
     expected: FAIL
 
@@ -13,3 +10,6 @@
 
   [test_removed_iframe]
     expected: FAIL
+
+  [test_basic]
+    expected: FAIL