From b420b80073d2aeb2c620fb26ccde01a901e1acd1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Wed, 17 May 2017 16:00:31 +0200
Subject: [PATCH] Bug 1365471: Check bool flags before going through FFI in a
 variety of situations. r=bz
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MozReview-Commit-ID: E6mitc11tYa
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
---
 components/style/gecko/wrapper.rs | 58 +++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index c171404d3eb..bbca65b51d1 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -46,7 +46,7 @@ use gecko_bindings::bindings::Gecko_MatchStringArgPseudo;
 use gecko_bindings::bindings::Gecko_UpdateAnimations;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{RawGeckoElement, RawGeckoNode};
-use gecko_bindings::structs::{nsIAtom, nsIContent, nsStyleContext};
+use gecko_bindings::structs::{nsIAtom, nsIContent, nsINode_BooleanFlag, nsStyleContext};
 use gecko_bindings::structs::ELEMENT_HANDLED_SNAPSHOT;
 use gecko_bindings::structs::ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO;
 use gecko_bindings::structs::ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO;
@@ -104,14 +104,17 @@ impl<'ln> fmt::Debug for GeckoNode<'ln> {
 }
 
 impl<'ln> GeckoNode<'ln> {
+    #[inline]
     fn from_content(content: &'ln nsIContent) -> Self {
         GeckoNode(&content._base)
     }
 
+    #[inline]
     fn flags(&self) -> u32 {
         (self.0)._base._base_1.mFlags
     }
 
+    #[inline]
     fn node_info(&self) -> &structs::NodeInfo {
         debug_assert!(!self.0.mNodeInfo.mRawPtr.is_null());
         unsafe { &*self.0.mNodeInfo.mRawPtr }
@@ -119,32 +122,43 @@ impl<'ln> GeckoNode<'ln> {
 
     // These live in different locations depending on processor architecture.
     #[cfg(target_pointer_width = "64")]
+    #[inline]
     fn bool_flags(&self) -> u32 {
         (self.0)._base._base_1.mBoolFlags
     }
 
     #[cfg(target_pointer_width = "32")]
+    #[inline]
     fn bool_flags(&self) -> u32 {
         (self.0).mBoolFlags
     }
 
+    #[inline]
+    fn get_bool_flag(&self, flag: nsINode_BooleanFlag) -> bool {
+        self.bool_flags() & (1u32 << flag as u32) != 0
+    }
+
     fn owner_doc(&self) -> &structs::nsIDocument {
         debug_assert!(!self.node_info().mDocument.is_null());
         unsafe { &*self.node_info().mDocument }
     }
 
+    #[inline]
     fn first_child(&self) -> Option<GeckoNode<'ln>> {
         unsafe { self.0.mFirstChild.as_ref().map(GeckoNode::from_content) }
     }
 
+    #[inline]
     fn last_child(&self) -> Option<GeckoNode<'ln>> {
         unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) }
     }
 
+    #[inline]
     fn prev_sibling(&self) -> Option<GeckoNode<'ln>> {
         unsafe { self.0.mPreviousSibling.as_ref().map(GeckoNode::from_content) }
     }
 
+    #[inline]
     fn next_sibling(&self) -> Option<GeckoNode<'ln>> {
         unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) }
     }
@@ -186,9 +200,9 @@ impl<'ln> GeckoNode<'ln> {
 }
 
 impl<'ln> NodeInfo for GeckoNode<'ln> {
+    #[inline]
     fn is_element(&self) -> bool {
-        use gecko_bindings::structs::nsINode_BooleanFlag;
-        self.bool_flags() & (1u32 << nsINode_BooleanFlag::NodeIsElement as u32) != 0
+        self.get_bool_flag(nsINode_BooleanFlag::NodeIsElement)
     }
 
     fn is_text_node(&self) -> bool {
@@ -408,10 +422,24 @@ impl<'le> GeckoElement<'le> {
         }
     }
 
+    #[inline]
     fn may_have_animations(&self) -> bool {
-        use gecko_bindings::structs::nsINode_BooleanFlag;
-        self.as_node().bool_flags() &
-            (1u32 << nsINode_BooleanFlag::ElementHasAnimations as u32) != 0
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations)
+    }
+
+    #[inline]
+    fn has_id(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasID)
+    }
+
+    #[inline]
+    fn may_have_class(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementMayHaveClass)
+    }
+
+    #[inline]
+    fn may_have_style_attribute(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementMayHaveStyle)
     }
 }
 
@@ -564,8 +592,12 @@ impl<'le> TElement for GeckoElement<'le> {
     }
 
     fn style_attribute(&self) -> Option<&Arc<Locked<PropertyDeclarationBlock>>> {
+        if !self.may_have_style_attribute() {
+            return None;
+        }
+
         let declarations = unsafe { Gecko_GetStyleAttrDeclarationBlock(self.0) };
-        declarations.map(|s| s.as_arc_opt()).unwrap_or(None)
+        declarations.map_or(None, |s| s.as_arc_opt())
     }
 
     fn get_smil_override(&self) -> Option<&Arc<Locked<PropertyDeclarationBlock>>> {
@@ -1245,6 +1277,10 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
     }
 
     fn get_id(&self) -> Option<Atom> {
+        if !self.has_id() {
+            return None;
+        }
+
         let ptr = unsafe {
             bindings::Gecko_AtomAttrValue(self.0,
                                           atom!("id").as_ptr())
@@ -1258,6 +1294,10 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
     }
 
     fn has_class(&self, name: &Atom) -> bool {
+        if !self.may_have_class() {
+            return false;
+        }
+
         snapshot_helpers::has_class(self.0,
                                     name,
                                     Gecko_ClassOrClassList)
@@ -1266,6 +1306,10 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
     fn each_class<F>(&self, callback: F)
         where F: FnMut(&Atom)
     {
+        if !self.may_have_class() {
+            return;
+        }
+
         snapshot_helpers::each_class(self.0,
                                      callback,
                                      Gecko_ClassOrClassList)