diff --git a/components/devtools/actors/browsing_context.rs b/components/devtools/actors/browsing_context.rs index 5875f039c66..261eb4112bf 100644 --- a/components/devtools/actors/browsing_context.rs +++ b/components/devtools/actors/browsing_context.rs @@ -10,7 +10,7 @@ use std::cell::{Cell, RefCell}; use std::collections::HashMap; use std::net::TcpStream; -use base::id::{BrowsingContextId, PipelineId, WebViewId}; +use base::id::PipelineId; use devtools_traits::DevtoolScriptControlMsg::{self, GetCssDatabase, WantsLiveNotifications}; use devtools_traits::{DevtoolsPageInfo, NavigationState}; use ipc_channel::ipc::{self, IpcSender}; @@ -26,6 +26,7 @@ use crate::actors::stylesheets::StyleSheetsActor; use crate::actors::tab::TabDescriptorActor; use crate::actors::thread::ThreadActor; use crate::actors::watcher::{SessionContext, SessionContextType, WatcherActor}; +use crate::id::{DevtoolsBrowserId, DevtoolsBrowsingContextId, DevtoolsOuterWindowId, IdMap}; use crate::protocol::JsonPacketStream; use crate::{EmptyReplyMsg, StreamId}; @@ -124,10 +125,11 @@ pub(crate) struct BrowsingContextActor { pub name: String, pub title: RefCell, pub url: RefCell, - /// This correspond to webview_id - pub browser_id: WebViewId, - pub active_pipeline: Cell, - pub browsing_context_id: BrowsingContextId, + /// This corresponds to webview_id + pub browser_id: DevtoolsBrowserId, + pub active_pipeline_id: Cell, + pub active_outer_window_id: Cell, + pub browsing_context_id: DevtoolsBrowsingContextId, pub accessibility: String, pub console: String, pub css_properties: String, @@ -169,19 +171,21 @@ impl Actor for BrowsingContextActor { self.streams.borrow_mut().remove(&id); if self.streams.borrow().is_empty() { self.script_chan - .send(WantsLiveNotifications(self.active_pipeline.get(), false)) + .send(WantsLiveNotifications(self.active_pipeline_id.get(), false)) .unwrap(); } } } impl BrowsingContextActor { + #[allow(clippy::too_many_arguments)] pub(crate) fn new( console: String, - browser_id: WebViewId, - browsing_context_id: BrowsingContextId, + browser_id: DevtoolsBrowserId, + browsing_context_id: DevtoolsBrowsingContextId, page_info: DevtoolsPageInfo, pipeline_id: PipelineId, + outer_window_id: DevtoolsOuterWindowId, script_sender: IpcSender, actors: &mut ActorRegistry, ) -> BrowsingContextActor { @@ -230,7 +234,8 @@ impl BrowsingContextActor { script_chan: script_sender, title: RefCell::new(title), url: RefCell::new(url.into_string()), - active_pipeline: Cell::new(pipeline_id), + active_pipeline_id: Cell::new(pipeline_id), + active_outer_window_id: Cell::new(outer_window_id), browser_id, browsing_context_id, accessibility: accessibility.name(), @@ -270,11 +275,9 @@ impl BrowsingContextActor { }, title: self.title.borrow().clone(), url: self.url.borrow().clone(), - browser_id: self.browser_id.0.index.0.get(), - //FIXME: shouldn't ignore pipeline namespace field - browsing_context_id: self.browsing_context_id.index.0.get(), - //FIXME: shouldn't ignore pipeline namespace field - outer_window_id: self.active_pipeline.get().index.0.get(), + browser_id: self.browser_id.value(), + browsing_context_id: self.browsing_context_id.value(), + outer_window_id: self.active_outer_window_id.get().value(), is_top_level_target: true, accessibility_actor: self.accessibility.clone(), console_actor: self.console.clone(), @@ -286,15 +289,17 @@ impl BrowsingContextActor { } } - pub(crate) fn navigate(&self, state: NavigationState) { - let (pipeline, title, url, state) = match state { + pub(crate) fn navigate(&self, state: NavigationState, id_map: &mut IdMap) { + let (pipeline_id, title, url, state) = match state { NavigationState::Start(url) => (None, None, url, "start"), NavigationState::Stop(pipeline, info) => { (Some(pipeline), Some(info.title), info.url, "stop") }, }; - if let Some(p) = pipeline { - self.active_pipeline.set(p); + if let Some(pipeline_id) = pipeline_id { + let outer_window_id = id_map.outer_window_id(pipeline_id); + self.active_outer_window_id.set(outer_window_id); + self.active_pipeline_id.set(pipeline_id); } url.as_str().clone_into(&mut self.url.borrow_mut()); if let Some(ref t) = title { @@ -317,7 +322,7 @@ impl BrowsingContextActor { } pub(crate) fn title_changed(&self, pipeline_id: PipelineId, title: String) { - if pipeline_id != self.active_pipeline.get() { + if pipeline_id != self.active_pipeline_id.get() { return; } *self.title.borrow_mut() = title; @@ -328,7 +333,7 @@ impl BrowsingContextActor { from: self.name(), type_: "frameUpdate".into(), frames: vec![FrameUpdateMsg { - id: self.browsing_context_id.index.0.get(), + id: self.browsing_context_id.value(), is_top_level: true, title: self.title.borrow().clone(), url: self.url.borrow().clone(), diff --git a/components/devtools/actors/console.rs b/components/devtools/actors/console.rs index bd22ef8e7b6..ecd718e47d4 100644 --- a/components/devtools/actors/console.rs +++ b/components/devtools/actors/console.rs @@ -152,7 +152,7 @@ impl ConsoleActor { Root::BrowsingContext(bc) => UniqueId::Pipeline( registry .find::(bc) - .active_pipeline + .active_pipeline_id .get(), ), Root::DedicatedWorker(w) => UniqueId::Worker(registry.find::(w).worker_id), diff --git a/components/devtools/actors/inspector.rs b/components/devtools/actors/inspector.rs index 0dd886ada9f..d41777f56c6 100644 --- a/components/devtools/actors/inspector.rs +++ b/components/devtools/actors/inspector.rs @@ -80,7 +80,7 @@ impl Actor for InspectorActor { _id: StreamId, ) -> Result { let browsing_context = registry.find::(&self.browsing_context); - let pipeline = browsing_context.active_pipeline.get(); + let pipeline = browsing_context.active_pipeline_id.get(); Ok(match msg_type { "getWalker" => { let (tx, rx) = ipc::channel().unwrap(); diff --git a/components/devtools/actors/tab.rs b/components/devtools/actors/tab.rs index 564259ab1ff..4a1c60b06c3 100644 --- a/components/devtools/actors/tab.rs +++ b/components/devtools/actors/tab.rs @@ -146,18 +146,15 @@ impl TabDescriptorActor { pub fn encodable(&self, registry: &ActorRegistry, selected: bool) -> TabDescriptorActorMsg { let ctx_actor = registry.find::(&self.browsing_context_actor); - let browser_id = ctx_actor.browser_id.0.index.0.get(); - let browsing_context_id = ctx_actor.browsing_context_id.index.0.get(); - let outer_window_id = ctx_actor.active_pipeline.get().index.0.get(); let title = ctx_actor.title.borrow().clone(); let url = ctx_actor.url.borrow().clone(); TabDescriptorActorMsg { actor: self.name(), - browser_id, - browsing_context_id, + browser_id: ctx_actor.browser_id.value(), + browsing_context_id: ctx_actor.browsing_context_id.value(), is_zombie_tab: false, - outer_window_id, + outer_window_id: ctx_actor.active_outer_window_id.get().value(), selected, title, traits: DescriptorTraits { diff --git a/components/devtools/actors/watcher.rs b/components/devtools/actors/watcher.rs index 94a15b36e7a..fab071faa2a 100644 --- a/components/devtools/actors/watcher.rs +++ b/components/devtools/actors/watcher.rs @@ -259,10 +259,9 @@ impl Actor for WatcherActor { ActorMessageStatus::Processed }, "getParentBrowsingContextID" => { - let browsing_context_id = target.browsing_context_id.index.0.get(); let msg = GetParentBrowsingContextIDReply { from: self.name(), - browsing_context_id, + browsing_context_id: target.browsing_context_id.value(), }; let _ = stream.write_json_packet(&msg); ActorMessageStatus::Processed diff --git a/components/devtools/id.rs b/components/devtools/id.rs new file mode 100644 index 00000000000..40408d91f95 --- /dev/null +++ b/components/devtools/id.rs @@ -0,0 +1,175 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use std::collections::HashMap; + +use base::id::{BrowsingContextId, PipelineId, WebViewId}; + +#[derive(Debug, Default)] +pub(crate) struct IdMap { + pub(crate) browser_ids: HashMap, + pub(crate) browsing_context_ids: HashMap, + pub(crate) outer_window_ids: HashMap, +} + +impl IdMap { + pub(crate) fn browser_id(&mut self, webview_id: WebViewId) -> DevtoolsBrowserId { + let len = self + .browser_ids + .len() + .checked_add(1) + .expect("WebViewId count overflow") + .try_into() + .expect("DevtoolsBrowserId overflow"); + DevtoolsBrowserId(*self.browser_ids.entry(webview_id).or_insert(len)) + } + pub(crate) fn browsing_context_id( + &mut self, + browsing_context_id: BrowsingContextId, + ) -> DevtoolsBrowsingContextId { + let len = self + .browsing_context_ids + .len() + .checked_add(1) + .expect("BrowsingContextId count overflow") + .try_into() + .expect("DevtoolsBrowsingContextId overflow"); + DevtoolsBrowsingContextId( + *self + .browsing_context_ids + .entry(browsing_context_id) + .or_insert(len), + ) + } + pub(crate) fn outer_window_id(&mut self, pipeline_id: PipelineId) -> DevtoolsOuterWindowId { + let len = self + .outer_window_ids + .len() + .checked_add(1) + .expect("PipelineId count overflow") + .try_into() + .expect("DevtoolsOuterWindowId overflow"); + DevtoolsOuterWindowId(*self.outer_window_ids.entry(pipeline_id).or_insert(len)) + } +} + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct DevtoolsBrowserId(u32); + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct DevtoolsBrowsingContextId(u32); + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct DevtoolsOuterWindowId(u32); + +impl DevtoolsBrowserId { + pub(crate) fn value(&self) -> u32 { + self.0 + } +} + +impl DevtoolsBrowsingContextId { + pub(crate) fn value(&self) -> u32 { + self.0 + } +} + +impl DevtoolsOuterWindowId { + pub(crate) fn value(&self) -> u32 { + self.0 + } +} + +#[test] +pub(crate) fn test_id_map() { + use std::thread; + + use base::id::{PipelineNamespace, PipelineNamespaceId}; + use crossbeam_channel::unbounded; + + macro_rules! test_sequential_id_assignment { + ($id_type:ident, $new_id_function:expr, $map_id_function:expr) => { + let (sender, receiver) = unbounded(); + let sender1 = sender.clone(); + let sender2 = sender.clone(); + let sender3 = sender.clone(); + let threads = [ + thread::spawn(move || { + PipelineNamespace::install(PipelineNamespaceId(1)); + sender1.send($new_id_function()).expect("Send failed"); + sender1.send($new_id_function()).expect("Send failed"); + sender1.send($new_id_function()).expect("Send failed"); + }), + thread::spawn(move || { + PipelineNamespace::install(PipelineNamespaceId(2)); + sender2.send($new_id_function()).expect("Send failed"); + sender2.send($new_id_function()).expect("Send failed"); + sender2.send($new_id_function()).expect("Send failed"); + }), + thread::spawn(move || { + PipelineNamespace::install(PipelineNamespaceId(3)); + sender3.send($new_id_function()).expect("Send failed"); + sender3.send($new_id_function()).expect("Send failed"); + sender3.send($new_id_function()).expect("Send failed"); + }), + ]; + for thread in threads { + thread.join().expect("Thread join failed"); + } + let mut id_map = IdMap::default(); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(1) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(2) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(3) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(4) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(5) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(6) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(7) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(8) + ); + assert_eq!( + $map_id_function(&mut id_map, receiver.recv().expect("Recv failed")), + $id_type(9) + ); + }; + } + + test_sequential_id_assignment!( + DevtoolsBrowserId, + || WebViewId::new(), + |id_map: &mut IdMap, id| id_map.browser_id(id) + ); + test_sequential_id_assignment!( + DevtoolsBrowsingContextId, + || BrowsingContextId::new(), + |id_map: &mut IdMap, id| id_map.browsing_context_id(id) + ); + test_sequential_id_assignment!( + DevtoolsOuterWindowId, + || PipelineId::new(), + |id_map: &mut IdMap, id| id_map.outer_window_id(id) + ); +} diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index 7e7de928ab1..4fc92457b04 100644 --- a/components/devtools/lib.rs +++ b/components/devtools/lib.rs @@ -44,6 +44,7 @@ use crate::actors::process::ProcessActor; use crate::actors::root::RootActor; use crate::actors::thread::ThreadActor; use crate::actors::worker::{WorkerActor, WorkerType}; +use crate::id::IdMap; use crate::network_handler::handle_network_event; use crate::protocol::JsonPacketStream; @@ -70,6 +71,7 @@ mod actors { pub mod watcher; pub mod worker; } +mod id; mod network_handler; mod protocol; @@ -106,6 +108,7 @@ pub(crate) struct StreamId(u32); struct DevtoolsInstance { actors: Arc>, + id_map: Arc>, browsing_contexts: HashMap, receiver: Receiver, pipelines: HashMap, @@ -167,6 +170,7 @@ impl DevtoolsInstance { let instance = Self { actors, + id_map: Arc::new(Mutex::new(IdMap::default())), browsing_contexts: HashMap::new(), pipelines: HashMap::new(), receiver, @@ -299,7 +303,7 @@ impl DevtoolsInstance { .lock() .unwrap() .find::(actor_name) - .navigate(state); + .navigate(state, &mut self.id_map.lock().expect("Mutex poisoned")); } // We need separate actor representations for each script global that exists; @@ -314,6 +318,10 @@ impl DevtoolsInstance { let mut actors = self.actors.lock().unwrap(); let (browsing_context_id, pipeline_id, worker_id, webview_id) = ids; + let id_map = &mut self.id_map.lock().expect("Mutex poisoned"); + let devtools_browser_id = id_map.browser_id(webview_id); + let devtools_browsing_context_id = id_map.browsing_context_id(browsing_context_id); + let devtools_outer_window_id = id_map.outer_window_id(pipeline_id); let console_name = actors.new_name("console"); @@ -351,10 +359,11 @@ impl DevtoolsInstance { .or_insert_with(|| { let browsing_context_actor = BrowsingContextActor::new( console_name.clone(), - webview_id, - browsing_context_id, + devtools_browser_id, + devtools_browsing_context_id, page_info, pipeline_id, + devtools_outer_window_id, script_sender, &mut actors, ); diff --git a/python/servo/testing_commands.py b/python/servo/testing_commands.py index 2094147df83..b19b4920336 100644 --- a/python/servo/testing_commands.py +++ b/python/servo/testing_commands.py @@ -163,6 +163,7 @@ class MachCommands(CommandBase): "background_hang_monitor", "base", "constellation", + "devtools", "fonts", "hyper_serde", "layout_2020",