webdriver: Assume more consistently that the BrowsingContext exists (#37389)

1. Tidy up some WebDriver handling with browsing context
2. Enable more wpt-test which no longer panic

Testing: `./mach test-wpt -r --log-raw "D:\servo test log\all.txt"
.\tests\wpt\tests\webdriver\tests\classic\ --product servodriver`

---------

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>

components/constellation/constellation.rs

@@ -4780,12 +4780,11 @@ where
             },
             WebDriverCommandMsg::Refresh(webview_id, response_sender) => {
                 let browsing_context_id = BrowsingContextId::from(webview_id);
-                let pipeline_id = match self.browsing_contexts.get(&browsing_context_id) {
+                let pipeline_id = self
-                    Some(browsing_context) => browsing_context.pipeline_id,
+                    .browsing_contexts
-                    None => {
+                    .get(&browsing_context_id)
-                        return warn!("{}: Refresh after closure", browsing_context_id);
+                    .expect("Refresh: Browsing context must exist at this point")
-                    },
+                    .pipeline_id;
-                };
                 let load_data = match self.pipelines.get(&pipeline_id) {
                     Some(pipeline) => pipeline.load_data.clone(),
                     None => return warn!("{}: Refresh after closure", pipeline_id),
@@ -4799,12 +4798,11 @@ where
             },
             // TODO: This should use the ScriptThreadMessage::EvaluateJavaScript command
             WebDriverCommandMsg::ScriptCommand(browsing_context_id, cmd) => {
-                let pipeline_id = match self.browsing_contexts.get(&browsing_context_id) {
+                let pipeline_id = self
-                    Some(browsing_context) => browsing_context.pipeline_id,
+                    .browsing_contexts
-                    None => {
+                    .get(&browsing_context_id)
-                        return warn!("{}: ScriptCommand after closure", browsing_context_id);
+                    .expect("ScriptCommand: Browsing context must exist at this point")
-                    },
+                    .pipeline_id;
-                };
                 let control_msg = ScriptThreadMessage::WebDriverScriptCommand(pipeline_id, cmd);
                 let result = match self.pipelines.get(&pipeline_id) {
                     Some(pipeline) => pipeline.event_loop.send(control_msg),
@@ -4815,12 +4813,11 @@ where
                 }
             },
             WebDriverCommandMsg::SendKeys(browsing_context_id, cmd) => {
-                let pipeline_id = match self.browsing_contexts.get(&browsing_context_id) {
+                let pipeline_id = self
-                    Some(browsing_context) => browsing_context.pipeline_id,
+                    .browsing_contexts
-                    None => {
+                    .get(&browsing_context_id)
-                        return warn!("{}: SendKeys after closure", browsing_context_id);
+                    .expect("SendKeys: Browsing context must exist at this point")
-                    },
+                    .pipeline_id;
-                };
                 let event_loop = match self.pipelines.get(&pipeline_id) {
                     Some(pipeline) => pipeline.event_loop.clone(),
                     None => return warn!("{}: SendKeys after closure", pipeline_id),

components/script/webdriver_handlers.rs

@@ -76,18 +76,10 @@ fn find_node_by_unique_id(
     pipeline: PipelineId,
     node_id: String,
 ) -> Result<DomRoot<Node>, ErrorStatus> {
-    match documents.find_document(pipeline) {
+    let doc = documents
-        Some(doc) => find_node_by_unique_id_in_document(&doc, node_id),
+        .find_document(pipeline)
-        None => {
+        .expect("webdriver_handlers::Document should exists");
-            // FIXME: This is unreacheable!! Because we already early return in Constellation
+    find_node_by_unique_id_in_document(&doc, node_id)
-            // To be Fixed soon
-            if ScriptThread::has_node_id(pipeline, &node_id) {
-                Err(ErrorStatus::StaleElementReference)
-            } else {
-                Err(ErrorStatus::NoSuchElement)
-            }
-        },
-    }
 }
 
 pub(crate) fn find_node_by_unique_id_in_document(

components/webdriver_server/actions.rs

@@ -11,6 +11,7 @@ use constellation_traits::EmbedderToConstellationMessage;
 use embedder_traits::{MouseButtonAction, WebDriverCommandMsg, WebDriverScriptCommand};
 use ipc_channel::ipc;
 use keyboard_types::webdriver::KeyInputState;
+use log::{error, info};
 use webdriver::actions::{
     ActionSequence, ActionsType, GeneralAction, KeyAction, KeyActionItem, KeyDownAction,
     KeyUpAction, NullActionItem, PointerAction, PointerActionItem, PointerDownAction,
@@ -158,7 +159,7 @@ impl Handler {
         }
 
         // Step 2. Return success with data null.
-        dbg!("Dispatch actions completed successfully");
+        info!("Dispatch actions completed successfully");
         Ok(())
     }
 
@@ -193,12 +194,12 @@ impl Handler {
                         .expect("Current id should be set before dispatch_actions_inner is called");
 
                     if current_waiting_id != response.id {
-                        dbg!("Dispatch actions completed with wrong id in response");
+                        error!("Dispatch actions completed with wrong id in response");
                         return Err(ErrorStatus::UnknownError);
                     }
                 },
                 Err(error) => {
-                    dbg!("Dispatch actions completed with IPC error: {:?}", error);
+                    error!("Dispatch actions completed with IPC error: {error}");
                     return Err(ErrorStatus::UnknownError);
                 },
             };

tests/wpt/meta/webdriver/tests/classic/close_window/close.py.ini

@@ -1,19 +1,9 @@
 [close.py]
-  disabled: panic in test_close_browsing_context_with_accepted_beforeunload_prompt
-  [test_no_top_browsing_context]
-    expected: FAIL
-
-  [test_no_browsing_context]
-    expected: FAIL
-
   [test_close_browsing_context_with_accepted_beforeunload_prompt[tab\]]
     expected: FAIL
 
   [test_close_browsing_context_with_accepted_beforeunload_prompt[window\]]
-    expected: ERROR
+    expected: FAIL
-
-  [test_close_last_browsing_context]
-    expected: ERROR
 
   [test_element_usage_after_closing_browsing_context]
     expected: ERROR

tests/wpt/meta/webdriver/tests/classic/element_click/center_point.py.ini

@@ -1,34 +0,0 @@
-[center_point.py]
-  expected: TIMEOUT
-  [test_entirely_in_view]
-    expected: FAIL
-
-  [test_css_pixel_rounding[1\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[2\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[3\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[4\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[5\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[6\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[7\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[8\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[9\]]
-    expected: FAIL
-
-  [test_css_pixel_rounding[10\]]
-    expected: FAIL

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

@@ -1,34 +1,12 @@
 [forward.py]
-  disabled: consistent panic
-  [test_no_top_browsing_context]
-    expected: ERROR
-
-  [test_no_browsing_context]
-    expected: ERROR
-
   [test_basic]
-    expected: ERROR
+    expected: FAIL
-
-  [test_no_browsing_history]
-    expected: ERROR
 
   [test_seen_nodes[http\]]
-    expected: ERROR
+    expected: FAIL
 
   [test_seen_nodes[https\]]
-    expected: ERROR
+    expected: FAIL
 
   [test_seen_nodes[https coop\]]
-    expected: ERROR
+    expected: FAIL
-
-  [test_history_pushstate]
-    expected: ERROR
-
-  [test_data_urls]
-    expected: ERROR
-
-  [test_fragments]
-    expected: ERROR
-
-  [test_removed_iframe]
-    expected: ERROR

tests/wpt/meta/webdriver/tests/classic/perform_actions/pointer_pen.py.ini

@@ -1,8 +1,4 @@
 [pointer_pen.py]
-  expected: TIMEOUT
-  [test_no_top_browsing_context]
-    expected: FAIL
-
   [test_no_browsing_context]
     expected: FAIL