Bug 1470145: Better debugging for media-query related code and ua-cache. r=xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 21 Jun 2018 15:19:48 +0200
changeset 809208 c41a55e25bc2d38089e08af88278a9876dbd4be1
parent 809207 528d2fede47228848264b9b5f5eff96ef9b9cc7a
push id113579
push userbmo:emilio@crisal.io
push dateThu, 21 Jun 2018 13:28:38 +0000
reviewersxidorn
bugs1470145
milestone62.0a1
Bug 1470145: Better debugging for media-query related code and ua-cache. r=xidorn MozReview-Commit-ID: 3XHAxK2BOTS
servo/components/style/gecko/media_queries.rs
servo/components/style/media_queries/media_list.rs
servo/components/style/stylist.rs
servo/components/style_derive/to_css.rs
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -59,16 +59,37 @@ pub struct Device {
     /// Whether any styles computed in the document relied on the root font-size
     /// by using rem units.
     used_root_font_size: AtomicBool,
     /// Whether any styles computed in the document relied on the viewport size
     /// by using vw/vh/vmin/vmax units.
     used_viewport_size: AtomicBool,
 }
 
+impl fmt::Debug for Device {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use nsstring::nsCString;
+
+        let mut doc_uri = nsCString::new();
+        unsafe {
+            let doc =
+                &*self.pres_context().mDocument.raw::<structs::nsIDocument>();
+
+            bindings::Gecko_nsIURI_Debug(
+                doc.mDocumentURI.raw::<structs::nsIURI>(),
+                &mut doc_uri,
+            )
+        };
+
+        f.debug_struct("Device")
+            .field("url", &doc_uri)
+            .finish()
+    }
+}
+
 unsafe impl Sync for Device {}
 unsafe impl Send for Device {}
 
 impl Device {
     /// Trivially constructs a new `Device`.
     pub fn new(pres_context: RawGeckoPresContextOwned) -> Self {
         assert!(!pres_context.is_null());
         Device {
--- a/servo/components/style/media_queries/media_list.rs
+++ b/servo/components/style/media_queries/media_list.rs
@@ -9,18 +9,18 @@
 use context::QuirksMode;
 use cssparser::{Delimiter, Parser};
 use cssparser::{ParserInput, Token};
 use error_reporting::ContextualParseError;
 use parser::ParserContext;
 use super::{Device, MediaQuery, Qualifier};
 
 /// A type that encapsulates a media query list.
-#[css(comma)]
-#[derive(Clone, Debug, MallocSizeOf, ToCss)]
+#[css(comma, derive_debug)]
+#[derive(Clone, MallocSizeOf, ToCss)]
 pub struct MediaList {
     /// The list of media queries.
     #[css(iterable)]
     pub media_queries: Vec<MediaQuery>,
 }
 
 impl MediaList {
     /// Parse a media query list from CSS.
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -70,59 +70,65 @@ struct UserAgentCascadeDataCache {
     entries: Vec<Arc<UserAgentCascadeData>>,
 }
 
 impl UserAgentCascadeDataCache {
     fn new() -> Self {
         Self { entries: vec![] }
     }
 
+    fn len(&self) -> usize {
+        self.entries.len()
+    }
+
     // FIXME(emilio): This may need to be keyed on quirks-mode too, though there
     // aren't class / id selectors on those sheets, usually, so it's probably
     // ok...
     fn lookup<'a, I, S>(
         &'a mut self,
         sheets: I,
         device: &Device,
         quirks_mode: QuirksMode,
         guard: &SharedRwLockReadGuard,
     ) -> Result<Arc<UserAgentCascadeData>, FailedAllocationError>
     where
         I: Iterator<Item = &'a S> + Clone,
         S: StylesheetInDocument + ToMediaListKey + PartialEq + 'static,
     {
         let mut key = EffectiveMediaQueryResults::new();
+        debug!("UserAgentCascadeDataCache::lookup({:?})", device);
         for sheet in sheets.clone() {
             CascadeData::collect_applicable_media_query_results_into(device, sheet, guard, &mut key)
         }
 
         for entry in &self.entries {
             if entry.cascade_data.effective_media_query_results == key {
                 return Ok(entry.clone());
             }
         }
 
         let mut new_data = UserAgentCascadeData {
             cascade_data: CascadeData::new(),
             precomputed_pseudo_element_decls: PrecomputedPseudoElementDeclarations::default(),
         };
 
+        debug!("> Picking the slow path", device);
+
         for sheet in sheets {
             new_data.cascade_data.add_stylesheet(
                 device,
                 quirks_mode,
                 sheet,
                 guard,
                 SheetRebuildKind::Full,
                 Some(&mut new_data.precomputed_pseudo_element_decls),
             )?;
         }
 
         let new_data = Arc::new(new_data);
-
         self.entries.push(new_data.clone());
         Ok(new_data)
     }
 
     fn expire_unused(&mut self) {
         self.entries.retain(|e| !e.is_unique())
     }
 
@@ -239,18 +245,18 @@ impl DocumentCascadeData {
     {
         // First do UA sheets.
         {
             if flusher.flush_origin(Origin::UserAgent).dirty() {
                 let mut ua_cache = UA_CASCADE_DATA_CACHE.lock().unwrap();
                 let origin_sheets = flusher.origin_sheets(Origin::UserAgent);
                 let ua_cascade_data =
                     ua_cache.lookup(origin_sheets, device, quirks_mode, guards.ua_or_user)?;
-
                 ua_cache.expire_unused();
+                debug!("User agent data cache size {:?}", ua_cache.len());
                 self.user_agent = ua_cascade_data;
             }
         }
 
         // Now do the user sheets.
         self.user.rebuild(
             device,
             quirks_mode,
@@ -1080,17 +1086,17 @@ impl Stylist {
     /// document itself, which is what is kept up-to-date.
     ///
     /// Arguably XBL should use something more lightweight than a Stylist.
     pub fn media_features_change_changed_style(
         &self,
         guards: &StylesheetGuards,
         device: &Device,
     ) -> OriginSet {
-        debug!("Stylist::media_features_change_changed_style");
+        debug!("Stylist::media_features_change_changed_style {:?}", device);
 
         let mut origins = OriginSet::empty();
         let stylesheets = self.stylesheets.iter();
 
         for (stylesheet, origin) in stylesheets {
             if origins.contains(origin.into()) {
                 continue;
             }
@@ -2140,26 +2146,29 @@ impl CascadeData {
         results: &mut EffectiveMediaQueryResults,
     ) where
         S: StylesheetInDocument + ToMediaListKey + 'static,
     {
         if !stylesheet.enabled() || !stylesheet.is_effective_for_device(device, guard) {
             return;
         }
 
+        debug!(" + {:?}", stylesheet);
         results.saw_effective(stylesheet);
 
         for rule in stylesheet.effective_rules(device, guard) {
             match *rule {
                 CssRule::Import(ref lock) => {
                     let import_rule = lock.read_with(guard);
+                    debug!(" + {:?}", import_rule.stylesheet.media(guard));
                     results.saw_effective(import_rule);
                 },
                 CssRule::Media(ref lock) => {
                     let media_rule = lock.read_with(guard);
+                    debug!(" + {:?}", media_rule.media_queries.read_with(guard));
                     results.saw_effective(media_rule);
                 },
                 _ => {},
             }
         }
     }
 
     // Returns Err(..) to signify OOM
@@ -2341,18 +2350,20 @@ impl CascadeData {
         use invalidation::media_queries::PotentiallyEffectiveMediaRules;
 
         let effective_now = stylesheet.is_effective_for_device(device, guard);
 
         let effective_then = self.effective_media_query_results.was_effective(stylesheet);
 
         if effective_now != effective_then {
             debug!(
-                " > Stylesheet changed -> {}, {}",
-                effective_then, effective_now
+                " > Stylesheet {:?} changed -> {}, {}",
+                stylesheet.media(guard),
+                effective_then,
+                effective_now
             );
             return false;
         }
 
         if !effective_now {
             return true;
         }
 
@@ -2377,18 +2388,20 @@ impl CascadeData {
                     let import_rule = lock.read_with(guard);
                     let effective_now = import_rule
                         .stylesheet
                         .is_effective_for_device(&device, guard);
                     let effective_then = self.effective_media_query_results
                         .was_effective(import_rule);
                     if effective_now != effective_then {
                         debug!(
-                            " > @import rule changed {} -> {}",
-                            effective_then, effective_now
+                            " > @import rule {:?} changed {} -> {}",
+                            import_rule.stylesheet.media(guard),
+                            effective_then,
+                            effective_now
                         );
                         return false;
                     }
 
                     if !effective_now {
                         iter.skip_children();
                     }
                 },
@@ -2396,18 +2409,20 @@ impl CascadeData {
                     let media_rule = lock.read_with(guard);
                     let mq = media_rule.media_queries.read_with(guard);
                     let effective_now = mq.evaluate(device, quirks_mode);
                     let effective_then =
                         self.effective_media_query_results.was_effective(media_rule);
 
                     if effective_now != effective_then {
                         debug!(
-                            " > @media rule changed {} -> {}",
-                            effective_then, effective_now
+                            " > @media rule {:?} changed {} -> {}",
+                            mq,
+                            effective_then,
+                            effective_now
                         );
                         return false;
                     }
 
                     if !effective_now {
                         iter.skip_children();
                     }
                 },
--- a/servo/components/style_derive/to_css.rs
+++ b/servo/components/style_derive/to_css.rs
@@ -226,16 +226,18 @@ pub struct CssInputAttrs {
     // Here because structs variants are also their whole type definition.
     pub comma: bool,
 }
 
 #[darling(attributes(css), default)]
 #[derive(Default, FromVariant)]
 pub struct CssVariantAttrs {
     pub function: Option<Override<String>>,
+    // Here because structs variants are also their whole type definition.
+    pub derive_debug: bool,
     pub comma: bool,
     pub dimension: bool,
     pub keyword: Option<String>,
     pub skip: bool,
 }
 
 #[darling(attributes(css), default)]
 #[derive(Default, FromField)]