Bug 1203973 - Move <style> and <link> attribute change handling to AfterSetAttr so that it doesn't trigger for no-op attribute changes. r=smaug, a=sylvestre
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 22 Sep 2015 21:19:49 -0400
changeset 296161 ab880d565bb0356017abedfae5aa4b91f0c37626
parent 296160 a3e1dc60e200004de0df58c8215736d2961c85c9
child 296162 e6ffbbdb9cc3b428778f1e8ea3530c87c48750f1
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, sylvestre
bugs1203973
milestone43.0a2
Bug 1203973 - Move <style> and <link> attribute change handling to AfterSetAttr so that it doesn't trigger for no-op attribute changes. r=smaug, a=sylvestre
dom/html/HTMLLinkElement.cpp
dom/html/HTMLLinkElement.h
dom/html/HTMLStyleElement.cpp
dom/html/HTMLStyleElement.h
--- a/dom/html/HTMLLinkElement.cpp
+++ b/dom/html/HTMLLinkElement.cpp
@@ -321,106 +321,87 @@ HTMLLinkElement::UpdatePreconnect()
     nsCOMPtr<nsIURI> uri = GetHrefURI();
     if (uri) {
         owner->MaybePreconnect(uri, GetCORSMode());
     }
   }
 }
 
 nsresult
-HTMLLinkElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                         nsIAtom* aPrefix, const nsAString& aValue,
-                         bool aNotify)
+HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                              const nsAttrValue* aValue, bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                              aValue, aNotify);
-
-  // The ordering of the parent class's SetAttr call and Link::ResetLinkState
-  // is important here!  The attribute is not set until SetAttr returns, and
-  // we will need the updated attribute value because notifying the document
-  // that content states have changed will call IntrinsicState, which will try
-  // to get updated information about the visitedness from Link.
+  // It's safe to call ResetLinkState here because our new attr value has
+  // already been set or unset.  ResetLinkState needs the updated attribute
+  // value because notifying the document that content states have changed will
+  // call IntrinsicState, which will try to get updated information about the
+  // visitedness from Link.
   if (aName == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) {
-    Link::ResetLinkState(!!aNotify, true);
+    bool hasHref = aValue;
+    Link::ResetLinkState(!!aNotify, hasHref);
     if (IsInUncomposedDoc()) {
       CreateAndDispatchEvent(OwnerDoc(), NS_LITERAL_STRING("DOMLinkChanged"));
     }
   }
 
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None &&
-      (aName == nsGkAtoms::href ||
-       aName == nsGkAtoms::rel ||
-       aName == nsGkAtoms::title ||
-       aName == nsGkAtoms::media ||
-       aName == nsGkAtoms::type)) {
-    bool dropSheet = false;
-    if (aName == nsGkAtoms::rel) {
-      uint32_t linkTypes = nsStyleLinkElement::ParseLinkTypes(aValue,
-                                                              NodePrincipal());
-      if (GetSheet()) {
-        dropSheet = !(linkTypes & nsStyleLinkElement::eSTYLESHEET);
-      } else if (linkTypes & eHTMLIMPORT) {
+  if (aValue) {
+    if (aNameSpaceID == kNameSpaceID_None &&
+        (aName == nsGkAtoms::href ||
+         aName == nsGkAtoms::rel ||
+         aName == nsGkAtoms::title ||
+         aName == nsGkAtoms::media ||
+         aName == nsGkAtoms::type)) {
+      bool dropSheet = false;
+      if (aName == nsGkAtoms::rel) {
+        nsAutoString value;
+        aValue->ToString(value);
+        uint32_t linkTypes = nsStyleLinkElement::ParseLinkTypes(value,
+                                                                NodePrincipal());
+        if (GetSheet()) {
+          dropSheet = !(linkTypes & nsStyleLinkElement::eSTYLESHEET);
+        } else if (linkTypes & eHTMLIMPORT) {
+          UpdateImport();
+        } else if ((linkTypes & ePRECONNECT) && IsInComposedDoc()) {
+          UpdatePreconnect();
+        }
+      }
+
+      if (aName == nsGkAtoms::href) {
         UpdateImport();
-      } else if ((linkTypes & ePRECONNECT) && IsInComposedDoc()) {
-        UpdatePreconnect();
+        if (IsInComposedDoc()) {
+          UpdatePreconnect();
+        }
       }
-    }
 
-    if (aName == nsGkAtoms::href) {
-      UpdateImport();
-      if (IsInComposedDoc()) {
-        UpdatePreconnect();
-      }
+      UpdateStyleSheetInternal(nullptr, nullptr,
+                               dropSheet ||
+                               (aName == nsGkAtoms::title ||
+                                aName == nsGkAtoms::media ||
+                                aName == nsGkAtoms::type));
     }
-    
-    UpdateStyleSheetInternal(nullptr, nullptr,
-                             dropSheet ||
-                             (aName == nsGkAtoms::title ||
-                              aName == nsGkAtoms::media ||
-                              aName == nsGkAtoms::type));
-  }
-
-  return rv;
-}
-
-nsresult
-HTMLLinkElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                           bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute,
-                                                aNotify);
-  // Since removing href or rel makes us no longer link to a
-  // stylesheet, force updates for those too.
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None) {
-    if (aAttribute == nsGkAtoms::href ||
-        aAttribute == nsGkAtoms::rel ||
-        aAttribute == nsGkAtoms::title ||
-        aAttribute == nsGkAtoms::media ||
-        aAttribute == nsGkAtoms::type) {
-      UpdateStyleSheetInternal(nullptr, nullptr, true);
-    }
-    if (aAttribute == nsGkAtoms::href ||
-        aAttribute == nsGkAtoms::rel) {
-      UpdateImport();
+  } else {
+    // Since removing href or rel makes us no longer link to a
+    // stylesheet, force updates for those too.
+    if (aNameSpaceID == kNameSpaceID_None) {
+      if (aName == nsGkAtoms::href ||
+          aName == nsGkAtoms::rel ||
+          aName == nsGkAtoms::title ||
+          aName == nsGkAtoms::media ||
+          aName == nsGkAtoms::type) {
+        UpdateStyleSheetInternal(nullptr, nullptr, true);
+      }
+      if (aName == nsGkAtoms::href ||
+          aName == nsGkAtoms::rel) {
+        UpdateImport();
+      }
     }
   }
 
-  // The ordering of the parent class's UnsetAttr call and Link::ResetLinkState
-  // is important here!  The attribute is not unset until UnsetAttr returns, and
-  // we will need the updated attribute value because notifying the document
-  // that content states have changed will call IntrinsicState, which will try
-  // to get updated information about the visitedness from Link.
-  if (aAttribute == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) {
-    Link::ResetLinkState(!!aNotify, false);
-    if (IsInUncomposedDoc()) {
-      CreateAndDispatchEvent(OwnerDoc(), NS_LITERAL_STRING("DOMLinkChanged"));
-    }
-  }
-
-  return rv;
+  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
+                                            aNotify);
 }
 
 nsresult
 HTMLLinkElement::PreHandleEvent(EventChainPreVisitor& aVisitor)
 {
   return PreHandleEventForAnchors(aVisitor);
 }
 
--- a/dom/html/HTMLLinkElement.h
+++ b/dom/html/HTMLLinkElement.h
@@ -56,26 +56,19 @@ public:
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   // nsIContent
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
-  nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                   const nsAString& aValue, bool aNotify)
-  {
-    return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify);
-  }
-  virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify) override;
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                bool aNotify) override;
   virtual bool IsLink(nsIURI** aURI) const override;
   virtual already_AddRefed<nsIURI> GetHrefURI() const override;
 
   // Element
   virtual bool ParseAttribute(int32_t aNamespaceID,
                               nsIAtom* aAttribute,
                               const nsAString& aValue,
                               nsAttrValue& aResult) override;
--- a/dom/html/HTMLStyleElement.cpp
+++ b/dom/html/HTMLStyleElement.cpp
@@ -167,52 +167,32 @@ HTMLStyleElement::UnbindFromTree(bool aD
     // do not need to be updated.
     return;
   }
 
   UpdateStyleSheetInternal(oldDoc, oldShadow);
 }
 
 nsresult
-HTMLStyleElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                          nsIAtom* aPrefix, const nsAString& aValue,
-                          bool aNotify)
+HTMLStyleElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                               const nsAttrValue* aValue, bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                              aValue, aNotify);
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None) {
+  if (aNameSpaceID == kNameSpaceID_None) {
     if (aName == nsGkAtoms::title ||
         aName == nsGkAtoms::media ||
         aName == nsGkAtoms::type) {
       UpdateStyleSheetInternal(nullptr, nullptr, true);
     } else if (aName == nsGkAtoms::scoped) {
-      UpdateStyleSheetScopedness(true);
+      bool isScoped = aValue;
+      UpdateStyleSheetScopedness(isScoped);
     }
   }
 
-  return rv;
-}
-
-nsresult
-HTMLStyleElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                            bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute,
-                                                aNotify);
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None) {
-    if (aAttribute == nsGkAtoms::title ||
-        aAttribute == nsGkAtoms::media ||
-        aAttribute == nsGkAtoms::type) {
-      UpdateStyleSheetInternal(nullptr, nullptr, true);
-    } else if (aAttribute == nsGkAtoms::scoped) {
-      UpdateStyleSheetScopedness(false);
-    }
-  }
-
-  return rv;
+  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
+                                            aNotify);
 }
 
 NS_IMETHODIMP
 HTMLStyleElement::GetInnerHTML(nsAString& aInnerHTML)
 {
   if (!nsContentUtils::GetNodeTextContent(this, false, aInnerHTML, fallible)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
--- a/dom/html/HTMLStyleElement.h
+++ b/dom/html/HTMLStyleElement.h
@@ -41,26 +41,19 @@ public:
   // nsIDOMHTMLStyleElement
   NS_DECL_NSIDOMHTMLSTYLEELEMENT
 
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
-  nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                   const nsAString& aValue, bool aNotify)
-  {
-    return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify);
-  }
-  virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify) override;
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                bool aNotify) override;
 
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const override;
 
   // nsIMutationObserver
   NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATACHANGED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED