From c3289ad46e93c7d4bd8ff17d874fbf367373dfaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Fri, 27 Jul 2018 17:44:48 +0200
Subject: [PATCH] style: Improve logging for attribute changes.

And general Element logging. We now print all the attributes for comparison.

If this turns out to be too verbose we can change it to diff them or something.

Differential Revision: https://phabricator.services.mozilla.com/D2471
---
 components/style/gecko/snapshot.rs            |  9 +++++
 components/style/gecko/wrapper.rs             | 27 ++++---------
 .../invalidation/element/element_wrapper.rs   |  5 +++
 .../element/state_and_attributes.rs           | 40 ++++++++++++-------
 4 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs
index bb0e4a99116..d70c6259bbf 100644
--- a/components/style/gecko/snapshot.rs
+++ b/components/style/gecko/snapshot.rs
@@ -156,6 +156,15 @@ impl GeckoElementSnapshot {
 }
 
 impl ElementSnapshot for GeckoElementSnapshot {
+    fn debug_list_attributes(&self) -> String {
+        use nsstring::nsCString;
+        let mut string = nsCString::new();
+        unsafe {
+            bindings::Gecko_Snapshot_DebugListAttributes(self, &mut string);
+        }
+        String::from_utf8_lossy(&*string).into_owned()
+    }
+
     fn state(&self) -> Option<ElementState> {
         if self.has_any(Flags::State) {
             Some(ElementState::from_bits_truncate(self.mState))
diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index 58768e17e83..eed9ebe00af 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -562,28 +562,15 @@ pub struct GeckoElement<'le>(pub &'le RawGeckoElement);
 
 impl<'le> fmt::Debug for GeckoElement<'le> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use nsstring::nsCString;
+
         write!(f, "<{}", self.local_name())?;
-        if let Some(id) = self.id() {
-            write!(f, " id={}", id)?;
+
+        let mut attrs = nsCString::new();
+        unsafe {
+            bindings::Gecko_Element_DebugListAttributes(self.0, &mut attrs);
         }
-
-        let mut first = true;
-        let mut any = false;
-        self.each_class(|c| {
-            if first {
-                first = false;
-                any = true;
-                let _ = f.write_str(" class=\"");
-            } else {
-                let _ = f.write_str(" ");
-            }
-            let _ = write!(f, "{}", c);
-        });
-
-        if any {
-            f.write_str("\"")?;
-        }
-
+        write!(f, "{}", attrs)?;
         write!(f, "> ({:#x})", self.as_node().opaque().0)
     }
 }
diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs
index ecbc3b94b78..b1e799d9efd 100644
--- a/components/style/invalidation/element/element_wrapper.rs
+++ b/components/style/invalidation/element/element_wrapper.rs
@@ -43,6 +43,11 @@ pub trait ElementSnapshot: Sized {
     /// If this snapshot contains attribute information.
     fn has_attrs(&self) -> bool;
 
+    /// Gets the attribute information of the snapshot as a string.
+    ///
+    /// Only for debugging purposes.
+    fn debug_list_attributes(&self) -> String { String::new() }
+
     /// The ID attribute per this snapshot. Should only be called if
     /// `has_attrs()` returns true.
     fn id_attr(&self) -> Option<&WeakAtom>;
diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs
index 73d73282910..a0f85ae30bb 100644
--- a/components/style/invalidation/element/state_and_attributes.rs
+++ b/components/style/invalidation/element/state_and_attributes.rs
@@ -208,20 +208,32 @@ where
             }
         }
 
-        debug!("Collecting changes for: {:?}", element);
-        debug!(" > state: {:?}", state_changes);
-        debug!(
-            " > id changed: {:?} -> +{:?} -{:?}",
-            snapshot.id_changed(),
-            id_added,
-            id_removed
-        );
-        debug!(
-            " > class changed: {:?} -> +{:?} -{:?}",
-            snapshot.class_changed(),
-            classes_added,
-            classes_removed
-        );
+        if log_enabled!(::log::Level::Debug) {
+            debug!("Collecting changes for: {:?}", element);
+            if !state_changes.is_empty() {
+                debug!(" > state: {:?}", state_changes);
+            }
+            if snapshot.id_changed() {
+                debug!(
+                    " > id changed: +{:?} -{:?}",
+                    id_added,
+                    id_removed
+                );
+            }
+            if snapshot.class_changed() {
+                debug!(
+                    " > class changed: +{:?} -{:?}",
+                    classes_added,
+                    classes_removed
+                );
+            }
+            if snapshot.other_attr_changed() {
+                debug!(
+                    " > attributes changed, old: {}",
+                    snapshot.debug_list_attributes()
+                )
+            }
+        }
 
         let lookup_element = if element.implemented_pseudo_element().is_some() {
             element.pseudo_element_originating_element().unwrap()