From b73c81630ab1d1e1a931dca84d70f997c12c286b Mon Sep 17 00:00:00 2001 From: Narfinger Date: Wed, 3 Sep 2025 14:18:08 +0200 Subject: [PATCH] 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 --- components/compositing/webview_manager.rs | 2 +- components/constellation/webview_manager.rs | 2 +- components/net/http_loader.rs | 6 +++--- components/net/tests/fetch.rs | 4 ++-- components/net/tests/http_loader.rs | 4 ++-- components/script/dom/window.rs | 5 +++-- components/script/script_thread.rs | 3 ++- components/shared/base/id.rs | 6 +++++- 8 files changed, 19 insertions(+), 13 deletions(-) diff --git a/components/compositing/webview_manager.rs b/components/compositing/webview_manager.rs index 953d6e3bd5d..fc03e0aad1c 100644 --- a/components/compositing/webview_manager.rs +++ b/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(), }) diff --git a/components/constellation/webview_manager.rs b/components/constellation/webview_manager.rs index f6673977740..e28e54d59a8 100644 --- a/components/constellation/webview_manager.rs +++ b/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"), }) diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 70de8b35d74..8bc519f402e 100644 --- a/components/net/http_loader.rs +++ b/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, diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index aacc25bf182..27524f23dd2 100644 --- a/components/net/tests/fetch.rs +++ b/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); diff --git a/components/net/tests/http_loader.rs b/components/net/tests/http_loader.rs index 7b536dd4010..62ee090225d 100644 --- a/components/net/tests/http_loader.rs +++ b/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); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 990ae915eaa..c0247a3999f 100644 --- a/components/script/dom/window.rs +++ b/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. /// pub(crate) fn webview_window_proxy(&self) -> Option> { - self.undiscarded_window_proxy() - .and_then(|window_proxy| ScriptThread::find_window_proxy(window_proxy.webview_id().0)) + self.undiscarded_window_proxy().and_then(|window_proxy| { + ScriptThread::find_window_proxy(window_proxy.webview_id().into()) + }) } #[cfg(feature = "bluetooth")] diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index eba18757f67..ef257477bb4 100644 --- a/components/script/script_thread.rs +++ b/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(), diff --git a/components/shared/base/id.rs b/components/shared/base/id.rs index a3dd57a973b..c0c9f203201 100644 --- a/components/shared/base/id.rs +++ b/components/shared/base/id.rs @@ -302,7 +302,7 @@ thread_local!(pub static WEBVIEW_ID: Cell> = #[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, 8); @@ -333,6 +333,10 @@ impl WebViewId { pub fn installed() -> Option { WEBVIEW_ID.with(|tls| tls.get()) } + + pub fn mock_for_testing(browsing_context_id: BrowsingContextId) -> WebViewId { + WebViewId(browsing_context_id) + } } impl From for BrowsingContextId {