Bug 1850238 - Make BROKEN state non-intrinsic. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D187108

dom/base/Element.h

@@ -74,6 +74,7 @@ class nsFocusManager;
 class nsGenericHTMLFormControlElementWithState;
 class nsGlobalWindowInner;
 class nsGlobalWindowOuter;
+class nsImageLoadingContent;
 class nsIAutoCompletePopup;
 class nsIBrowser;
 class nsIDOMXULButtonElement;
@@ -93,6 +94,7 @@ class nsIPrincipal;
 class nsIScreen;
 class nsIScrollableFrame;
 class nsIURI;
+class nsObjectLoadingContent;
 class nsPresContext;
 class nsWindowSizes;
 struct JSContext;
@@ -758,6 +760,8 @@ class Element : public FragmentOrElement {
   friend class ::nsGlobalWindowInner;
   friend class ::nsGlobalWindowOuter;
   friend class ::nsFocusManager;
+  friend class ::nsImageLoadingContent;
+  friend class ::nsObjectLoadingContent;
   friend class mozilla::dom::HTMLFormElement;
 
   // Allow CusomtElementRegistry to call AddStates.

dom/base/GeneratedImageContent.h

@@ -32,13 +32,6 @@ class GeneratedImageContent final : public nsGenericHTMLElement {
                "Someone messed up our nodeinfo");
   }
 
-  ElementState IntrinsicState() const override {
-    ElementState state = nsGenericHTMLElement::IntrinsicState();
-    if (mBroken) {
-      state |= ElementState::BROKEN;
-    }
-    return state;
-  }
   nsresult Clone(dom::NodeInfo* aNodeInfo, nsINode** aResult) const final;
 
   nsresult CopyInnerTo(GeneratedImageContent* aDest) {
@@ -56,8 +49,7 @@ class GeneratedImageContent final : public nsGenericHTMLElement {
 
   // Notify this image failed to load.
   void NotifyLoadFailed() {
-    mBroken = true;
+    SetStates(ElementState::BROKEN, true);
-    UpdateState(true);
   }
 
  protected:
@@ -66,7 +58,6 @@ class GeneratedImageContent final : public nsGenericHTMLElement {
  private:
   virtual ~GeneratedImageContent() = default;
   uint32_t mIndex = 0;
-  bool mBroken = false;
 };
 
 }  // namespace mozilla::dom

dom/base/nsImageLoadingContent.cpp

@@ -103,8 +103,6 @@ nsImageLoadingContent::nsImageLoadingContent()
       mRequestGeneration(0),
       mLoadingEnabled(true),
       mLoading(false),
-      // mBroken starts out true, since an image without a URI is broken....
-      mBroken(true),
       mNewRequestsWillNeedAnimationReset(false),
       mUseUrgentStartForChannel(false),
       mLazyLoading(false),
@@ -1346,14 +1344,6 @@ CSSIntSize nsImageLoadingContent::GetWidthHeightForImage() {
   return size;
 }
 
-ElementState nsImageLoadingContent::ImageState() const {
-  ElementState states;
-  if (mBroken) {
-    states |= ElementState::BROKEN;
-  }
-  return states;
-}
-
 void nsImageLoadingContent::UpdateImageState(bool aNotify) {
   if (mStateChangerDepth > 0) {
     // Ignore this call; we'll update our state when the outermost state changer
@@ -1365,9 +1355,12 @@ void nsImageLoadingContent::UpdateImageState(bool aNotify) {
     return;
   }
 
-  nsIContent* thisContent = AsContent();
+  Element* thisElement = AsContent()->AsElement();
 
-  mLoading = mBroken = false;
+  mLoading = false;
+
+  Element::AutoStateChangeNotifier notifier(*thisElement, aNotify);
+  thisElement->RemoveStatesSilently(ElementState::BROKEN);
 
   // If we were blocked, we're broken, so are we if we don't have an image
   // request at all or the image has errored.
@@ -1375,21 +1368,19 @@ void nsImageLoadingContent::UpdateImageState(bool aNotify) {
     if (!mLazyLoading) {
       // In case of non-lazy loading, no current request means error, since we
       // weren't disabled or suppressed
-      mBroken = true;
+      thisElement->AddStatesSilently(ElementState::BROKEN);
       RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
     }
   } else {
     uint32_t currentLoadStatus;
     nsresult rv = mCurrentRequest->GetImageStatus(&currentLoadStatus);
     if (NS_FAILED(rv) || (currentLoadStatus & imgIRequest::STATUS_ERROR)) {
-      mBroken = true;
+      thisElement->AddStatesSilently(ElementState::BROKEN);
       RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
     } else if (!(currentLoadStatus & imgIRequest::STATUS_SIZE_AVAILABLE)) {
       mLoading = true;
     }
   }
-
-  thisContent->AsElement()->UpdateState(aNotify);
 }
 
 void nsImageLoadingContent::CancelImageRequests(bool aNotify) {

dom/base/nsImageLoadingContent.h

@@ -560,7 +560,6 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
    * document of a state change.  These are maintained by UpdateImageState.
    */
   bool mLoading : 1;
-  bool mBroken : 1;
 
   /**
    * A hack to get animations to reset, see bug 594771. On requests

dom/base/nsObjectLoadingContent.cpp

@@ -676,35 +676,6 @@ nsObjectLoadingContent::AsyncOnChannelRedirect(
   return NS_OK;
 }
 
-// <public>
-ElementState nsObjectLoadingContent::ObjectState() const {
-  switch (mType) {
-    case eType_Loading:
-      return {};
-    case eType_Image:
-      return ImageState();
-    case eType_FakePlugin:
-    case eType_Document: {
-      // These are OK. If documents start to load successfully, they display
-      // something, and are thus not broken in this sense. The same goes for
-      // plugins.
-      ElementState states = ElementState();
-      if (mLoadingSyntheticDocument) {
-        states |= ImageState();
-      }
-      return states;
-    }
-    case eType_Fallback:
-      // This may end up handled as TYPE_NULL or as a "special" type, as
-      // chosen by the layout.use-plugin-fallback pref.
-      return ElementState();
-    case eType_Null:
-      return ElementState::BROKEN;
-  }
-  MOZ_ASSERT_UNREACHABLE("unknown type?");
-  return {};
-}
-
 void nsObjectLoadingContent::MaybeRewriteYoutubeEmbed(nsIURI* aURI,
                                                       nsIURI* aBaseURI,
                                                       nsIURI** aRewrittenURI) {
@@ -1300,7 +1271,7 @@ nsresult nsObjectLoadingContent::LoadObject(bool aNotify, bool aForceLoad,
     ObjectType oldType = mType;
     mType = eType_Fallback;
     ConfigureFallback();
-    NotifyStateChanged(oldType, ObjectState(), true, false);
+    NotifyStateChanged(oldType, true);
     return NS_OK;
   }
 
@@ -1333,7 +1304,6 @@ nsresult nsObjectLoadingContent::LoadObject(bool aNotify, bool aForceLoad,
   }
 
   // Save these for NotifyStateChanged();
-  ElementState oldState = ObjectState();
   ObjectType oldType = mType;
 
   ParameterUpdateFlags stateChange = UpdateObjectParameters();
@@ -1586,7 +1556,7 @@ nsresult nsObjectLoadingContent::LoadObject(bool aNotify, bool aForceLoad,
   }
 
   // Notify of our final state
-  NotifyStateChanged(oldType, oldState, aNotify, false);
+  NotifyStateChanged(oldType, aNotify);
   NS_ENSURE_TRUE(mIsLoading, NS_OK);
 
   //
@@ -1625,7 +1595,7 @@ nsresult nsObjectLoadingContent::LoadObject(bool aNotify, bool aForceLoad,
     NS_ENSURE_TRUE(mIsLoading, NS_OK);
     CloseChannel();
     ConfigureFallback();
-    NotifyStateChanged(oldType, ObjectState(), true, false);
+    NotifyStateChanged(oldType, true);
   }
 
   return NS_OK;
@@ -1883,26 +1853,17 @@ void nsObjectLoadingContent::UnloadObject(bool aResetState) {
 }
 
 void nsObjectLoadingContent::NotifyStateChanged(ObjectType aOldType,
-                                                ElementState aOldState,
+                                                bool aNotify) {
-                                                bool aNotify,
+  LOG(("OBJLC [%p]: NotifyStateChanged: (%u) -> (%u) (notify %i)", this,
-                                                bool aForceRestyle) {
+       aOldType, mType, aNotify));
-  LOG(("OBJLC [%p]: NotifyStateChanged: (%u, %" PRIx64 ") -> (%u, %" PRIx64 ")"
-       " (notify %i)",
-       this, aOldType, aOldState.GetInternalValue(), mType,
-       ObjectState().GetInternalValue(), aNotify));
 
-  nsCOMPtr<dom::Element> thisEl = AsContent()->AsElement();
+  dom::Element* thisEl = AsContent()->AsElement();
-  MOZ_ASSERT(thisEl, "must be an element");
+  if (mType != eType_Image) {
+    // Non-images are always not broken.
+    thisEl->RemoveStates(ElementState::BROKEN, aNotify);
+  }
 
-  // XXX(johns): A good bit of the code below replicates UpdateState(true)
+  if (mType == aOldType) {
-
-  // Unfortunately, we do some state changes without notifying
-  // (e.g. in Fallback when canceling image requests), so we have to
-  // manually notify object state changes.
-  thisEl->UpdateState(aForceRestyle);
-
-  if (!aNotify) {
-    // We're done here
     return;
   }
 
@@ -1911,21 +1872,13 @@ void nsObjectLoadingContent::NotifyStateChanged(ObjectType aOldType,
     return;  // Nothing to do
   }
 
-  const ElementState newState = ObjectState();
+  PresShell* presShell = doc->GetPresShell();
-  if (newState == aOldState && mType == aOldType) {
-    return;  // Also done.
-  }
-
-  RefPtr<PresShell> presShell = doc->GetPresShell();
   // If there is no PresShell or it hasn't been initialized there isn't much to
   // do.
   if (!presShell || !presShell->DidInitialize()) {
     return;
   }
-
-  if (presShell && (aOldType != mType)) {
   presShell->PostRecreateFramesFor(thisEl);
-  }
 }
 
 nsObjectLoadingContent::ObjectType nsObjectLoadingContent::GetTypeOfContent(
@@ -2200,7 +2153,6 @@ void nsObjectLoadingContent::SubdocumentIntrinsicSizeOrRatioChanged(
 }
 
 void nsObjectLoadingContent::SubdocumentImageLoadComplete(nsresult aResult) {
-  ElementState oldState = ObjectState();
   ObjectType oldType = mType;
   mLoadingSyntheticDocument = false;
 
@@ -2208,15 +2160,14 @@ void nsObjectLoadingContent::SubdocumentImageLoadComplete(nsresult aResult) {
     UnloadObject();
     mType = eType_Fallback;
     ConfigureFallback();
-    NotifyStateChanged(oldType, oldState, true, false);
+    NotifyStateChanged(oldType, true);
     return;
   }
 
   // (mChannelLoaded && mChannel) indicates this is a good state, not any sort
   // of failures.
   MOZ_DIAGNOSTIC_ASSERT_IF(mChannelLoaded && mChannel, mType == eType_Document);
-
+  NotifyStateChanged(oldType, true);
-  NotifyStateChanged(oldType, oldState, true, true);
 }
 
 void nsObjectLoadingContent::MaybeStoreCrossOriginFeaturePolicy() {

dom/base/nsObjectLoadingContent.h

@@ -78,12 +78,6 @@ class nsObjectLoadingContent : public nsImageLoadingContent,
   NS_DECL_NSIOBJECTLOADINGCONTENT
   NS_DECL_NSICHANNELEVENTSINK
 
-  /**
-   * Object state. This is a bitmask of NS_EVENT_STATEs epresenting the
-   * current state of the object.
-   */
-  mozilla::dom::ElementState ObjectState() const;
-
   ObjectType Type() const { return mType; }
 
   void SetIsNetworkCreated(bool aNetworkCreated) {
@@ -429,9 +423,7 @@ class nsObjectLoadingContent : public nsImageLoadingContent,
    *
    * @param aNotify if false, only need to update the state of our element.
    */
-  void NotifyStateChanged(ObjectType aOldType,
+  void NotifyStateChanged(ObjectType aOldType, bool aNotify);
-                          mozilla::dom::ElementState aOldState, bool aNotify,
-                          bool aForceRestyle);
 
   /**
    * Returns a ObjectType value corresponding to the type of content we would

dom/base/rust/lib.rs

@@ -189,6 +189,7 @@ bitflags! {
             Self::OUTOFRANGE.bits |
             Self::VISITED.bits |
             Self::UNVISITED.bits |
+            Self::BROKEN.bits |
             Self::VALIDITY_STATES.bits;
 
         const INTRINSIC_STATES = !Self::EXTERNALLY_MANAGED_STATES.bits;

dom/html/HTMLEmbedElement.cpp

@@ -233,10 +233,6 @@ void HTMLEmbedElement::StartObjectLoad(bool aNotify, bool aForceLoad) {
   SetIsNetworkCreated(false);
 }
 
-ElementState HTMLEmbedElement::IntrinsicState() const {
-  return nsGenericHTMLElement::IntrinsicState() | ObjectState();
-}
-
 uint32_t HTMLEmbedElement::GetCapabilities() const {
   return eSupportPlugins | eAllowPluginSkipChannel | eSupportImages |
          eSupportDocuments;

dom/html/HTMLEmbedElement.h

@@ -49,7 +49,6 @@ class HTMLEmbedElement final : public nsGenericHTMLElement,
                       nsAttrValue& aResult) override;
   nsMapRuleToAttributesFunc GetAttributeMappingFunction() const override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
-  ElementState IntrinsicState() const override;
   void DestroyContent() override;
 
   // nsObjectLoadingContent

dom/html/HTMLImageElement.cpp

@@ -649,11 +649,6 @@ void HTMLImageElement::MaybeLoadImage(bool aAlwaysForceLoad) {
   }
 }
 
-ElementState HTMLImageElement::IntrinsicState() const {
-  return nsGenericHTMLElement::IntrinsicState() |
-         nsImageLoadingContent::ImageState();
-}
-
 void HTMLImageElement::NodeInfoChanged(Document* aOldDoc) {
   nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
 

dom/html/HTMLImageElement.h

@@ -76,7 +76,6 @@ class HTMLImageElement final : public nsGenericHTMLElement,
   nsresult BindToTree(BindContext&, nsINode& aParent) override;
   void UnbindFromTree(bool aNullParent) override;
 
-  ElementState IntrinsicState() const override;
   nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
 
   void NodeInfoChanged(Document* aOldDoc) override;

dom/html/HTMLInputElement.cpp

@@ -4528,11 +4528,14 @@ void HTMLInputElement::HandleTypeChange(FormControlType aNewType,
       // We're no longer an image input.  Cancel our image requests, if we have
       // any.
       CancelImageRequests(aNotify);
-    } else if (aNotify) {
+      RemoveStates(ElementState::BROKEN, aNotify);
+    } else {
       // We just got switched to be an image input; we should see whether we
       // have an image to load;
+      bool hasSrc = false;
+      if (aNotify) {
         nsAutoString src;
-      if (GetAttr(nsGkAtoms::src, src)) {
+        if ((hasSrc = GetAttr(nsGkAtoms::src, src))) {
           // Mark channel as urgent-start before load image if the image load is
           // initiated by a user interaction.
           mUseUrgentStartForChannel = UserActivation::IsHandlingUserInput();
@@ -4540,6 +4543,12 @@ void HTMLInputElement::HandleTypeChange(FormControlType aNewType,
           LoadImage(src, false, aNotify, eImageLoadType_Normal,
                     mSrcTriggeringPrincipal);
         }
+      } else {
+        hasSrc = HasAttr(nsGkAtoms::src);
+      }
+      if (!hasSrc) {
+        AddStates(ElementState::BROKEN, aNotify);
+      }
     }
     // We should update our mapped attribute mapping function.
     if (mAttrs.HasAttrs() && !mAttrs.IsPendingMappedAttributeEvaluation()) {
@@ -6059,17 +6068,6 @@ void HTMLInputElement::DestroyContent() {
   TextControlElement::DestroyContent();
 }
 
-ElementState HTMLInputElement::IntrinsicState() const {
-  // If you add states here, and they're type-dependent, you need to add them to
-  // HandleTypeChange.
-  ElementState state =
-      nsGenericHTMLFormControlElementWithState::IntrinsicState();
-  if (mType == FormControlType::InputImage) {
-    state |= nsImageLoadingContent::ImageState();
-  }
-  return state;
-}
-
 void HTMLInputElement::UpdateValidityElementStates(bool aNotify) {
   AutoStateChangeNotifier notifier(*this, aNotify);
   RemoveStatesSilently(ElementState::VALIDITY_STATES);

dom/html/HTMLInputElement.h

@@ -211,8 +211,6 @@ class HTMLInputElement final : public TextControlElement,
 
   void DestroyContent() override;
 
-  ElementState IntrinsicState() const override;
-
   void SetLastValueChangeWasInteractive(bool);
 
   // TextControlElement

dom/html/HTMLMeterElement.cpp

@@ -61,11 +61,6 @@ void HTMLMeterElement::UpdateOptimumState(bool aNotify) {
   AddStatesSilently(GetOptimumState());
 }
 
-/*
- * Value getters :
- * const getters used by XPCOM methods and by IntrinsicState
- */
-
 double HTMLMeterElement::Min() const {
   /**
    * If the attribute min is defined, the minimum is this value.

dom/html/HTMLObjectElement.cpp

@@ -266,10 +266,6 @@ void HTMLObjectElement::StartObjectLoad(bool aNotify, bool aForce) {
   SetIsNetworkCreated(false);
 }
 
-ElementState HTMLObjectElement::IntrinsicState() const {
-  return nsGenericHTMLFormControlElement::IntrinsicState() | ObjectState();
-}
-
 uint32_t HTMLObjectElement::GetCapabilities() const {
   return nsObjectLoadingContent::GetCapabilities() | eFallbackIfClassIDPresent;
 }

dom/html/HTMLObjectElement.h

@@ -58,7 +58,6 @@ class HTMLObjectElement final : public nsGenericHTMLFormControlElement,
                       nsAttrValue& aResult) override;
   nsMapRuleToAttributesFunc GetAttributeMappingFunction() const override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
-  ElementState IntrinsicState() const override;
   void DestroyContent() override;
 
   // nsObjectLoadingContent

dom/svg/SVGFEImageElement.cpp

@@ -175,11 +175,6 @@ void SVGFEImageElement::UnbindFromTree(bool aNullParent) {
   SVGFEImageElementBase::UnbindFromTree(aNullParent);
 }
 
-ElementState SVGFEImageElement::IntrinsicState() const {
-  return SVGFEImageElementBase::IntrinsicState() |
-         nsImageLoadingContent::ImageState();
-}
-
 void SVGFEImageElement::DestroyContent() {
   nsImageLoadingContent::Destroy();
   SVGFEImageElementBase::DestroyContent();

dom/svg/SVGFEImageElement.h

@@ -71,7 +71,6 @@ class SVGFEImageElement final : public SVGFEImageElementBase,
                     nsIPrincipal* aSubjectPrincipal, bool aNotify) override;
   nsresult BindToTree(BindContext&, nsINode& aParent) override;
   void UnbindFromTree(bool aNullParent) override;
-  ElementState IntrinsicState() const override;
   void DestroyContent() override;
 
   NS_DECL_IMGINOTIFICATIONOBSERVER

dom/svg/SVGImageElement.cpp

@@ -265,11 +265,6 @@ void SVGImageElement::UnbindFromTree(bool aNullParent) {
   SVGImageElementBase::UnbindFromTree(aNullParent);
 }
 
-ElementState SVGImageElement::IntrinsicState() const {
-  return SVGImageElementBase::IntrinsicState() |
-         nsImageLoadingContent::ImageState();
-}
-
 void SVGImageElement::DestroyContent() {
   nsImageLoadingContent::Destroy();
   SVGImageElementBase::DestroyContent();

dom/svg/SVGImageElement.h

@@ -62,8 +62,6 @@ class SVGImageElement final : public SVGImageElementBase,
   nsresult BindToTree(BindContext&, nsINode& aParent) override;
   void UnbindFromTree(bool aNullParent) override;
 
-  ElementState IntrinsicState() const override;
-
   void DestroyContent() override;
 
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* name) const override;

layout/base/RestyleManager.cpp

@@ -35,6 +35,7 @@
 #include "mozilla/dom/DocumentInlines.h"
 #include "mozilla/dom/ElementInlines.h"
 #include "mozilla/dom/HTMLBodyElement.h"
+#include "mozilla/dom/HTMLInputElement.h"
 
 #include "ScrollSnap.h"
 #include "nsAnimationManager.h"
@@ -505,31 +506,36 @@ static bool StateChangeMayAffectFrame(const Element& aElement,
                                       const nsIFrame& aFrame,
                                       ElementState aStates) {
   const bool brokenChanged = aStates.HasState(ElementState::BROKEN);
-  if (aFrame.IsGeneratedContentFrame()) {
-    if (aElement.IsHTMLElement(nsGkAtoms::mozgeneratedcontentimage)) {
-      return brokenChanged;
-    }
-    // If it's other generated content, ignore LOADING/etc state changes on it.
-    return false;
-  }
-
   if (!brokenChanged) {
     return false;
   }
 
+  if (aFrame.IsGeneratedContentFrame()) {
+    // If it's other generated content, ignore state changes on it.
+    return aElement.IsHTMLElement(nsGkAtoms::mozgeneratedcontentimage);
+  }
+
+  if (aElement.IsAnyOfHTMLElements(nsGkAtoms::object, nsGkAtoms::embed)) {
+    // Broken affects object fallback behavior.
+    return true;
+  }
+
+  const bool mightChange = [&] {
     if (aElement.IsHTMLElement(nsGkAtoms::img)) {
+      return true;
+    }
+    const auto* input = HTMLInputElement::FromNode(aElement);
+    return input && input->ControlType() == FormControlType::InputImage;
+  }();
+
+  if (!mightChange) {
+    return false;
+  }
+
   const bool needsImageFrame =
       nsImageFrame::ImageFrameTypeFor(aElement, *aFrame.Style()) !=
       nsImageFrame::ImageFrameType::None;
   return needsImageFrame != aFrame.IsImageFrameOrSubclass();
-  }
-
-  if (aElement.IsSVGElement(nsGkAtoms::image)) {
-    // <image> gets an SVGImageFrame all the time.
-    return false;
-  }
-
-  return brokenChanged;
 }
 
 /**

layout/base/nsCSSFrameConstructor.cpp

@@ -3642,20 +3642,19 @@ static nsIFrame* NS_NewSubDocumentOrImageFrame(mozilla::PresShell* aPresShell,
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindObjectData(const Element& aElement,
                                       ComputedStyle& aStyle) {
-  // GetDisplayedType isn't necessarily nsIObjectLoadingContent::TYPE_NULL for
-  // cases when the object is broken/suppressed/etc (e.g. a broken image), but
-  // we want to treat those cases as TYPE_NULL
   uint32_t type;
-  if (aElement.State().HasState(ElementState::BROKEN)) {
-    type = nsIObjectLoadingContent::TYPE_NULL;
-  } else {
   nsCOMPtr<nsIObjectLoadingContent> objContent =
       do_QueryInterface(const_cast<Element*>(&aElement));
   NS_ASSERTION(objContent,
                "embed and object must implement "
                "nsIObjectLoadingContent!");
-
   objContent->GetDisplayedType(&type);
+  if (type == nsIObjectLoadingContent::TYPE_IMAGE &&
+      aElement.State().HasState(ElementState::BROKEN)) {
+    // GetDisplayedType isn't necessarily nsIObjectLoadingContent::TYPE_NULL for
+    // cases when the object is broken/suppressed/etc (e.g. a broken image), but
+    // we want to treat those cases as TYPE_NULL
+    type = nsIObjectLoadingContent::TYPE_NULL;
   }
 
   if (type == nsIObjectLoadingContent::TYPE_FALLBACK &&