Bug 1470145: Better debugging for media-query related code and ua-cache. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 21 Jun 2018 15:19:48 +0200
changeset 480032 1440edb27dc4460e268387c57459506dbcbfe256
parent 480031 a1eb2582c7d29e2d6182e7eadaec56a91d6fe4ff
child 480033 393be8743349aa97b9c3413cdc164f2d976cf34d
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1470145
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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("document_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");
+
         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)]