Change BrowingContextId from WebViewId explicitly (#39095)

There were still some accesses to the inner BrowsingContextId from the
WebViewId. This changes it to completely rely on the From trait for
these methods. This also means we can make the field private.

For testing we add a way to create arbitrary WebViewIds.

Testing: Does not change functionality.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>

components/compositing/webview_manager.rs

@@ -128,7 +128,7 @@ mod test {
     use crate::webview_renderer::UnknownWebView;
 
     fn top_level_id(namespace_id: u32, index: u32) -> WebViewId {
-        WebViewId(BrowsingContextId {
+        WebViewId::mock_for_testing(BrowsingContextId {
             namespace_id: PipelineNamespaceId(namespace_id),
             index: Index::new(index).unwrap(),
         })

components/constellation/webview_manager.rs

@@ -89,7 +89,7 @@ mod test {
     use crate::webview_manager::WebViewManager;
 
     fn id(namespace_id: u32, index: u32) -> WebViewId {
-        WebViewId(BrowsingContextId {
+        WebViewId::mock_for_testing(BrowsingContextId {
             namespace_id: PipelineNamespaceId(namespace_id),
             index: Index::new(index).expect("Incorrect test case"),
         })

components/net/http_loader.rs

@@ -470,7 +470,7 @@ pub fn send_response_values_to_devtools(
         request.pipeline_id,
         request.target_webview_id,
     ) {
-        let browsing_context_id = webview_id.0;
+        let browsing_context_id = webview_id.into();
 
         let devtoolsresponse = DevtoolsHttpResponse {
             headers,
@@ -494,7 +494,7 @@ pub fn send_response_values_to_devtools(
 pub fn send_early_httprequest_to_devtools(request: &Request, context: &FetchContext) {
     if let (Some(devtools_chan), Some(browsing_context_id), Some(pipeline_id)) = (
         context.devtools_chan.as_ref(),
-        request.target_webview_id.map(|id| id.0),
+        request.target_webview_id.map(|id| id.into()),
         request.pipeline_id,
     ) {
         // Build the partial DevtoolsHttpRequest
@@ -1965,7 +1965,7 @@ async fn http_network_fetch(
         let _ = fetch_terminated_sender.send(false);
     }
 
-    let browsing_context_id = request.target_webview_id.map(|id| id.0);
+    let browsing_context_id = request.target_webview_id.map(Into::into);
 
     let response_future = obtain_response(
         &context.state.client,

components/net/tests/fetch.rs

@@ -1342,7 +1342,7 @@ fn test_fetch_with_devtools() {
         send_time: devhttprequests.1.send_time,
         destination: Destination::None,
         is_xhr: true,
-        browsing_context_id: TEST_WEBVIEW_ID.0,
+        browsing_context_id: TEST_WEBVIEW_ID.into(),
     };
 
     let content = "Yay!";
@@ -1359,7 +1359,7 @@ fn test_fetch_with_devtools() {
         status: HttpStatus::default(),
         body: Some(content.as_bytes().to_vec()),
         pipeline_id: TEST_PIPELINE_ID,
-        browsing_context_id: TEST_WEBVIEW_ID.0,
+        browsing_context_id: TEST_WEBVIEW_ID.into(),
     };
 
     assert_eq!(devhttprequests.1, httprequest);

components/net/tests/http_loader.rs

@@ -402,7 +402,7 @@ fn test_request_and_response_data_with_network_messages() {
         send_time: devhttprequests.1.send_time,
         destination: Destination::Document,
         is_xhr: false,
-        browsing_context_id: TEST_WEBVIEW_ID.0,
+        browsing_context_id: TEST_WEBVIEW_ID.into(),
     };
 
     let content = "Yay!";
@@ -424,7 +424,7 @@ fn test_request_and_response_data_with_network_messages() {
         status: HttpStatus::default(),
         body: Some(content.as_bytes().to_vec()),
         pipeline_id: TEST_PIPELINE_ID,
-        browsing_context_id: TEST_WEBVIEW_ID.0,
+        browsing_context_id: TEST_WEBVIEW_ID.into(),
     };
 
     assert_eq!(devhttprequests.1, httprequest);

components/script/dom/window.rs

@@ -588,8 +588,9 @@ impl Window {
     /// Returns the window proxy of the webview, which is the top-level ancestor browsing context.
     /// <https://html.spec.whatwg.org/multipage/#top-level-browsing-context>
     pub(crate) fn webview_window_proxy(&self) -> Option<DomRoot<WindowProxy>> {
-        self.undiscarded_window_proxy()
+        self.undiscarded_window_proxy().and_then(|window_proxy| {
-            .and_then(|window_proxy| ScriptThread::find_window_proxy(window_proxy.webview_id().0))
+            ScriptThread::find_window_proxy(window_proxy.webview_id().into())
+        })
     }
 
     #[cfg(feature = "bluetooth")]

components/script/script_thread.rs

@@ -3477,7 +3477,8 @@ impl ScriptThread {
             .unwrap();
 
         // Notify devtools that a new script global exists.
-        let is_top_level_global = incomplete.webview_id.0 == incomplete.browsing_context_id;
+        let incomplete_browsing_context_id: BrowsingContextId = incomplete.webview_id.into();
+        let is_top_level_global = incomplete_browsing_context_id == incomplete.browsing_context_id;
         self.notify_devtools(
             document.Title(),
             final_url.clone(),

components/shared/base/id.rs

@@ -302,7 +302,7 @@ thread_local!(pub static WEBVIEW_ID: Cell<Option<WebViewId>> =
 #[derive(
     Clone, Copy, Deserialize, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd, Serialize,
 )]
-pub struct WebViewId(pub BrowsingContextId);
+pub struct WebViewId(BrowsingContextId);
 
 size_of_test!(WebViewId, 8);
 size_of_test!(Option<WebViewId>, 8);
@@ -333,6 +333,10 @@ impl WebViewId {
     pub fn installed() -> Option<WebViewId> {
         WEBVIEW_ID.with(|tls| tls.get())
     }
+
+    pub fn mock_for_testing(browsing_context_id: BrowsingContextId) -> WebViewId {
+        WebViewId(browsing_context_id)
+    }
 }
 
 impl From<WebViewId> for BrowsingContextId {