constellation: clean up logging of pipelines and browsing contexts (#30657)

* constellation: clean up logging of pipelines and browsing contexts

* bring back sentence case; remove commented code

* fix more typos and wordings

components/compositing/compositor.rs

@@ -961,7 +961,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
 
     fn set_frame_tree(&mut self, frame_tree: &SendableFrameTree) {
         debug!(
-            "Setting the frame tree for pipeline {}",
+            "Setting the frame tree for pipeline {:?}",
             frame_tree.pipeline.id
         );
 

components/constellation/logging.rs

@@ -9,7 +9,7 @@ use std::thread;
 use backtrace::Backtrace;
 use compositing_traits::ConstellationMsg as FromCompositorMsg;
 use crossbeam_channel::Sender;
-use log::{debug, Level, LevelFilter, Log, Metadata, Record};
+use log::{Level, LevelFilter, Log, Metadata, Record};
 use msg::constellation_msg::TopLevelBrowsingContextId;
 use script_traits::{LogEntry, ScriptMsg as FromScriptMsg, ScriptToConstellationChan};
 use servo_remutex::ReentrantMutex;
@@ -53,7 +53,6 @@ impl Log for FromScriptLogger {
 
     fn log(&self, record: &Record) {
         if let Some(entry) = log_entry(record) {
-            debug!("Sending log entry {:?}.", entry);
             let thread_name = thread::current().name().map(ToOwned::to_owned);
             let msg = FromScriptMsg::LogEntry(thread_name, entry);
             let chan = self
@@ -95,7 +94,6 @@ impl Log for FromCompositorLogger {
 
     fn log(&self, record: &Record) {
         if let Some(entry) = log_entry(record) {
-            debug!("Sending log entry {:?}.", entry);
             let top_level_id = TopLevelBrowsingContextId::installed();
             let thread_name = thread::current().name().map(ToOwned::to_owned);
             let msg = FromCompositorMsg::LogEntry(top_level_id, thread_name, entry);

components/constellation/network_listener.rs

@@ -165,7 +165,7 @@ impl NetworkListener {
         if self.should_send {
             if let Err(e) = self.sender.send((self.pipeline_id, msg)) {
                 warn!(
-                    "Failed to forward network message to pipeline {}: {:?}",
+                    "Failed to forward network message to pipeline {:?}: {:?}",
                     self.pipeline_id, e
                 );
             }

components/constellation/session_history.rs

@@ -91,7 +91,7 @@ impl JointSessionHistory {
     }
 
     pub fn remove_entries_for_browsing_context(&mut self, context_id: BrowsingContextId) {
-        debug!("removing entries for context {}", context_id);
+        debug!("{}: Removing entries for browsing context", context_id);
         self.past.retain(|diff| match diff {
             SessionHistoryDiff::BrowsingContextDiff {
                 browsing_context_id,

components/script/script_thread.rs

@@ -794,7 +794,7 @@ impl ScriptThreadFactory for ScriptThread {
         let (sender, receiver) = unbounded();
         let layout_chan = sender.clone();
         thread::Builder::new()
-            .name(format!("Script{}", state.id))
+            .name(format!("Script{:?}", state.id))
             .spawn(move || {
                 thread_state::initialize(ThreadState::SCRIPT);
                 PipelineNamespace::install(state.pipeline_namespace_id);
@@ -836,7 +836,7 @@ impl ScriptThreadFactory for ScriptThread {
                 );
                 script_thread.pre_page_load(new_load, load_data);
 
-                let reporter_name = format!("script-reporter-{}", id);
+                let reporter_name = format!("script-reporter-{:?}", id);
                 mem_profiler_chan.run_with_memory_reporting(
                     || {
                         script_thread.start();

components/shared/msg/constellation_msg.rs

@@ -32,7 +32,7 @@ macro_rules! namespace_id_method {
 }
 
 macro_rules! namespace_id {
-    ($id_name:ident, $index_name:ident) => {
+    ($id_name:ident, $index_name:ident, $display_prefix:literal) => {
         #[derive(
             Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize,
         )]
@@ -40,22 +40,26 @@ macro_rules! namespace_id {
         malloc_size_of_is_0!($index_name);
 
         #[derive(
-            Clone,
+            Clone, Copy, Deserialize, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd, Serialize,
-            Copy,
-            Debug,
-            Deserialize,
-            Eq,
-            Hash,
-            MallocSizeOf,
-            Ord,
-            PartialEq,
-            PartialOrd,
-            Serialize,
         )]
         pub struct $id_name {
             pub namespace_id: PipelineNamespaceId,
             pub index: $index_name,
         }
+
+        impl fmt::Debug for $id_name {
+            fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+                let PipelineNamespaceId(namespace_id) = self.namespace_id;
+                let $index_name(index) = self.index;
+                write!(fmt, "({},{})", namespace_id, index.get())
+            }
+        }
+
+        impl fmt::Display for $id_name {
+            fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+                write!(fmt, "{}{:?}", $display_prefix, self)
+            }
+        }
     };
 }
 
@@ -200,7 +204,7 @@ thread_local!(pub static PIPELINE_NAMESPACE: Cell<Option<PipelineNamespace>> = C
 )]
 pub struct PipelineNamespaceId(pub u32);
 
-namespace_id! {PipelineId, PipelineIndex}
+namespace_id! {PipelineId, PipelineIndex, "Pipeline"}
 
 size_of_test!(PipelineId, 8);
 size_of_test!(Option<PipelineId>, 8);
@@ -237,15 +241,7 @@ impl PipelineId {
     }
 }
 
-impl fmt::Display for PipelineId {
+namespace_id! {BrowsingContextId, BrowsingContextIndex, "BrowsingContext"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let PipelineIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index.get())
-    }
-}
-
-namespace_id! {BrowsingContextId, BrowsingContextIndex}
 
 size_of_test!(BrowsingContextId, 8);
 size_of_test!(Option<BrowsingContextId>, 8);
@@ -261,27 +257,31 @@ impl BrowsingContextId {
     }
 }
 
-impl fmt::Display for BrowsingContextId {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let BrowsingContextIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index.get())
-    }
-}
-
 #[derive(Clone, Default, Eq, Hash, PartialEq)]
 pub struct BrowsingContextGroupId(pub u32);
 
 thread_local!(pub static TOP_LEVEL_BROWSING_CONTEXT_ID: Cell<Option<TopLevelBrowsingContextId>> = Cell::new(None));
 
 #[derive(
-    Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd, Serialize,
+    Clone, Copy, Deserialize, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd, Serialize,
 )]
 pub struct TopLevelBrowsingContextId(pub BrowsingContextId);
 
 size_of_test!(TopLevelBrowsingContextId, 8);
 size_of_test!(Option<TopLevelBrowsingContextId>, 8);
 
+impl fmt::Debug for TopLevelBrowsingContextId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "TopLevel{:?}", self.0)
+    }
+}
+
+impl fmt::Display for TopLevelBrowsingContextId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "TopLevel{}", self.0)
+    }
+}
+
 impl TopLevelBrowsingContextId {
     pub fn new() -> TopLevelBrowsingContextId {
         TopLevelBrowsingContextId(BrowsingContextId::new())
@@ -297,12 +297,6 @@ impl TopLevelBrowsingContextId {
     }
 }
 
-impl fmt::Display for TopLevelBrowsingContextId {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        self.0.fmt(fmt)
-    }
-}
-
 impl From<TopLevelBrowsingContextId> for BrowsingContextId {
     fn from(id: TopLevelBrowsingContextId) -> BrowsingContextId {
         id.0
@@ -321,7 +315,7 @@ impl PartialEq<BrowsingContextId> for TopLevelBrowsingContextId {
     }
 }
 
-namespace_id! {MessagePortId, MessagePortIndex}
+namespace_id! {MessagePortId, MessagePortIndex, "MessagePort"}
 
 impl MessagePortId {
     pub fn new() -> MessagePortId {
@@ -334,15 +328,7 @@ impl MessagePortId {
     }
 }
 
-impl fmt::Display for MessagePortId {
+namespace_id! {MessagePortRouterId, MessagePortRouterIndex, "MessagePortRouter"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let MessagePortIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index.get())
-    }
-}
-
-namespace_id! {MessagePortRouterId, MessagePortRouterIndex}
 
 impl MessagePortRouterId {
     pub fn new() -> MessagePortRouterId {
@@ -355,15 +341,7 @@ impl MessagePortRouterId {
     }
 }
 
-impl fmt::Display for MessagePortRouterId {
+namespace_id! {BroadcastChannelRouterId, BroadcastChannelRouterIndex, "BroadcastChannelRouter"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let MessagePortRouterIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index.get())
-    }
-}
-
-namespace_id! {BroadcastChannelRouterId, BroadcastChannelRouterIndex}
 
 impl BroadcastChannelRouterId {
     pub fn new() -> BroadcastChannelRouterId {
@@ -376,20 +354,7 @@ impl BroadcastChannelRouterId {
     }
 }
 
-impl fmt::Display for BroadcastChannelRouterId {
+namespace_id! {ServiceWorkerId, ServiceWorkerIndex, "ServiceWorker"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let BroadcastChannelRouterIndex(index) = self.index;
-        write!(
-            fmt,
-            "(BroadcastChannelRouterId{},{})",
-            namespace_id,
-            index.get()
-        )
-    }
-}
-
-namespace_id! {ServiceWorkerId, ServiceWorkerIndex}
 
 impl ServiceWorkerId {
     pub fn new() -> ServiceWorkerId {
@@ -402,15 +367,7 @@ impl ServiceWorkerId {
     }
 }
 
-impl fmt::Display for ServiceWorkerId {
+namespace_id! {ServiceWorkerRegistrationId, ServiceWorkerRegistrationIndex, "ServiceWorkerRegistration"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let ServiceWorkerIndex(index) = self.index;
-        write!(fmt, "(ServiceWorkerId{},{})", namespace_id, index.get())
-    }
-}
-
-namespace_id! {ServiceWorkerRegistrationId, ServiceWorkerRegistrationIndex}
 
 impl ServiceWorkerRegistrationId {
     pub fn new() -> ServiceWorkerRegistrationId {
@@ -424,20 +381,7 @@ impl ServiceWorkerRegistrationId {
     }
 }
 
-impl fmt::Display for ServiceWorkerRegistrationId {
+namespace_id! {BlobId, BlobIndex, "Blob"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let ServiceWorkerRegistrationIndex(index) = self.index;
-        write!(
-            fmt,
-            "(ServiceWorkerRegistrationId{},{})",
-            namespace_id,
-            index.get()
-        )
-    }
-}
-
-namespace_id! {BlobId, BlobIndex}
 
 impl BlobId {
     pub fn new() -> BlobId {
@@ -450,15 +394,7 @@ impl BlobId {
     }
 }
 
-impl fmt::Display for BlobId {
+namespace_id! {HistoryStateId, HistoryStateIndex, "HistoryState"}
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let BlobIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index.get())
-    }
-}
-
-namespace_id! {HistoryStateId, HistoryStateIndex}
 
 impl HistoryStateId {
     pub fn new() -> HistoryStateId {
@@ -471,14 +407,6 @@ impl HistoryStateId {
     }
 }
 
-impl fmt::Display for HistoryStateId {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let PipelineNamespaceId(namespace_id) = self.namespace_id;
-        let HistoryStateIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index.get())
-    }
-}
-
 // We provide ids just for unit testing.
 pub const TEST_NAMESPACE: PipelineNamespaceId = PipelineNamespaceId(1234);
 #[allow(unsafe_code)]