Bug 1365092 - Move side effects of SetAttr, UnsetAttr, and ParseAttribute functions to BeforeSetAttr and AfterSetAttr r=bz
authorKirk Steuber <ksteuber@mozilla.com>
Wed, 07 Jun 2017 10:28:20 -0700
changeset 363017 f5da859b433fe40803847ad77cc4a573fbf8dc25
parent 363016 c0a03c5be17267f19ed9b925e5faf471eebe7802
child 363018 dcdff55098560d60df1ec0a7d26e8e1303b1c6e8
push id31994
push usercbook@mozilla.com
push dateFri, 09 Jun 2017 10:56:24 +0000
treeherdermozilla-central@7c9d96bbc400 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1365092
milestone55.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 1365092 - Move side effects of SetAttr, UnsetAttr, and ParseAttribute functions to BeforeSetAttr and AfterSetAttr r=bz This is necessary to facilitate the transition to cloning attributes instead of reparsing them. MozReview-Commit-ID: Gyd1tD6ldly
dom/base/nsStyledElement.cpp
dom/base/nsStyledElement.h
dom/html/HTMLAreaElement.cpp
dom/html/HTMLAreaElement.h
dom/html/HTMLCanvasElement.cpp
dom/html/HTMLCanvasElement.h
dom/html/HTMLContentElement.cpp
dom/html/HTMLContentElement.h
dom/html/HTMLFormElement.cpp
dom/html/HTMLFormElement.h
dom/html/HTMLFrameSetElement.cpp
dom/html/HTMLFrameSetElement.h
dom/html/HTMLIFrameElement.cpp
dom/html/HTMLIFrameElement.h
dom/html/HTMLLegendElement.cpp
dom/html/HTMLLegendElement.h
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/html/HTMLObjectElement.cpp
dom/html/HTMLObjectElement.h
dom/html/HTMLSelectElement.cpp
dom/html/HTMLSelectElement.h
dom/html/HTMLSharedElement.cpp
dom/html/HTMLSharedElement.h
dom/html/HTMLSharedObjectElement.cpp
dom/html/HTMLSharedObjectElement.h
dom/html/nsGenericHTMLFrameElement.cpp
dom/html/nsGenericHTMLFrameElement.h
dom/mathml/nsMathMLElement.cpp
dom/mathml/nsMathMLElement.h
dom/xbl/XBLChildrenElement.cpp
dom/xbl/XBLChildrenElement.h
--- a/dom/base/nsStyledElement.cpp
+++ b/dom/base/nsStyledElement.cpp
@@ -35,26 +35,41 @@ NS_IMPL_QUERY_INTERFACE_INHERITED(nsStyl
 
 bool
 nsStyledElement::ParseAttribute(int32_t aNamespaceID,
                                 nsIAtom* aAttribute,
                                 const nsAString& aValue,
                                 nsAttrValue& aResult)
 {
   if (aAttribute == nsGkAtoms::style && aNamespaceID == kNameSpaceID_None) {
-    SetMayHaveStyle();
     ParseStyleAttribute(aValue, aResult, false);
     return true;
   }
 
   return nsStyledElementBase::ParseAttribute(aNamespaceID, aAttribute, aValue,
                                              aResult);
 }
 
 nsresult
+nsStyledElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                               const nsAttrValueOrString* aValue, bool aNotify)
+{
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::style) {
+      if (aValue) {
+        SetMayHaveStyle();
+      }
+    }
+  }
+
+  return nsStyledElementBase::BeforeSetAttr(aNamespaceID, aName, aValue,
+                                            aNotify);
+}
+
+nsresult
 nsStyledElement::SetInlineStyleDeclaration(DeclarationBlock* aDeclaration,
                                            const nsAString* aSerialized,
                                            bool aNotify)
 {
   SetMayHaveStyle();
   bool modification = false;
   nsAttrValue oldValue;
   bool oldValueSet = false;
--- a/dom/base/nsStyledElement.h
+++ b/dom/base/nsStyledElement.h
@@ -77,12 +77,16 @@ protected:
    * first put into a document.  Only has an effect if the old value is a
    * string.  If aForceInDataDoc is true, will reparse even if we're in a data
    * document. If aForceIfAlreadyParsed is set, this will always reparse even
    * if the value has already been parsed.
    */
   nsresult ReparseStyleAttribute(bool aForceInDataDoc, bool aForceIfAlreadyParsed);
 
   virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
+
+  virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                 const nsAttrValueOrString* aValue,
+                                 bool aNotify) override;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsStyledElement, NS_STYLED_ELEMENT_IID)
 #endif // __NS_STYLEDELEMENT_H_
--- a/dom/html/HTMLAreaElement.cpp
+++ b/dom/html/HTMLAreaElement.cpp
@@ -141,52 +141,32 @@ HTMLAreaElement::UnbindFromTree(bool aDe
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false, Link::ElementHasHref());
 
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
 nsresult
-HTMLAreaElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                         nsIAtom* aPrefix, const nsAString& aValue,
-                         bool aNotify)
+HTMLAreaElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                              const nsAttrValue* aValue,
+                              const nsAttrValue* aOldValue, 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.
-  if (aName == nsGkAtoms::href && aNameSpaceID == kNameSpaceID_None) {
-    Link::ResetLinkState(!!aNotify, true);
+  if (aNamespaceID == kNameSpaceID_None) {
+    // This must happen after the attribute is set. 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 (aName == nsGkAtoms::href) {
+      Link::ResetLinkState(aNotify, !!aValue);
+    }
   }
 
-  return rv;
-}
-
-nsresult
-HTMLAreaElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                           bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute,
-                                                aNotify);
-
-  // 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);
-  }
-
-  return rv;
+  return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue,
+                                            aOldValue, aNotify);
 }
 
 #define IMPL_URI_PART(_part)                                 \
   NS_IMETHODIMP                                              \
   HTMLAreaElement::Get##_part(nsAString& a##_part)           \
   {                                                          \
     Link::Get##_part(a##_part);                              \
     return NS_OK;                                            \
--- a/dom/html/HTMLAreaElement.h
+++ b/dom/html/HTMLAreaElement.h
@@ -51,26 +51,16 @@ public:
   virtual void GetLinkTarget(nsAString& aTarget) override;
   virtual already_AddRefed<nsIURI> GetHrefURI() const override;
 
   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 Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult,
                          bool aPreallocateChildren) const override;
 
   virtual EventStates IntrinsicState() const override;
 
   // WebIDL
 
@@ -188,15 +178,20 @@ public:
     nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
   }
 
 protected:
   virtual ~HTMLAreaElement();
 
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
+  virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+
   RefPtr<nsDOMTokenList > mRelList;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_HTMLAreaElement_h */
--- a/dom/html/HTMLCanvasElement.cpp
+++ b/dom/html/HTMLCanvasElement.cpp
@@ -456,48 +456,47 @@ HTMLCanvasElement::GetWidthHeight()
   return size;
 }
 
 NS_IMPL_UINT_ATTR_DEFAULT_VALUE(HTMLCanvasElement, Width, width, DEFAULT_CANVAS_WIDTH)
 NS_IMPL_UINT_ATTR_DEFAULT_VALUE(HTMLCanvasElement, Height, height, DEFAULT_CANVAS_HEIGHT)
 NS_IMPL_BOOL_ATTR(HTMLCanvasElement, MozOpaque, moz_opaque)
 
 nsresult
-HTMLCanvasElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify)
+HTMLCanvasElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue, bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue,
-                                              aNotify);
-  if (NS_SUCCEEDED(rv) && mCurrentContext &&
-      aNameSpaceID == kNameSpaceID_None &&
-      (aName == nsGkAtoms::width || aName == nsGkAtoms::height || aName == nsGkAtoms::moz_opaque))
-  {
-    ErrorResult dummy;
-    rv = UpdateContext(nullptr, JS::NullHandleValue, dummy);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
+  AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
 
-  return rv;
+  return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue,
+                                            aOldValue, aNotify);
 }
 
 nsresult
-HTMLCanvasElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                             bool aNotify)
+HTMLCanvasElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aName, aNotify);
-  if (NS_SUCCEEDED(rv) && mCurrentContext &&
-      aNameSpaceID == kNameSpaceID_None &&
-      (aName == nsGkAtoms::width || aName == nsGkAtoms::height || aName == nsGkAtoms::moz_opaque))
-  {
+  AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
+
+  return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName,
+                                                      aValue, aNotify);
+}
+
+void
+HTMLCanvasElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                        bool aNotify)
+{
+  if (mCurrentContext && aNamespaceID == kNameSpaceID_None &&
+      (aName == nsGkAtoms::width || aName == nsGkAtoms::height ||
+       aName == nsGkAtoms::moz_opaque)) {
     ErrorResult dummy;
-    rv = UpdateContext(nullptr, JS::NullHandleValue, dummy);
-    NS_ENSURE_SUCCESS(rv, rv);
+    UpdateContext(nullptr, JS::NullHandleValue, dummy);
   }
-  return rv;
 }
 
 void
 HTMLCanvasElement::HandlePrintCallback(nsPresContext::nsPresContextType aType)
 {
   // Only call the print callback here if 1) we're in a print testing mode or
   // print preview mode, 2) the canvas has a print callback and 3) the callback
   // hasn't already been called. For real printing the callback is handled in
--- a/dom/html/HTMLCanvasElement.h
+++ b/dom/html/HTMLCanvasElement.h
@@ -288,30 +288,16 @@ public:
                        const TimeStamp& aTime);
 
   virtual bool ParseAttribute(int32_t aNamespaceID,
                                 nsIAtom* aAttribute,
                                 const nsAString& aValue,
                                 nsAttrValue& aResult) override;
   nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute, int32_t aModType) const override;
 
-  // SetAttr override.  C++ is stupid, so have to override both
-  // overloaded methods.
-  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* aName,
-                             bool aNotify) override;
-
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const override;
   nsresult CopyInnerTo(mozilla::dom::Element* aDest,
                        bool aPreallocateChildren);
 
   virtual nsresult GetEventTargetParent(
                      mozilla::EventChainPreVisitor& aVisitor) override;
 
@@ -372,16 +358,24 @@ protected:
                          const nsAString& aMimeType,
                          const JS::Value& aEncoderOptions,
                          nsAString& aDataURL);
   nsresult MozGetAsFileImpl(const nsAString& aName,
                             const nsAString& aType,
                             File** aResult);
   void CallPrintCallback();
 
+  virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+  virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify) override;
+
   AsyncCanvasRenderer* GetAsyncCanvasRenderer();
 
   bool mResetLayer;
   RefPtr<HTMLCanvasElement> mOriginalCanvas;
   RefPtr<PrintCallback> mPrintCallback;
   RefPtr<HTMLCanvasPrintState> mPrintState;
   nsTArray<WeakPtr<FrameCaptureListener>> mRequestedFrameListeners;
   RefPtr<RequestedFrameRefreshObserver> mRequestedFrameRefreshObserver;
@@ -405,16 +399,28 @@ public:
 
   void ResetPrintCallback();
 
   HTMLCanvasElement* GetOriginalCanvas();
 
   CanvasContextType GetCurrentContextType() {
     return mCurrentContextType;
   }
+
+private:
+  /**
+   * This function is called by AfterSetAttr and OnAttrSetButNotChanged.
+   * This function will be called by AfterSetAttr whether the attribute is being
+   * set or unset.
+   *
+   * @param aNamespaceID the namespace of the attr being set
+   * @param aName the localname of the attribute being set
+   * @param aNotify Whether we plan to notify document observers.
+   */
+  void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, bool aNotify);
 };
 
 class HTMLCanvasPrintState final : public nsWrapperCache
 {
 public:
   HTMLCanvasPrintState(HTMLCanvasElement* aCanvas,
                        nsICanvasRenderingContextInternal* aContext,
                        nsITimerCallback* aCallback);
--- a/dom/html/HTMLContentElement.cpp
+++ b/dom/html/HTMLContentElement.cpp
@@ -205,85 +205,73 @@ IsValidContentSelectors(nsCSSSelector* a
 
     currentSelector = currentSelector->mNext;
   }
 
   return true;
 }
 
 nsresult
-HTMLContentElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                            nsIAtom* aPrefix, const nsAString& aValue,
-                            bool aNotify)
+HTMLContentElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                 const nsAttrValue* aValue,
+                                 const nsAttrValue* aOldValue, bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                              aValue, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::select) {
+    if (aValue) {
+      // Select attribute was updated, the insertion point may match different
+      // elements.
+      nsIDocument* doc = OwnerDoc();
+      nsCSSParser parser(doc->CSSLoader());
 
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::select) {
-    // Select attribute was updated, the insertion point may match different
-    // elements.
-    nsIDocument* doc = OwnerDoc();
-    nsCSSParser parser(doc->CSSLoader());
+      mValidSelector = true;
+      mSelectorList = nullptr;
 
-    mValidSelector = true;
-    mSelectorList = nullptr;
+      nsAutoString valueStr;
+      aValue->ToString(valueStr);
+      nsresult rv = parser.ParseSelectorString(valueStr,
+                                               doc->GetDocumentURI(),
+                                               // Bug 11240
+                                               0, // XXX get the line number!
+                                               getter_Transfers(mSelectorList));
 
-    nsresult rv = parser.ParseSelectorString(aValue,
-                                             doc->GetDocumentURI(),
-                                             // Bug 11240
-                                             0, // XXX get the line number!
-                                             getter_Transfers(mSelectorList));
+      // We don't want to return an exception if parsing failed because
+      // the spec does not define it as an exception case.
+      if (NS_SUCCEEDED(rv)) {
+        // Ensure that all the selectors are valid
+        nsCSSSelectorList* selectors = mSelectorList;
+        while (selectors) {
+          if (!IsValidContentSelectors(selectors->mSelectors)) {
+            // If we have an invalid selector, we can not match anything.
+            mValidSelector = false;
+            mSelectorList = nullptr;
+            break;
+          }
+          selectors = selectors->mNext;
+        }
+      }
 
-    // We don't want to return an exception if parsing failed because
-    // the spec does not define it as an exception case.
-    if (NS_SUCCEEDED(rv)) {
-      // Ensure that all the selectors are valid
-      nsCSSSelectorList* selectors = mSelectorList;
-      while (selectors) {
-        if (!IsValidContentSelectors(selectors->mSelectors)) {
-          // If we have an invalid selector, we can not match anything.
-          mValidSelector = false;
-          mSelectorList = nullptr;
-          break;
-        }
-        selectors = selectors->mNext;
+      ShadowRoot* containingShadow = GetContainingShadow();
+      if (containingShadow) {
+        containingShadow->DistributeAllNodes();
       }
-    }
+    } else {
+      // The select attribute was removed. This insertion point becomes
+      // a universal selector.
+      mValidSelector = true;
+      mSelectorList = nullptr;
 
-    ShadowRoot* containingShadow = GetContainingShadow();
-    if (containingShadow) {
-      containingShadow->DistributeAllNodes();
+      ShadowRoot* containingShadow = GetContainingShadow();
+      if (containingShadow) {
+        containingShadow->DistributeAllNodes();
+      }
     }
   }
 
-  return NS_OK;
-}
-
-nsresult
-HTMLContentElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                              bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID,
-                                                aAttribute, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::select) {
-    // The select attribute was removed. This insertion point becomes
-    // a universal selector.
-    mValidSelector = true;
-    mSelectorList = nullptr;
-
-    ShadowRoot* containingShadow = GetContainingShadow();
-    if (containingShadow) {
-      containingShadow->DistributeAllNodes();
-    }
-  }
-
-  return NS_OK;
+  return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue,
+                                            aOldValue, aNotify);
 }
 
 bool
 HTMLContentElement::Match(nsIContent* aContent)
 {
   if (!mValidSelector) {
     return false;
   }
--- a/dom/html/HTMLContentElement.h
+++ b/dom/html/HTMLContentElement.h
@@ -59,23 +59,16 @@ public:
   bool Match(nsIContent* aContent);
   bool IsInsertionPoint() const { return mIsInsertionPoint; }
   nsCOMArray<nsIContent>& MatchedNodes() { return mMatchedNodes; }
   void AppendMatchedNode(nsIContent* aContent);
   void RemoveMatchedNode(nsIContent* aContent);
   void InsertMatchedNode(uint32_t aIndex, nsIContent* aContent);
   void ClearMatchedNodes();
 
-  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;
-
   // WebIDL methods.
   already_AddRefed<DistributedContentList> GetDistributedNodes();
   void GetSelect(nsAString& aSelect)
   {
     Element::GetAttr(kNameSpaceID_None, nsGkAtoms::select, aSelect);
   }
   void SetSelect(const nsAString& aSelect)
   {
@@ -91,16 +84,21 @@ protected:
    * Updates the destination insertion points of the fallback
    * content of this insertion point. If there are nodes matched
    * to this insertion point, then destination insertion points
    * of fallback are cleared, otherwise, this insertion point
    * is a destination insertion point.
    */
   void UpdateFallbackDistribution();
 
+  virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+
   /**
    * An array of nodes from the ShadowRoot host that match the
    * content insertion selector.
    */
   nsCOMArray<nsIContent> mMatchedNodes;
 
   nsAutoPtr<nsCSSSelectorList> mSelectorList;
   bool mValidSelector;
--- a/dom/html/HTMLFormElement.cpp
+++ b/dom/html/HTMLFormElement.cpp
@@ -185,37 +185,41 @@ NS_IMETHODIMP
 HTMLFormElement::GetElements(nsIDOMHTMLCollection** aElements)
 {
   *aElements = Elements();
   NS_ADDREF(*aElements);
   return NS_OK;
 }
 
 nsresult
-HTMLFormElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                         nsIAtom* aPrefix, const nsAString& aValue,
-                         bool aNotify)
+HTMLFormElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                               const nsAttrValueOrString* aValue, bool aNotify)
 {
-  if ((aName == nsGkAtoms::action || aName == nsGkAtoms::target) &&
-      aNameSpaceID == kNameSpaceID_None) {
-    if (mPendingSubmission) {
-      // aha, there is a pending submission that means we're in
-      // the script and we need to flush it. let's tell it
-      // that the event was ignored to force the flush.
-      // the second argument is not playing a role at all.
-      FlushPendingSubmission();
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::action || aName == nsGkAtoms::target) {
+      // This check is mostly to preserve previous behavior.
+      if (aValue) {
+        if (mPendingSubmission) {
+          // aha, there is a pending submission that means we're in
+          // the script and we need to flush it. let's tell it
+          // that the event was ignored to force the flush.
+          // the second argument is not playing a role at all.
+          FlushPendingSubmission();
+        }
+        // Don't forget we've notified the password manager already if the
+        // page sets the action/target in the during submit. (bug 343182)
+        bool notifiedObservers = mNotifiedObservers;
+        ForgetCurrentSubmission();
+        mNotifiedObservers = notifiedObservers;
+      }
     }
-    // Don't forget we've notified the password manager already if the
-    // page sets the action/target in the during submit. (bug 343182)
-    bool notifiedObservers = mNotifiedObservers;
-    ForgetCurrentSubmission();
-    mNotifiedObservers = notifiedObservers;
   }
-  return nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue,
-                                       aNotify);
+
+  return nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, aValue,
+                                             aNotify);
 }
 
 nsresult
 HTMLFormElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                               const nsAttrValue* aValue,
                               const nsAttrValue* aOldValue, bool aNotify)
 {
   if (aName == nsGkAtoms::novalidate && aNameSpaceID == kNameSpaceID_None) {
--- a/dom/html/HTMLFormElement.h
+++ b/dom/html/HTMLFormElement.h
@@ -100,24 +100,19 @@ public:
   virtual nsresult PostHandleEvent(
                      EventChainPostVisitor& aVisitor) override;
 
   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 BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                 const nsAttrValueOrString* aValue,
+                                 bool aNotify) override;
   virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                 const nsAttrValue* aValue,
                                 const nsAttrValue* aOldValue,
                                 bool aNotify) override;
 
   /**
    * Forget all information about the current submission (and the fact that we
    * are currently submitting at all).
--- a/dom/html/HTMLFrameSetElement.cpp
+++ b/dom/html/HTMLFrameSetElement.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "HTMLFrameSetElement.h"
 #include "mozilla/dom/HTMLFrameSetElementBinding.h"
 #include "mozilla/dom/EventHandlerBinding.h"
 #include "nsGlobalWindow.h"
 #include "mozilla/UniquePtrExtensions.h"
+#include "nsAttrValueOrString.h"
 
 NS_IMPL_NS_NEW_HTML_ELEMENT(FrameSet)
 
 namespace mozilla {
 namespace dom {
 
 HTMLFrameSetElement::~HTMLFrameSetElement()
 {
@@ -60,53 +61,55 @@ HTMLFrameSetElement::GetRows(nsAString& 
 {
   DOMString rows;
   GetRows(rows);
   rows.ToString(aRows);
   return NS_OK;
 }
 
 nsresult
-HTMLFrameSetElement::SetAttr(int32_t aNameSpaceID,
-                             nsIAtom* aAttribute,
-                             nsIAtom* aPrefix,
-                             const nsAString& aValue,
-                             bool aNotify)
+HTMLFrameSetElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                   const nsAttrValueOrString* aValue,
+                                   bool aNotify)
 {
-  nsresult rv;
   /* The main goal here is to see whether the _number_ of rows or
-   *  columns has changed.  If it has, we need to reframe; otherwise
-   *  we want to reflow.  So we set mCurrentRowColHint here, then call
-   *  nsGenericHTMLElement::SetAttr, which will end up calling
-   *  GetAttributeChangeHint and notifying layout with that hint.
-   *  Once nsGenericHTMLElement::SetAttr returns, we want to go back to our
-   *  normal hint, which is NS_STYLE_HINT_REFLOW.
+   * columns has changed. If it has, we need to reframe; otherwise
+   * we want to reflow.
+   * Ideally, the style hint would be changed back to reflow after the reframe
+   * has been performed. Unfortunately, however, the reframe will be performed
+   * by the call to nsNodeUtils::AttributeChanged, which occurs *after*
+   * AfterSetAttr is called, leaving us with no convenient way of changing the
+   * value back to reflow afterwards. However, nsNodeUtils::AttributeChanged is
+   * effectively the only consumer of this value, so as long as we always set
+   * the value correctly here, we should be fine.
    */
-  if (aAttribute == nsGkAtoms::rows && aNameSpaceID == kNameSpaceID_None) {
-    int32_t oldRows = mNumRows;
-    ParseRowCol(aValue, mNumRows, &mRowSpecs);
+  mCurrentRowColHint = NS_STYLE_HINT_REFLOW;
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::rows) {
+      if (aValue) {
+        int32_t oldRows = mNumRows;
+        ParseRowCol(aValue->String(), mNumRows, &mRowSpecs);
 
-    if (mNumRows != oldRows) {
-      mCurrentRowColHint = nsChangeHint_ReconstructFrame;
-    }
-  } else if (aAttribute == nsGkAtoms::cols &&
-             aNameSpaceID == kNameSpaceID_None) {
-    int32_t oldCols = mNumCols;
-    ParseRowCol(aValue, mNumCols, &mColSpecs);
+        if (mNumRows != oldRows) {
+          mCurrentRowColHint = nsChangeHint_ReconstructFrame;
+        }
+      }
+    } else if (aName == nsGkAtoms::cols) {
+      if (aValue) {
+        int32_t oldCols = mNumCols;
+        ParseRowCol(aValue->String(), mNumCols, &mColSpecs);
 
-    if (mNumCols != oldCols) {
-      mCurrentRowColHint = nsChangeHint_ReconstructFrame;
+        if (mNumCols != oldCols) {
+          mCurrentRowColHint = nsChangeHint_ReconstructFrame;
+        }
+      }
     }
   }
 
-  rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aAttribute, aPrefix,
-                                     aValue, aNotify);
-  mCurrentRowColHint = NS_STYLE_HINT_REFLOW;
-
-  return rv;
+  return nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, aValue, aNotify);
 }
 
 nsresult
 HTMLFrameSetElement::GetRowSpec(int32_t *aNumValues,
                                 const nsFramesetSpec** aSpecs)
 {
   NS_PRECONDITION(aNumValues, "Must have a pointer to an integer here!");
   NS_PRECONDITION(aSpecs, "Must have a pointer to an array of nsFramesetSpecs");
--- a/dom/html/HTMLFrameSetElement.h
+++ b/dom/html/HTMLFrameSetElement.h
@@ -95,27 +95,16 @@ public:
 #define BEFOREUNLOAD_EVENT(name_, id_, type_, struct_)                  \
   WINDOW_EVENT_HELPER(name_, OnBeforeUnloadEventHandlerNonNull)
 #include "mozilla/EventNameList.h" // IWYU pragma: keep
 #undef BEFOREUNLOAD_EVENT
 #undef WINDOW_EVENT
 #undef WINDOW_EVENT_HELPER
 #undef EVENT
 
-  // These override the SetAttr methods in nsGenericHTMLElement (need
-  // both here to silence compiler warnings).
-  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;
-
    /**
     * GetRowSpec is used to get the "rows" spec.
     * @param out int32_t aNumValues The number of row sizes specified.
     * @param out nsFramesetSpec* aSpecs The array of size specifications.
              This is _not_ owned by the caller, but by the nsFrameSetElement
              implementation.  DO NOT DELETE IT.
     */
   nsresult GetRowSpec(int32_t *aNumValues, const nsFramesetSpec** aSpecs);
@@ -139,16 +128,20 @@ public:
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const override;
 
 protected:
   virtual ~HTMLFrameSetElement();
 
   virtual JSObject* WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto) override;
 
+  virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                 const nsAttrValueOrString* aValue,
+                                 bool aNotify) override;
+
 private:
   nsresult ParseRowCol(const nsAString& aValue,
                        int32_t& aNumSpecs,
                        UniquePtr<nsFramesetSpec[]>* aSpecs);
 
   /**
    * The number of size specs in our "rows" attr
    */
--- a/dom/html/HTMLIFrameElement.cpp
+++ b/dom/html/HTMLIFrameElement.cpp
@@ -149,64 +149,59 @@ HTMLIFrameElement::IsAttributeMapped(con
 
 nsMapRuleToAttributesFunc
 HTMLIFrameElement::GetAttributeMappingFunction() const
 {
   return &MapAttributesIntoRule;
 }
 
 nsresult
-HTMLIFrameElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify)
-{
-  nsresult rv = nsGenericHTMLFrameElement::SetAttr(aNameSpaceID, aName,
-                                                   aPrefix, aValue, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::srcdoc) {
-    // Don't propagate error here. The attribute was successfully set, that's
-    // what we should reflect.
-    LoadSrc();
-  }
-
-  return NS_OK;
-}
-
-nsresult
 HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                 const nsAttrValue* aValue,
                                 const nsAttrValue* aOldValue, bool aNotify)
 {
-  if (aName == nsGkAtoms::sandbox &&
-      aNameSpaceID == kNameSpaceID_None && mFrameLoader) {
-    // If we have an nsFrameLoader, apply the new sandbox flags.
-    // Since this is called after the setter, the sandbox flags have
-    // alreay been updated.
-    mFrameLoader->ApplySandboxFlags(GetSandboxFlags());
+  AfterMaybeChangeAttr(aNameSpaceID, aName, aNotify);
+
+  if (aNameSpaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::sandbox) {
+      if (mFrameLoader) {
+        // If we have an nsFrameLoader, apply the new sandbox flags.
+        // Since this is called after the setter, the sandbox flags have
+        // alreay been updated.
+        mFrameLoader->ApplySandboxFlags(GetSandboxFlags());
+      }
+    }
   }
   return nsGenericHTMLFrameElement::AfterSetAttr(aNameSpaceID, aName, aValue,
                                                  aOldValue, aNotify);
 }
 
 nsresult
-HTMLIFrameElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify)
+HTMLIFrameElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify)
 {
-  // Invoke on the superclass.
-  nsresult rv = nsGenericHTMLFrameElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
+  AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
+
+  return nsGenericHTMLFrameElement::OnAttrSetButNotChanged(aNamespaceID, aName,
+                                                           aValue, aNotify);
+}
 
-  if (aNameSpaceID == kNameSpaceID_None &&
-      aAttribute == nsGkAtoms::srcdoc) {
-    // Fall back to the src attribute, if any
-    LoadSrc();
+void
+HTMLIFrameElement::AfterMaybeChangeAttr(int32_t aNamespaceID,
+                                        nsIAtom* aName,
+                                        bool aNotify)
+{
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::srcdoc) {
+      // Don't propagate errors from LoadSrc. The attribute was successfully
+      // set/unset, that's what we should reflect.
+      LoadSrc();
+    }
   }
-
-  return NS_OK;
 }
 
 uint32_t
 HTMLIFrameElement::GetSandboxFlags()
 {
   const nsAttrValue* sandboxAttr = GetParsedAttr(nsGkAtoms::sandbox);
   // No sandbox attribute, no sandbox flags.
   if (!sandboxAttr) {
--- a/dom/html/HTMLIFrameElement.h
+++ b/dom/html/HTMLIFrameElement.h
@@ -42,31 +42,16 @@ public:
                                 const nsAString& aValue,
                                 nsAttrValue& aResult) override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsIAtom* aAttribute) const override;
   virtual nsMapRuleToAttributesFunc GetAttributeMappingFunction() const override;
 
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const 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 AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                                const nsAttrValue* aValue,
-                                const nsAttrValue* aOldValue,
-                                bool aNotify) override;
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
-
   uint32_t GetSandboxFlags();
 
   // Web IDL binding methods
   // The XPCOM GetSrc is fine for our purposes
   void SetSrc(const nsAString& aSrc, ErrorResult& aError)
   {
     SetHTMLAttr(nsGkAtoms::src, aSrc, aError);
   }
@@ -193,19 +178,38 @@ public:
   bool FullscreenFlag() const { return mFullscreenFlag; }
   void SetFullscreenFlag(bool aValue) { mFullscreenFlag = aValue; }
 
 protected:
   virtual ~HTMLIFrameElement();
 
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+  virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify) override;
+
 private:
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     GenericSpecifiedValues* aGenericData);
 
   static const DOMTokenListSupportedToken sSupportedSandboxTokens[];
+
+  /**
+   * This function is called by AfterSetAttr and OnAttrSetButNotChanged.
+   * This function will be called by AfterSetAttr whether the attribute is being
+   * set or unset.
+   *
+   * @param aNamespaceID the namespace of the attr being set
+   * @param aName the localname of the attribute being set
+   * @param aNotify Whether we plan to notify document observers.
+   */
+  void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, bool aNotify);
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif
--- a/dom/html/HTMLLegendElement.cpp
+++ b/dom/html/HTMLLegendElement.cpp
@@ -66,31 +66,16 @@ HTMLLegendElement::GetAttributeChangeHin
       nsGenericHTMLElement::GetAttributeChangeHint(aAttribute, aModType);
   if (aAttribute == nsGkAtoms::align) {
     retval |= NS_STYLE_HINT_REFLOW;
   }
   return retval;
 }
 
 nsresult
-HTMLLegendElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify)
-{
-  return nsGenericHTMLElement::SetAttr(aNameSpaceID, aAttribute,
-                                       aPrefix, aValue, aNotify);
-}
-nsresult
-HTMLLegendElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify)
-{
-  return nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify);
-}
-
-nsresult
 HTMLLegendElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers)
 {
   return nsGenericHTMLElement::BindToTree(aDocument, aParent,
                                           aBindingParent,
                                           aCompileEventHandlers);
 }
--- a/dom/html/HTMLLegendElement.h
+++ b/dom/html/HTMLLegendElement.h
@@ -37,26 +37,16 @@ public:
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
   virtual bool ParseAttribute(int32_t aNamespaceID,
                                 nsIAtom* aAttribute,
                                 const nsAString& aValue,
                                 nsAttrValue& aResult) override;
   virtual nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute,
                                               int32_t aModType) const 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 Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult,
                          bool aPreallocateChildren) const override;
 
   Element* GetFormElement() const
   {
     nsCOMPtr<nsIFormControl> fieldsetControl = do_QueryInterface(GetFieldSet());
 
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -4185,90 +4185,86 @@ bool HTMLMediaElement::IsHTMLFocusable(b
   return false;
 }
 
 int32_t HTMLMediaElement::TabIndexDefault()
 {
   return 0;
 }
 
-nsresult HTMLMediaElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                                   nsIAtom* aPrefix, const nsAString& aValue,
-                                   bool aNotify)
-{
-  nsresult rv =
-    nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue,
-                                  aNotify);
-  if (NS_FAILED(rv))
-    return rv;
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src) {
-    DoLoad();
-  }
-  if (aNotify && aNameSpaceID == kNameSpaceID_None) {
-    if (aName == nsGkAtoms::autoplay) {
-      StopSuspendingAfterFirstFrame();
-      CheckAutoplayDataReady();
-      // This attribute can affect AddRemoveSelfReference
-      AddRemoveSelfReference();
-      UpdatePreloadAction();
-    } else if (aName == nsGkAtoms::preload) {
-      UpdatePreloadAction();
-    }
-  }
-
-  return rv;
-}
-
-nsresult HTMLMediaElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttr,
-                                     bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttr, aNotify);
-  if (NS_FAILED(rv))
-    return rv;
-  if (aNotify && aNameSpaceID == kNameSpaceID_None) {
-    if (aAttr == nsGkAtoms::autoplay) {
-      // This attribute can affect AddRemoveSelfReference
-      AddRemoveSelfReference();
-      UpdatePreloadAction();
-    } else if (aAttr == nsGkAtoms::preload) {
-      UpdatePreloadAction();
-    }
-  }
-
-  return rv;
-}
-
 nsresult
 HTMLMediaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                 const nsAttrValue* aValue,
                                 const nsAttrValue* aOldValue, bool aNotify)
 {
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src) {
-    mSrcMediaSource = nullptr;
-    if (aValue) {
-      nsString srcStr = aValue->GetStringValue();
-      nsCOMPtr<nsIURI> uri;
-      NewURIFromString(srcStr, getter_AddRefs(uri));
-      if (uri && IsMediaSourceURI(uri)) {
-        nsresult rv =
-          NS_GetSourceForMediaSourceURI(uri, getter_AddRefs(mSrcMediaSource));
-        if (NS_FAILED(rv)) {
-          nsAutoString spec;
-          GetCurrentSrc(spec);
-          const char16_t* params[] = { spec.get() };
-          ReportLoadError("MediaLoadInvalidURI", params, ArrayLength(params));
+  if (aNameSpaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::src) {
+      mSrcMediaSource = nullptr;
+      if (aValue) {
+        nsString srcStr = aValue->GetStringValue();
+        nsCOMPtr<nsIURI> uri;
+        NewURIFromString(srcStr, getter_AddRefs(uri));
+        if (uri && IsMediaSourceURI(uri)) {
+          nsresult rv =
+            NS_GetSourceForMediaSourceURI(uri, getter_AddRefs(mSrcMediaSource));
+          if (NS_FAILED(rv)) {
+            nsAutoString spec;
+            GetCurrentSrc(spec);
+            const char16_t* params[] = { spec.get() };
+            ReportLoadError("MediaLoadInvalidURI", params, ArrayLength(params));
+          }
         }
       }
-    }
+    } else if (aName == nsGkAtoms::autoplay) {
+      if (aNotify) {
+        if (aValue) {
+          StopSuspendingAfterFirstFrame();
+          CheckAutoplayDataReady();
+        }
+        // This attribute can affect AddRemoveSelfReference
+        AddRemoveSelfReference();
+        UpdatePreloadAction();
+      }
+    } else if (aName == nsGkAtoms::preload) {
+      UpdatePreloadAction();
+    }
+  }
+
+  // Since AfterMaybeChangeAttr may call DoLoad, make sure that it is called
+  // *after* any possible changes to mSrcMediaSource.
+  if (aValue) {
+    AfterMaybeChangeAttr(aNameSpaceID, aName, aNotify);
   }
 
   return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName,
                                             aValue, aOldValue, aNotify);
 }
 
+nsresult
+HTMLMediaElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                         const nsAttrValueOrString& aValue,
+                                         bool aNotify)
+{
+  AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
+
+  return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName,
+                                                      aValue, aNotify);
+}
+
+void
+HTMLMediaElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                       bool aNotify)
+{
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::src) {
+      DoLoad();
+    }
+  }
+}
+
 nsresult HTMLMediaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                                       nsIContent* aBindingParent,
                                       bool aCompileEventHandlers)
 {
   nsresult rv = nsGenericHTMLElement::BindToTree(aDocument,
                                                  aParent,
                                                  aBindingParent,
                                                  aCompileEventHandlers);
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -121,32 +121,16 @@ public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(HTMLMediaElement,
                                            nsGenericHTMLElement)
 
   virtual bool ParseAttribute(int32_t aNamespaceID,
                               nsIAtom* aAttribute,
                               const nsAString& aValue,
                               nsAttrValue& aResult) override;
-  // SetAttr override.  C++ is stupid, so have to override both
-  // overloaded methods.
-  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* aAttr,
-                             bool aNotify) override;
-  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                                const nsAttrValue* aValue,
-                                const nsAttrValue* aOldValue,
-                                bool aNotify) override;
 
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
   virtual void DoneCreatingElement() override;
 
@@ -1310,16 +1294,24 @@ protected:
 
   // Pass information for deciding the video decode mode to decoder.
   void NotifyDecoderActivityChanges() const;
 
   // Mark the decoder owned by the element as tainted so that the
   // suspend-video-decoder is disabled.
   void MarkAsTainted();
 
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+  virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify) override;
+
   // The current decoder. Load() has been called on this decoder.
   // At most one of mDecoder and mSrcStream can be non-null.
   RefPtr<MediaDecoder> mDecoder;
 
   // The DocGroup-specific AbstractThread::MainThread() of this HTML element.
   RefPtr<AbstractThread> mAbstractMainThread;
 
   // Observers listening to changes to the mDecoder principal.
@@ -1717,16 +1709,26 @@ public:
       return mCount + 1;
     }
   private:
     TimeStamp mStartTime;
     TimeDuration mSum;
     uint32_t mCount;
   };
 private:
+  /**
+   * This function is called by AfterSetAttr and OnAttrSetButNotChanged.
+   * It will not be called if the value is being unset.
+   *
+   * @param aNamespaceID the namespace of the attr being set
+   * @param aName the localname of the attribute being set
+   * @param aNotify Whether we plan to notify document observers.
+   */
+  void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, bool aNotify);
+
   // Total time a video has spent playing.
   TimeDurationAccumulator mPlayTime;
 
   // Total time a video has spent playing while hidden.
   TimeDurationAccumulator mHiddenPlayTime;
 
   // Total time a video has (or would have) spent in video-decode-suspend mode.
   TimeDurationAccumulator mVideoDecodeSuspendTime;
--- a/dom/html/HTMLObjectElement.cpp
+++ b/dom/html/HTMLObjectElement.cpp
@@ -286,56 +286,56 @@ HTMLObjectElement::UnbindFromTree(bool a
   // still contains a focused plugin when it doesn't -- which in turn can
   // disable text input in the browser window. See bug 1137229.
   OnFocusBlurPlugin(this, false);
 #endif
   nsObjectLoadingContent::UnbindFromTree(aDeep, aNullParent);
   nsGenericHTMLFormElement::UnbindFromTree(aDeep, aNullParent);
 }
 
-
-
 nsresult
-HTMLObjectElement::SetAttr(int32_t aNameSpaceID, nsIAtom *aName,
-                           nsIAtom *aPrefix, const nsAString &aValue,
-                           bool aNotify)
+HTMLObjectElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue, bool aNotify)
 {
-  nsresult rv = nsGenericHTMLFormElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                                  aValue, aNotify);
+  nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // if aNotify is false, we are coming from the parser or some such place;
-  // we'll get bound after all the attributes have been set, so we'll do the
-  // object load from BindToTree/DoneAddingChildren.
-  // Skip the LoadObject call in that case.
-  // We also don't want to start loading the object when we're not yet in
-  // a document, just in case that the caller wants to set additional
-  // attributes before inserting the node into the document.
-  if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
-      aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::data &&
-      !BlockEmbedOrObjectContentLoading()) {
-    return LoadObject(aNotify, true);
-  }
-
-  return NS_OK;
+  return nsGenericHTMLFormElement::AfterSetAttr(aNamespaceID, aName, aValue,
+                                                aOldValue, aNotify);
 }
 
 nsresult
-HTMLObjectElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify)
+HTMLObjectElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify)
 {
-  nsresult rv = nsGenericHTMLFormElement::UnsetAttr(aNameSpaceID,
-                                                    aAttribute, aNotify);
+  nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // See comment in SetAttr
-  if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
-      aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::data &&
-      !BlockEmbedOrObjectContentLoading()) {
-    return LoadObject(aNotify, true);
+  return nsGenericHTMLFormElement::OnAttrSetButNotChanged(aNamespaceID, aName,
+                                                          aValue, aNotify);
+}
+
+nsresult
+HTMLObjectElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                        bool aNotify)
+{
+  if (aNamespaceID == kNameSpaceID_None) {
+    // if aNotify is false, we are coming from the parser or some such place;
+    // we'll get bound after all the attributes have been set, so we'll do the
+    // object load from BindToTree/DoneAddingChildren.
+    // Skip the LoadObject call in that case.
+    // We also don't want to start loading the object when we're not yet in
+    // a document, just in case that the caller wants to set additional
+    // attributes before inserting the node into the document.
+    if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
+        aName == nsGkAtoms::data && !BlockEmbedOrObjectContentLoading()) {
+      return LoadObject(aNotify, true);
+    }
   }
 
   return NS_OK;
 }
 
 bool
 HTMLObjectElement::IsFocusableForTabIndex()
 {
--- a/dom/html/HTMLObjectElement.h
+++ b/dom/html/HTMLObjectElement.h
@@ -54,21 +54,16 @@ public:
   // nsIDOMHTMLObjectElement
   NS_DECL_NSIDOMHTMLOBJECTELEMENT
 
   virtual nsresult BindToTree(nsIDocument *aDocument, nsIContent *aParent,
                               nsIContent *aBindingParent,
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
-  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 bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override;
   virtual IMEState GetDesiredIMEState() override;
 
   // Overriden nsIFormControl methods
   NS_IMETHOD Reset() override;
   NS_IMETHOD SubmitNamesValues(HTMLFormSubmission *aFormSubmission) override;
 
@@ -247,16 +242,24 @@ public:
    * Calls LoadObject with the correct arguments to start the plugin load.
    */
   void StartObjectLoad(bool aNotify, bool aForceLoad);
 
 protected:
   // Override for nsImageLoadingContent.
   nsIContent* AsContent() override { return this; }
 
+  virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+  virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify) override;
+
 private:
   /**
    * Returns if the element is currently focusable regardless of it's tabindex
    * value. This is used to know the default tabindex value.
    */
   bool IsFocusableForTabIndex();
 
   nsContentPolicyType GetContentPolicyType() const override
@@ -266,15 +269,27 @@ private:
 
   virtual ~HTMLObjectElement();
 
   virtual JSObject* WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     GenericSpecifiedValues* aGenericData);
 
+  /**
+   * This function is called by AfterSetAttr and OnAttrSetButNotChanged.
+   * This function will be called by AfterSetAttr whether the attribute is being
+   * set or unset.
+   *
+   * @param aNamespaceID the namespace of the attr being set
+   * @param aName the localname of the attribute being set
+   * @param aNotify Whether we plan to notify document observers.
+   */
+  nsresult AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                bool aNotify);
+
   bool mIsDoneAddingChildren;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_HTMLObjectElement_h
--- a/dom/html/HTMLSelectElement.cpp
+++ b/dom/html/HTMLSelectElement.cpp
@@ -1296,19 +1296,32 @@ HTMLSelectElement::UnbindFromTree(bool a
   UpdateState(false);
 }
 
 nsresult
 HTMLSelectElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                  const nsAttrValueOrString* aValue,
                                  bool aNotify)
 {
-  if (aNotify && aName == nsGkAtoms::disabled &&
-      aNameSpaceID == kNameSpaceID_None) {
-    mDisabledChanged = true;
+  if (aNameSpaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::disabled) {
+      if (aNotify) {
+        mDisabledChanged = true;
+      }
+    } else if (aName == nsGkAtoms::multiple) {
+      if (!aValue && aNotify && mSelectedIndex >= 0) {
+        // We're changing from being a multi-select to a single-select.
+        // Make sure we only have one option selected before we do that.
+        // Note that this needs to come before we really unset the attr,
+        // since SetOptionsSelectedByIndex does some bail-out type
+        // optimization for cases when the select is not multiple that
+        // would lead to only a single option getting deselected.
+        SetSelectedIndexInternal(mSelectedIndex, aNotify);
+      }
+    }
   }
 
   return nsGenericHTMLFormElementWithState::BeforeSetAttr(aNameSpaceID, aName,
                                                           aValue, aNotify);
 }
 
 nsresult
 HTMLSelectElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
@@ -1319,55 +1332,30 @@ HTMLSelectElement::AfterSetAttr(int32_t 
     if (aName == nsGkAtoms::disabled) {
       UpdateBarredFromConstraintValidation();
     } else if (aName == nsGkAtoms::required) {
       UpdateValueMissingValidityState();
     } else if (aName == nsGkAtoms::autocomplete) {
       // Clear the cached @autocomplete attribute and autocompleteInfo state.
       mAutocompleteAttrState = nsContentUtils::eAutocompleteAttrState_Unknown;
       mAutocompleteInfoState = nsContentUtils::eAutocompleteAttrState_Unknown;
+    } else if (aName == nsGkAtoms::multiple) {
+      if (!aValue && aNotify) {
+        // We might have become a combobox; make sure _something_ gets
+        // selected in that case
+        CheckSelectSomething(aNotify);
+      }
     }
   }
 
   return nsGenericHTMLFormElementWithState::AfterSetAttr(aNameSpaceID, aName,
                                                          aValue, aOldValue,
                                                          aNotify);
 }
 
-nsresult
-HTMLSelectElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify)
-{
-  if (aNotify && aNameSpaceID == kNameSpaceID_None &&
-      aAttribute == nsGkAtoms::multiple) {
-    // We're changing from being a multi-select to a single-select.
-    // Make sure we only have one option selected before we do that.
-    // Note that this needs to come before we really unset the attr,
-    // since SetOptionsSelectedByIndex does some bail-out type
-    // optimization for cases when the select is not multiple that
-    // would lead to only a single option getting deselected.
-    if (mSelectedIndex >= 0) {
-      SetSelectedIndexInternal(mSelectedIndex, aNotify);
-    }
-  }
-
-  nsresult rv = nsGenericHTMLFormElementWithState::UnsetAttr(aNameSpaceID, aAttribute,
-                                                             aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (aNotify && aNameSpaceID == kNameSpaceID_None &&
-      aAttribute == nsGkAtoms::multiple) {
-    // We might have become a combobox; make sure _something_ gets
-    // selected in that case
-    CheckSelectSomething(aNotify);
-  }
-
-  return rv;
-}
-
 void
 HTMLSelectElement::DoneAddingChildren(bool aHaveNotified)
 {
   mIsDoneAddingChildren = true;
 
   nsISelectControlFrame* selectFrame = GetSelectFrame();
 
   // If we foolishly tried to restore before we were done adding
--- a/dom/html/HTMLSelectElement.h
+++ b/dom/html/HTMLSelectElement.h
@@ -386,18 +386,16 @@ public:
   virtual void UnbindFromTree(bool aDeep, bool aNullParent) override;
   virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                  const nsAttrValueOrString* aValue,
                                  bool aNotify) override;
   virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                 const nsAttrValue* aValue,
                                 const nsAttrValue* aOldValue,
                                 bool aNotify) override;
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
 
   virtual void DoneAddingChildren(bool aHaveNotified) override;
   virtual bool IsDoneAddingChildren() override {
     return mIsDoneAddingChildren;
   }
 
   virtual bool ParseAttribute(int32_t aNamespaceID,
                                 nsIAtom* aAttribute,
--- a/dom/html/HTMLSharedElement.cpp
+++ b/dom/html/HTMLSharedElement.cpp
@@ -225,62 +225,43 @@ SetBaseTargetUsingFirstBaseWithTarget(ns
       return;
     }
   }
 
   aDocument->SetBaseTarget(EmptyString());
 }
 
 nsresult
-HTMLSharedElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify)
+HTMLSharedElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue, bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                              aValue, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // If the href attribute of a <base> tag is changing, we may need to update
-  // the document's base URI, which will cause all the links on the page to be
-  // re-resolved given the new base.  If the target attribute is changing, we
-  // similarly need to change the base target.
-  if (mNodeInfo->Equals(nsGkAtoms::base) &&
-      aNameSpaceID == kNameSpaceID_None &&
-      IsInUncomposedDoc()) {
+  if (aNamespaceID == kNameSpaceID_None) {
     if (aName == nsGkAtoms::href) {
-      SetBaseURIUsingFirstBaseWithHref(GetUncomposedDoc(), this);
+      // If the href attribute of a <base> tag is changing, we may need to
+      // update the document's base URI, which will cause all the links on the
+      // page to be re-resolved given the new base.
+      // If the href is being unset (aValue is null), we will need to find a new
+      // <base>.
+      if (mNodeInfo->Equals(nsGkAtoms::base) && IsInUncomposedDoc()) {
+        SetBaseURIUsingFirstBaseWithHref(GetUncomposedDoc(),
+                                         aValue ? this : nullptr);
+      }
     } else if (aName == nsGkAtoms::target) {
-      SetBaseTargetUsingFirstBaseWithTarget(GetUncomposedDoc(), this);
+      // The target attribute is in pretty much the same situation as the href
+      // attribute, above.
+      if (mNodeInfo->Equals(nsGkAtoms::base) && IsInUncomposedDoc()) {
+        SetBaseTargetUsingFirstBaseWithTarget(GetUncomposedDoc(),
+                                              aValue ? this : nullptr);
+      }
     }
   }
 
-  return NS_OK;
-}
-
-nsresult
-HTMLSharedElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                             bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aName, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // If we're the first <base> with an href and our href attribute is being
-  // unset, then we're no longer the first <base> with an href, and we need to
-  // find the new one.  Similar for target.
-  if (mNodeInfo->Equals(nsGkAtoms::base) &&
-      aNameSpaceID == kNameSpaceID_None &&
-      IsInUncomposedDoc()) {
-    if (aName == nsGkAtoms::href) {
-      SetBaseURIUsingFirstBaseWithHref(GetUncomposedDoc(), nullptr);
-    } else if (aName == nsGkAtoms::target) {
-      SetBaseTargetUsingFirstBaseWithTarget(GetUncomposedDoc(), nullptr);
-    }
-  }
-
-  return NS_OK;
+  return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue,
+                                            aOldValue, aNotify);
 }
 
 nsresult
 HTMLSharedElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers)
 {
   nsresult rv = nsGenericHTMLElement::BindToTree(aDocument, aParent,
--- a/dom/html/HTMLSharedElement.h
+++ b/dom/html/HTMLSharedElement.h
@@ -54,27 +54,16 @@ public:
   // nsIDOMHTMLHtmlElement
   NS_DECL_NSIDOMHTMLHTMLELEMENT
 
   // nsIContent
   virtual bool ParseAttribute(int32_t aNamespaceID,
                                 nsIAtom* aAttribute,
                                 const nsAString& aValue,
                                 nsAttrValue& aResult) 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* aName,
-                             bool aNotify) override;
 
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers) override;
 
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
 
@@ -177,14 +166,19 @@ public:
     MOZ_ASSERT(mNodeInfo->Equals(nsGkAtoms::html));
     SetHTMLAttr(nsGkAtoms::version, aValue, aResult);
   }
 
 protected:
   virtual ~HTMLSharedElement();
 
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
+
+  virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_HTMLSharedElement_h
--- a/dom/html/HTMLSharedObjectElement.cpp
+++ b/dom/html/HTMLSharedObjectElement.cpp
@@ -156,37 +156,64 @@ HTMLSharedObjectElement::UnbindFromTree(
   // still contains a focused plugin when it doesn't -- which in turn can
   // disable text input in the browser window. See bug 1137229.
   HTMLObjectElement::OnFocusBlurPlugin(this, false);
 #endif
   nsObjectLoadingContent::UnbindFromTree(aDeep, aNullParent);
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
+nsresult
+HTMLSharedObjectElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                      const nsAttrValue* aValue,
+                                      const nsAttrValue* aOldValue,
+                                      bool aNotify)
+{
+  if (aValue) {
+    nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue,
+                                            aOldValue, aNotify);
+}
 
 nsresult
-HTMLSharedObjectElement::SetAttr(int32_t aNameSpaceID, nsIAtom *aName,
-                                 nsIAtom *aPrefix, const nsAString &aValue,
-                                 bool aNotify)
+HTMLSharedObjectElement::OnAttrSetButNotChanged(int32_t aNamespaceID,
+                                                nsIAtom* aName,
+                                                const nsAttrValueOrString& aValue,
+                                                bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                              aValue, aNotify);
+  nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // if aNotify is false, we are coming from the parser or some such place;
-  // we'll get bound after all the attributes have been set, so we'll do the
-  // object load from BindToTree/DoneAddingChildren.
-  // Skip the LoadObject call in that case.
-  // We also don't want to start loading the object when we're not yet in
-  // a document, just in case that the caller wants to set additional
-  // attributes before inserting the node into the document.
-  if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
-      aNameSpaceID == kNameSpaceID_None && aName == URIAttrName()
-      && !BlockEmbedOrObjectContentLoading()) {
-    return LoadObject(aNotify, true);
+  return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName,
+                                                      aValue, aNotify);
+}
+
+nsresult
+HTMLSharedObjectElement::AfterMaybeChangeAttr(int32_t aNamespaceID,
+                                              nsIAtom* aName,
+                                              bool aNotify)
+{
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == URIAttrName()) {
+      // If aNotify is false, we are coming from the parser or some such place;
+      // we'll get bound after all the attributes have been set, so we'll do the
+      // object load from BindToTree/DoneAddingChildren.
+      // Skip the LoadObject call in that case.
+      // We also don't want to start loading the object when we're not yet in
+      // a document, just in case that the caller wants to set additional
+      // attributes before inserting the node into the document.
+      if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
+          !BlockEmbedOrObjectContentLoading()) {
+        nsresult rv = LoadObject(aNotify, true);
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+    }
   }
 
   return NS_OK;
 }
 
 bool
 HTMLSharedObjectElement::IsHTMLFocusable(bool aWithMouse,
                                          bool *aIsFocusable,
--- a/dom/html/HTMLSharedObjectElement.h
+++ b/dom/html/HTMLSharedObjectElement.h
@@ -52,19 +52,16 @@ public:
   NS_IMETHOD GetType(nsAString &aType) override;
   NS_IMETHOD SetType(const nsAString &aType) override;
 
   virtual nsresult BindToTree(nsIDocument *aDocument, nsIContent *aParent,
                               nsIContent *aBindingParent,
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
-  virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom *aName,
-                           nsIAtom *aPrefix, const nsAString &aValue,
-                           bool aNotify) override;
 
   virtual bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override;
   virtual IMEState GetDesiredIMEState() override;
 
   virtual void DoneAddingChildren(bool aHaveNotified) override;
   virtual bool IsDoneAddingChildren() override;
 
   virtual bool ParseAttribute(int32_t aNamespaceID,
@@ -201,16 +198,24 @@ public:
    * Calls LoadObject with the correct arguments to start the plugin load.
    */
   void StartObjectLoad(bool aNotify, bool aForceLoad);
 
 protected:
   // Override for nsImageLoadingContent.
   nsIContent* AsContent() override { return this; }
 
+  virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+  virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify) override;
+
 private:
   virtual ~HTMLSharedObjectElement();
 
   nsIAtom *URIAttrName() const
   {
     return mNodeInfo->Equals(nsGkAtoms::applet) ?
            nsGkAtoms::code :
            nsGkAtoms::src;
@@ -221,14 +226,25 @@ private:
   // mIsDoneAddingChildren is only really used for <applet>.  This boolean is
   // always true for <embed>, per the documentation in nsIContent.h.
   bool mIsDoneAddingChildren;
 
   virtual JSObject* WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     GenericSpecifiedValues* aGenericData);
+
+  /**
+   * This function is called by AfterSetAttr and OnAttrSetButNotChanged.
+   * It will not be called if the value is being unset.
+   *
+   * @param aNamespaceID the namespace of the attr being set
+   * @param aName the localname of the attribute being set
+   * @param aNotify Whether we plan to notify document observers.
+   */
+  nsresult AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                bool aNotify);
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_HTMLSharedObjectElement_h
--- a/dom/html/nsGenericHTMLFrameElement.cpp
+++ b/dom/html/nsGenericHTMLFrameElement.cpp
@@ -299,65 +299,16 @@ nsGenericHTMLFrameElement::UnbindFromTre
     // later bug.
     mFrameLoader->Destroy();
     mFrameLoader = nullptr;
   }
 
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
-nsresult
-nsGenericHTMLFrameElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                                   nsIAtom* aPrefix, const nsAString& aValue,
-                                   bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
-                                              aValue, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src &&
-      (!IsHTMLElement(nsGkAtoms::iframe) ||
-       !HasAttr(kNameSpaceID_None,nsGkAtoms::srcdoc))) {
-    // Don't propagate error here. The attribute was successfully set, that's
-    // what we should reflect.
-    LoadSrc();
-  } else if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::name) {
-    // Propagate "name" to the docshell to make browsing context names live,
-    // per HTML5.
-    nsIDocShell *docShell = mFrameLoader ? mFrameLoader->GetExistingDocShell()
-                                         : nullptr;
-    if (docShell) {
-      docShell->SetName(aValue);
-    }
-  }
-
-  return NS_OK;
-}
-
-nsresult
-nsGenericHTMLFrameElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                                     bool aNotify)
-{
-  // Invoke on the superclass.
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::name) {
-    // Propagate "name" to the docshell to make browsing context names live,
-    // per HTML5.
-    nsIDocShell *docShell = mFrameLoader ? mFrameLoader->GetExistingDocShell()
-                                         : nullptr;
-    if (docShell) {
-      docShell->SetName(EmptyString());
-    }
-  }
-
-  return NS_OK;
-}
-
 /* static */ int32_t
 nsGenericHTMLFrameElement::MapScrollingAttribute(const nsAttrValue* aValue)
 {
   int32_t mappedValue = nsIScrollable::Scrollbar_Auto;
   if (aValue && aValue->Type() == nsAttrValue::eEnum) {
     switch (aValue->GetEnumValue()) {
       case NS_STYLE_FRAME_OFF:
       case NS_STYLE_FRAME_NOSCROLL:
@@ -380,47 +331,96 @@ PrincipalAllowsBrowserFrame(nsIPrincipal
   return permission == nsIPermissionManager::ALLOW_ACTION;
 }
 
 /* virtual */ nsresult
 nsGenericHTMLFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
                                         const nsAttrValue* aValue,
                                         const nsAttrValue* aOldValue, bool aNotify)
 {
-  if (aName == nsGkAtoms::scrolling && aNameSpaceID == kNameSpaceID_None) {
-    if (mFrameLoader) {
-      nsIDocShell* docshell = mFrameLoader->GetExistingDocShell();
-      nsCOMPtr<nsIScrollable> scrollable = do_QueryInterface(docshell);
-      if (scrollable) {
-        int32_t cur;
-        scrollable->GetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, &cur);
-        int32_t val = MapScrollingAttribute(aValue);
-        if (cur != val) {
-          scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, val);
-          scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_Y, val);
-          RefPtr<nsPresContext> presContext;
-          docshell->GetPresContext(getter_AddRefs(presContext));
-          nsIPresShell* shell = presContext ? presContext->GetPresShell() : nullptr;
-          nsIFrame* rootScroll = shell ? shell->GetRootScrollFrame() : nullptr;
-          if (rootScroll) {
-            shell->FrameNeedsReflow(rootScroll, nsIPresShell::eStyleChange,
-                                    NS_FRAME_IS_DIRTY);
+  if (aValue) {
+    nsAttrValueOrString value(aValue);
+    AfterMaybeChangeAttr(aNameSpaceID, aName, &value, aNotify);
+  } else {
+    AfterMaybeChangeAttr(aNameSpaceID, aName, nullptr, aNotify);
+  }
+
+  if (aNameSpaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::scrolling) {
+      if (mFrameLoader) {
+        nsIDocShell* docshell = mFrameLoader->GetExistingDocShell();
+        nsCOMPtr<nsIScrollable> scrollable = do_QueryInterface(docshell);
+        if (scrollable) {
+          int32_t cur;
+          scrollable->GetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, &cur);
+          int32_t val = MapScrollingAttribute(aValue);
+          if (cur != val) {
+            scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, val);
+            scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_Y, val);
+            RefPtr<nsPresContext> presContext;
+            docshell->GetPresContext(getter_AddRefs(presContext));
+            nsIPresShell* shell = presContext ? presContext->GetPresShell() : nullptr;
+            nsIFrame* rootScroll = shell ? shell->GetRootScrollFrame() : nullptr;
+            if (rootScroll) {
+              shell->FrameNeedsReflow(rootScroll, nsIPresShell::eStyleChange,
+                                      NS_FRAME_IS_DIRTY);
+            }
           }
         }
       }
+    } else if (aName == nsGkAtoms::mozbrowser) {
+      mReallyIsBrowser = !!aValue && BrowserFramesEnabled() &&
+                         PrincipalAllowsBrowserFrame(NodePrincipal());
+    }
+  }
+
+  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
+                                            aOldValue, aNotify);
+}
+
+nsresult
+nsGenericHTMLFrameElement::OnAttrSetButNotChanged(int32_t aNamespaceID,
+                                                  nsIAtom* aName,
+                                                  const nsAttrValueOrString& aValue,
+                                                  bool aNotify)
+{
+  AfterMaybeChangeAttr(aNamespaceID, aName, &aValue, aNotify);
+
+  return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName,
+                                                      aValue, aNotify);
+}
+
+void
+nsGenericHTMLFrameElement::AfterMaybeChangeAttr(int32_t aNamespaceID,
+                                                nsIAtom* aName,
+                                                const nsAttrValueOrString* aValue,
+                                                bool aNotify)
+{
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::src) {
+      if (aValue && (!IsHTMLElement(nsGkAtoms::iframe) ||
+          !HasAttr(kNameSpaceID_None, nsGkAtoms::srcdoc))) {
+        // Don't propagate error here. The attribute was successfully set,
+        // that's what we should reflect.
+        LoadSrc();
+      }
+    } else if (aName == nsGkAtoms::name) {
+      // Propagate "name" to the docshell to make browsing context names live,
+      // per HTML5.
+      nsIDocShell* docShell = mFrameLoader ? mFrameLoader->GetExistingDocShell()
+                                           : nullptr;
+      if (docShell) {
+        if (aValue) {
+          docShell->SetName(aValue->String());
+        } else {
+          docShell->SetName(EmptyString());
+        }
+      }
     }
   }
-
-  if (aName == nsGkAtoms::mozbrowser && aNameSpaceID == kNameSpaceID_None) {
-    mReallyIsBrowser = !!aValue && BrowserFramesEnabled() &&
-                       PrincipalAllowsBrowserFrame(NodePrincipal());
-  }
-
-  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
-                                            aOldValue, aNotify);
 }
 
 void
 nsGenericHTMLFrameElement::DestroyContent()
 {
   if (mFrameLoader) {
     mFrameLoader->Destroy();
     mFrameLoader = nullptr;
--- a/dom/html/nsGenericHTMLFrameElement.h
+++ b/dom/html/nsGenericHTMLFrameElement.h
@@ -48,30 +48,16 @@ public:
 
   // nsIContent
   virtual bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override;
   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,
-                                const nsAttrValue* aOldValue,
-                                bool aNotify) override;
   virtual void DestroyContent() override;
 
   nsresult CopyInnerTo(mozilla::dom::Element* aDest, bool aPreallocateChildren);
 
   virtual int32_t TabIndexDefault() override;
 
   virtual nsIMozBrowserFrame* GetAsMozBrowserFrame() override { return this; }
 
@@ -109,16 +95,24 @@ protected:
   // This doesn't really ensure a frame loader in all cases, only when
   // it makes sense.
   void EnsureFrameLoader();
   nsresult LoadSrc();
   nsIDocument* GetContentDocument(nsIPrincipal& aSubjectPrincipal);
   nsresult GetContentDocument(nsIDOMDocument** aContentDocument);
   already_AddRefed<nsPIDOMWindowOuter> GetContentWindow();
 
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+  virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
+                                          const nsAttrValueOrString& aValue,
+                                          bool aNotify) override;
+
   RefPtr<nsFrameLoader> mFrameLoader;
   nsCOMPtr<nsPIDOMWindowOuter> mOpenerWindow;
 
   /**
    * True when the element is created by the parser using the
    * NS_FROM_PARSER_NETWORK flag.
    * If the element is modified, it may lose the flag.
    */
@@ -131,11 +125,23 @@ protected:
 
   // This flag is only used by <iframe>. See HTMLIFrameElement::
   // FullscreenFlag() for details. It is placed here so that we
   // do not bloat any struct.
   bool mFullscreenFlag = false;
 
 private:
   void GetManifestURL(nsAString& aOut);
+
+  /**
+   * This function is called by AfterSetAttr and OnAttrSetButNotChanged.
+   * It will be called whether the value is being set or unset.
+   *
+   * @param aNamespaceID the namespace of the attr being set
+   * @param aName the localname of the attribute being set
+   * @param aValue the value being set or null if the value is being unset
+   * @param aNotify Whether we plan to notify document observers.
+   */
+  void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
+                            const nsAttrValueOrString* aValue, bool aNotify);
 };
 
 #endif // nsGenericHTMLFrameElement_h
--- a/dom/mathml/nsMathMLElement.cpp
+++ b/dom/mathml/nsMathMLElement.cpp
@@ -1063,59 +1063,36 @@ nsMathMLElement::GetLinkTarget(nsAString
 already_AddRefed<nsIURI>
 nsMathMLElement::GetHrefURI() const
 {
   nsCOMPtr<nsIURI> hrefURI;
   return IsLink(getter_AddRefs(hrefURI)) ? hrefURI.forget() : nullptr;
 }
 
 nsresult
-nsMathMLElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                         nsIAtom* aPrefix, const nsAString& aValue,
-                         bool aNotify)
+nsMathMLElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                              const nsAttrValue* aValue,
+                              const nsAttrValue* aOldValue, bool aNotify)
 {
-  nsresult rv = nsMathMLElementBase::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
+  // It is important that this be done after the attribute is set/unset.
+  // 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 (aName == nsGkAtoms::href &&
       (aNameSpaceID == kNameSpaceID_None ||
        aNameSpaceID == kNameSpaceID_XLink)) {
-    if (aNameSpaceID == kNameSpaceID_XLink) {
+    if (aValue && aNameSpaceID == kNameSpaceID_XLink) {
       WarnDeprecated(u"xlink:href", u"href", OwnerDoc());
     }
-    Link::ResetLinkState(!!aNotify, true);
+    // Note: When unsetting href, there may still be another href since there
+    // are 2 possible namespaces.
+    Link::ResetLinkState(aNotify, aValue || Link::ElementHasHref());
   }
 
-  return rv;
-}
-
-nsresult
-nsMathMLElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttr,
-                           bool aNotify)
-{
-  nsresult rv = nsMathMLElementBase::UnsetAttr(aNameSpaceID, aAttr, aNotify);
-
-  // 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 (aAttr == nsGkAtoms::href &&
-      (aNameSpaceID == kNameSpaceID_None ||
-       aNameSpaceID == kNameSpaceID_XLink)) {
-    // Note: just because we removed a single href attr doesn't mean there's no href,
-    // since there are 2 possible namespaces.
-    Link::ResetLinkState(!!aNotify, Link::ElementHasHref());
-  }
-
-  return rv;
+  return nsMathMLElementBase::AfterSetAttr(aNameSpaceID, aName, aValue,
+                                           aOldValue, aNotify);
 }
 
 JSObject*
 nsMathMLElement::WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return ElementBinding::Wrap(aCx, this, aGivenProto);
 }
--- a/dom/mathml/nsMathMLElement.h
+++ b/dom/mathml/nsMathMLElement.h
@@ -89,37 +89,32 @@ public:
   bool GetIncrementScriptLevel() const {
     return mIncrementScriptLevel;
   }
 
   virtual bool IsFocusableInternal(int32_t* aTabIndex, bool aWithMouse) override;
   virtual bool IsLink(nsIURI** aURI) const override;
   virtual void GetLinkTarget(nsAString& aTarget) override;
   virtual already_AddRefed<nsIURI> GetHrefURI() const 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 nsIDOMNode* AsDOMNode() override { return this; }
 
   virtual void NodeInfoChanged(nsIDocument* aOldDoc) override
   {
     ClearHasPendingLinkUpdate();
     nsMathMLElementBase::NodeInfoChanged(aOldDoc);
   }
 
 protected:
   virtual ~nsMathMLElement() {}
 
   virtual JSObject* WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto) override;
 
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                bool aNotify) override;
+
 private:
   bool mIncrementScriptLevel;
 };
 
 #endif // nsMathMLElement_h
--- a/dom/xbl/XBLChildrenElement.cpp
+++ b/dom/xbl/XBLChildrenElement.cpp
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/XBLChildrenElement.h"
 #include "nsCharSeparatedTokenizer.h"
 #include "mozilla/dom/NodeListBinding.h"
+#include "nsAttrValueOrString.h"
 
 namespace mozilla {
 namespace dom {
 
 XBLChildrenElement::~XBLChildrenElement()
 {
 }
 
@@ -22,45 +23,34 @@ NS_INTERFACE_TABLE_HEAD(XBLChildrenEleme
   NS_INTERFACE_TABLE_INHERITED(XBLChildrenElement, nsIDOMNode,
                                nsIDOMElement)
   NS_ELEMENT_INTERFACE_TABLE_TO_MAP_SEGUE
 NS_INTERFACE_MAP_END_INHERITING(Element)
 
 NS_IMPL_ELEMENT_CLONE(XBLChildrenElement)
 
 nsresult
-XBLChildrenElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                              bool aNotify)
+XBLChildrenElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                  const nsAttrValueOrString* aValue,
+                                  bool aNotify)
 {
-  if (aAttribute == nsGkAtoms::includes &&
-      aNameSpaceID == kNameSpaceID_None) {
-    mIncludes.Clear();
-  }
-
-  return Element::UnsetAttr(aNameSpaceID, aAttribute, aNotify);
-}
-
-bool
-XBLChildrenElement::ParseAttribute(int32_t aNamespaceID,
-                                   nsIAtom* aAttribute,
-                                   const nsAString& aValue,
-                                   nsAttrValue& aResult)
-{
-  if (aAttribute == nsGkAtoms::includes &&
-      aNamespaceID == kNameSpaceID_None) {
-    mIncludes.Clear();
-    nsCharSeparatedTokenizer tok(aValue, '|',
-                                 nsCharSeparatedTokenizer::SEPARATOR_OPTIONAL);
-    while (tok.hasMoreTokens()) {
-      mIncludes.AppendElement(NS_Atomize(tok.nextToken()));
+  if (aNamespaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::includes) {
+      mIncludes.Clear();
+      if (aValue) {
+        nsCharSeparatedTokenizer tok(aValue->String(), '|',
+                                     nsCharSeparatedTokenizer::SEPARATOR_OPTIONAL);
+        while (tok.hasMoreTokens()) {
+          mIncludes.AppendElement(NS_Atomize(tok.nextToken()));
+        }
+      }
     }
   }
 
-  return nsXMLElement::ParseAttribute(aNamespaceID, aAttribute,
-                                      aValue, aResult);
+  return nsXMLElement::BeforeSetAttr(aNamespaceID, aName, aValue, aNotify);
 }
 
 } // namespace dom
 } // namespace mozilla
 
 using namespace mozilla::dom;
 
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(nsAnonymousContentList, mParent)
--- a/dom/xbl/XBLChildrenElement.h
+++ b/dom/xbl/XBLChildrenElement.h
@@ -33,24 +33,16 @@ public:
   NS_DECL_ISUPPORTS_INHERITED
 
   // nsINode interface methods
   virtual nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult,
                          bool aPreallocateChildren) const override;
 
   virtual nsIDOMNode* AsDOMNode() override { return this; }
 
-  // nsIContent interface methods
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
-  virtual bool ParseAttribute(int32_t aNamespaceID,
-                              nsIAtom* aAttribute,
-                              const nsAString& aValue,
-                              nsAttrValue& aResult) override;
-
   void AppendInsertedChild(nsIContent* aChild)
   {
     mInsertedChildren.AppendElement(aChild);
     aChild->SetXBLInsertionParent(GetParent());
 
     // Appending an inserted child causes the inserted
     // children to be projected instead of default content.
     MaybeRemoveDefaultContent();
@@ -142,16 +134,19 @@ public:
 
   nsIContent* InsertedChild(uint32_t aIndex)
   {
     return mInsertedChildren[aIndex];
   }
 
 protected:
   ~XBLChildrenElement();
+  virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
+                                 const nsAttrValueOrString* aValue,
+                                 bool aNotify) override;
 
 private:
   nsTArray<nsIContent*> mInsertedChildren; // WEAK
   nsTArray<nsCOMPtr<nsIAtom> > mIncludes;
 };
 
 } // namespace dom
 } // namespace mozilla