Bug 1037643 - Part 2 - Line up with new spec for <img srcset> and <picture> mutations. r=bz

content/html/content/src/HTMLImageElement.cpp

@@ -16,6 +16,8 @@
 #include "nsIURL.h"
 #include "nsIIOService.h"
 #include "nsIServiceManager.h"
+#include "nsIAppShell.h"
+#include "nsWidgetsCID.h"
 #include "nsNetUtil.h"
 #include "nsContentUtils.h"
 #include "nsContainerFrame.h"
@@ -49,6 +51,8 @@
 #include "mozilla/Preferences.h"
 static const char *kPrefSrcsetEnabled = "dom.image.srcset.enabled";
 
+static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);
+
 NS_IMPL_NS_NEW_HTML_ELEMENT(Image)
 
 // Is aSubject a previous sibling of aNode.
@@ -69,6 +73,30 @@ static bool IsPreviousSibling(nsINode *aSubject, nsINode *aNode)
 namespace mozilla {
 namespace dom {
 
+// Calls LoadSelectedImage on host element unless it has been superseded or
+// canceled -- this is the synchronous section of "update the image data".
+// https://html.spec.whatwg.org/multipage/embedded-content.html#update-the-image-data
+class ImageLoadTask : public nsRunnable
+{
+public:
+  explicit ImageLoadTask(HTMLImageElement *aElement) :
+    mElement(aElement)
+  {}
+
+  NS_IMETHOD Run()
+  {
+    if (mElement->mPendingImageLoadTask == this) {
+      mElement->mPendingImageLoadTask = nullptr;
+      mElement->LoadSelectedImage(true, true);
+    }
+    return NS_OK;
+  }
+
+private:
+  ~ImageLoadTask() {}
+  nsRefPtr<HTMLImageElement> mElement;
+};
+
 HTMLImageElement::HTMLImageElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
   : nsGenericHTMLElement(aNodeInfo)
   , mForm(nullptr)
@@ -367,39 +395,47 @@ HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
   // from the parser or some such place; we'll get bound after all the
   // attributes have been set, so we'll do the image load from BindToTree.
 
-  nsCOMPtr<nsIContent> thisContent = AsContent();
   nsAttrValueOrString attrVal(aValue);
 
   if (aName == nsGkAtoms::src &&
-      aNameSpaceID == kNameSpaceID_None) {
+      aNameSpaceID == kNameSpaceID_None &&
-    // SetAttr handles setting src in the non-responsive case, so only handle it
+      !aValue) {
-    // for responsive mode or unsetting
+    // SetAttr handles setting src since it needs to catch img.src =
-    if (!aValue) {
+    // img.src, so we only need to handle the unset case
+    if (mResponsiveSelector) {
+      if (mResponsiveSelector->Content() == this) {
+        mResponsiveSelector->SetDefaultSource(nullptr);
+      }
+      QueueImageLoadTask();
+    } else {
+      // Bug 1076583 - We still behave synchronously in the non-responsive case
       CancelImageRequests(aNotify);
-    } else if (mResponsiveSelector) {
-      mResponsiveSelector->SetDefaultSource(attrVal.String());
-      LoadSelectedImage(false, aNotify);
     }
   } else if (aName == nsGkAtoms::srcset &&
              aNameSpaceID == kNameSpaceID_None &&
-             aNotify &&
-             AsContent()->IsInDoc() &&
              IsSrcsetEnabled()) {
-    // We currently don't handle responsive mode until BindToTree
+    PictureSourceSrcsetChanged(this, attrVal.String(), aNotify);
-    PictureSourceSrcsetChanged(thisContent, attrVal.String(), aNotify);
   } else if (aName == nsGkAtoms::sizes &&
              aNameSpaceID == kNameSpaceID_None &&
-             thisContent->IsInDoc() &&
              HTMLPictureElement::IsPictureEnabled()) {
-    PictureSourceSizesChanged(thisContent, attrVal.String(), aNotify);
+    PictureSourceSizesChanged(this, attrVal.String(), aNotify);
   } else if (aName == nsGkAtoms::crossorigin &&
              aNameSpaceID == kNameSpaceID_None &&
              aNotify) {
-    // We want aForce == true in this LoadImage call, because we want to force
+    // Force a new load of the image with the new cross origin policy.
-    // a new load of the image with the new cross origin policy.
+    if (mResponsiveSelector) {
-    nsCOMPtr<nsIURI> currentURI;
+      // per spec, full selection runs when this changes, even though
-    if (NS_SUCCEEDED(GetCurrentURI(getter_AddRefs(currentURI))) && currentURI) {
+      // it doesn't directly affect the source selection
-      LoadImage(currentURI, true, aNotify);
+      QueueImageLoadTask();
+    } else {
+      // Bug 1076583 - We still use the older synchronous algorithm in
+      // non-responsive mode. We want aForce == true in this LoadImage
+      // call, because we want to force a new load of the image with
+      // the new cross origin policy.
+      nsCOMPtr<nsIURI> currentURI;
+      if (NS_SUCCEEDED(GetCurrentURI(getter_AddRefs(currentURI))) && currentURI) {
+        LoadImage(currentURI, true, aNotify);
+      }
     }
   }
 
@@ -456,7 +492,7 @@ HTMLImageElement::IsHTMLFocusable(bool aWithMouse,
     *aTabIndex = (sTabFocusModel & eTabFocus_formElementsMask)? tabIndex : -1;
   }
 
-  *aIsFocusable = 
+  *aIsFocusable =
 #ifdef XP_MACOSX
     (!aWithMouse || nsFocusManager::sMouseFocusesFormControl) &&
 #endif
@@ -475,32 +511,45 @@ HTMLImageElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
   // being set to its existing value, which is normally optimized away as a
   // no-op.
   //
-  // If aNotify is false, we are coming from the parser or some such place;
+  // If we are in responsive mode, we drop the forced reload behavior,
-  // we'll get bound after all the attributes have been set, so we'll do the
+  // but still trigger a image load task for img.src = img.src per
-  // image load from BindToTree. Skip the LoadImage call in that case.
+  // spec.
   //
-  // If we are in responsive mode, we drop the forced reload behavior, and
+  // Both cases handle unsetting src in AfterSetAttr
-  // handle updates in AfterSetAttr
+  if (aNameSpaceID == kNameSpaceID_None &&
-  if (aNotify && !mResponsiveSelector &&
-      aNameSpaceID == kNameSpaceID_None &&
       aName == nsGkAtoms::src) {
 
-    // Prevent setting image.src by exiting early
+    // This is for dom.disable_image_src_set, which predates "srcset"
+    // as an attribute. See Bug 773429
     if (nsContentUtils::IsImageSrcSetDisabled()) {
       return NS_OK;
     }
 
-    // A hack to get animations to reset. See bug 594771.
+    if (mResponsiveSelector || mPendingImageLoadTask) {
-    mNewRequestsWillNeedAnimationReset = true;
+      if (mResponsiveSelector &&
+          mResponsiveSelector->Content() == this) {
+        mResponsiveSelector->SetDefaultSource(aValue);
+      }
+      QueueImageLoadTask();
+    } else if (aNotify) {
+      // 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
+      // sync image load from BindToTree. Skip the LoadImage call in that case.
 
-    // Force image loading here, so that we'll try to load the image from
+      // Note that this sync behavior is partially removed from the spec, bug 1076583
-    // network if it's set to be not cacheable...  If we change things so that
-    // the state gets in Element's attr-setting happen around this
-    // LoadImage call, we could start passing false instead of aNotify
-    // here.
-    LoadImage(aValue, true, aNotify);
 
-    mNewRequestsWillNeedAnimationReset = false;
+      // A hack to get animations to reset. See bug 594771.
+      mNewRequestsWillNeedAnimationReset = true;
+
+      // Force image loading here, so that we'll try to load the image from
+      // network if it's set to be not cacheable...  If we change things so that
+      // the state gets in Element's attr-setting happen around this
+      // LoadImage call, we could start passing false instead of aNotify
+      // here.
+      LoadImage(aValue, true, aNotify);
+
+      mNewRequestsWillNeedAnimationReset = false;
+    }
   }
 
   return nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue,
@@ -526,27 +575,31 @@ HTMLImageElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
 
   bool addedToPicture = aParent && aParent->Tag() == nsGkAtoms::picture &&
                         HTMLPictureElement::IsPictureEnabled();
-  bool haveSrcset = IsSrcsetEnabled() &&
+  if (addedToPicture) {
-                    HasAttr(kNameSpaceID_None, nsGkAtoms::srcset);
+    QueueImageLoadTask();
-  if (addedToPicture || haveSrcset ||
+  } else if (!mResponsiveSelector &&
-      HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
+             HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
+    // We skip loading when our attributes were set from parser land,
+    // so trigger a aForce=false load now to check if things changed.
+    // This isn't necessary for responsive mode, since creating the
+    // image load task is asynchronous we don't need to take special
+    // care to avoid doing so when being filled by the parser.
+
     // FIXME: Bug 660963 it would be nice if we could just have
     // ClearBrokenState update our state and do it fast...
     ClearBrokenState();
     RemoveStatesSilently(NS_EVENT_STATE_BROKEN);
 
-    // We don't handle responsive changes when not bound to a tree, update them
+    // We still act synchronously for the non-responsive case (Bug
-    // now if necessary
+    // 1076583), but still need to delay if it is unsafe to run
-    if (addedToPicture || haveSrcset) {
+    // script.
-      MaybeUpdateResponsiveSelector();
-    }
 
     // If loading is temporarily disabled, don't even launch MaybeLoadImage.
     // Otherwise MaybeLoadImage may run later when someone has reenabled
     // loading.
     if (LoadingEnabled()) {
       nsContentUtils::AddScriptRunner(
-        NS_NewRunnableMethod(this, &HTMLImageElement::MaybeLoadImage));
+          NS_NewRunnableMethod(this, &HTMLImageElement::MaybeLoadImage));
     }
   }
 
@@ -564,7 +617,13 @@ HTMLImageElement::UnbindFromTree(bool aDeep, bool aNullParent)
     }
   }
 
-  mResponsiveSelector = nullptr;
+  if (aNullParent &&
+      nsINode::GetParentNode()->Tag() == nsGkAtoms::picture &&
+      HTMLPictureElement::IsPictureEnabled()) {
+    // Being removed from picture re-triggers selection, even if we
+    // weren't using a <source> peer
+    QueueImageLoadTask();
+  }
 
   nsImageLoadingContent::UnbindFromTree(aDeep, aNullParent);
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
@@ -606,9 +665,9 @@ HTMLImageElement::MaybeLoadImage()
 
   // Note, check LoadingEnabled() after LoadImage call.
 
-  nsresult rv = LoadSelectedImage(false, true);
+  LoadSelectedImage(false, true);
 
-  if (NS_FAILED(rv) || !LoadingEnabled()) {
+  if (!LoadingEnabled()) {
     CancelImageRequests(true);
   }
 }
@@ -785,19 +844,41 @@ HTMLImageElement::ClearForm(bool aRemoveFromForm)
   mForm = nullptr;
 }
 
+void
+HTMLImageElement::QueueImageLoadTask()
+{
+  // If loading is temporarily disabled, we don't want to queue tasks
+  // that may then run when loading is re-enabled.
+  if (!LoadingEnabled() || !this->OwnerDoc()->IsCurrentActiveDocument()) {
+    return;
+  }
+
+  // The task checks this to determine if it was the last queued event, so this
+  // implicitly cancels earlier tasks
+  mPendingImageLoadTask = new ImageLoadTask(this);
+  nsCOMPtr<nsIAppShell> appShell = do_GetService(kAppShellCID);
+  if (appShell) {
+    appShell->RunInStableState(mPendingImageLoadTask);
+  } else {
+    MOZ_ASSERT(false, "expect appshell for HTMLImageElement");
+  }
+}
+
 nsresult
 HTMLImageElement::LoadSelectedImage(bool aForce, bool aNotify)
 {
   nsresult rv = NS_ERROR_FAILURE;
 
+  if (aForce) {
+    // In responsive mode we generally want to re-run the full
+    // selection algorithm whenever starting a new load, per
+    // spec. This also causes us to re-resolve the URI as appropriate.
+    UpdateResponsiveSource();
+  }
+
   if (mResponsiveSelector) {
     nsCOMPtr<nsIURI> url = mResponsiveSelector->GetSelectedImageURL();
-    if (url) {
+    rv = LoadImage(url, aForce, aNotify);
-      rv = LoadImage(url, aForce, aNotify);
-    } else {
-      CancelImageRequests(aNotify);
-      rv = NS_OK;
-    }
   } else {
     nsAutoString src;
     if (!GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
@@ -805,12 +886,12 @@ HTMLImageElement::LoadSelectedImage(bool aForce, bool aNotify)
       rv = NS_OK;
     } else {
       rv = LoadImage(src, aForce, aNotify);
-      if (NS_FAILED(rv)) {
-        CancelImageRequests(aNotify);
-      }
     }
   }
 
+  if (NS_FAILED(rv)) {
+    CancelImageRequests(aNotify);
+  }
   return rv;
 }
 
@@ -819,31 +900,28 @@ HTMLImageElement::PictureSourceSrcsetChanged(nsIContent *aSourceNode,
                                              const nsAString& aNewValue,
                                              bool aNotify)
 {
-  if (aSourceNode != AsContent() && !HTMLPictureElement::IsPictureEnabled()) {
+  bool isSelf = aSourceNode == this;
-    // Don't consider <source> nodes if picture is pref'd off
+
+  if (!IsSrcsetEnabled() ||
+      (!isSelf && !HTMLPictureElement::IsPictureEnabled())) {
     return;
   }
 
-  nsIContent *currentSrc = mResponsiveSelector ? mResponsiveSelector->Content()
+  MOZ_ASSERT(isSelf || IsPreviousSibling(aSourceNode, this),
-                                               : nullptr;
+             "Should not be getting notifications for non-previous-siblings");
+
+  nsIContent *currentSrc =
+    mResponsiveSelector ? mResponsiveSelector->Content() : nullptr;
 
   if (aSourceNode == currentSrc) {
-    // We're currently using this node as our responsive selector source.
+    // We're currently using this node as our responsive selector
+    // source.
     mResponsiveSelector->SetCandidatesFromSourceSet(aNewValue);
-    // Search for a new source if we are no longer valid.
-    MaybeUpdateResponsiveSelector(currentSrc);
-
-    LoadSelectedImage(false, aNotify);
-  } else if (currentSrc && IsPreviousSibling(currentSrc, aSourceNode)) {
-      // If we have a source and it is previous to the one being updated, ignore
-      return;
-  } else {
-    // This is previous to our current source or we don't have a current source,
-    // use it if valid.
-    if (TryCreateResponsiveSelector(aSourceNode, &aNewValue, nullptr)) {
-      LoadSelectedImage(false, aNotify);
-    }
   }
+
+  // This always triggers the image update steps per the spec, even if
+  // we are not using this source.
+  QueueImageLoadTask();
 }
 
 void
@@ -852,109 +930,119 @@ HTMLImageElement::PictureSourceSizesChanged(nsIContent *aSourceNode,
                                             bool aNotify)
 {
   if (!HTMLPictureElement::IsPictureEnabled()) {
-    // Don't consider sizes at all if picture support is disabled
     return;
   }
 
-  nsIContent *currentSrc = mResponsiveSelector ? mResponsiveSelector->Content()
+  MOZ_ASSERT(aSourceNode == this ||
-                                               : nullptr;
+             IsPreviousSibling(aSourceNode, this),
+             "Should not be getting notifications for non-previous-siblings");
+
+  nsIContent *currentSrc =
+    mResponsiveSelector ? mResponsiveSelector->Content() : nullptr;
 
   if (aSourceNode == currentSrc) {
-    // We're currently using this node as our responsive selector source.
+    // We're currently using this node as our responsive selector
+    // source.
     mResponsiveSelector->SetSizesFromDescriptor(aNewValue);
-    LoadSelectedImage(false, aNotify);
   }
+
+  // This always triggers the image update steps per the spec, even if
+  // we are not using this source.
+  QueueImageLoadTask();
+}
+
+void
+HTMLImageElement::PictureSourceMediaChanged(nsIContent *aSourceNode,
+                                            const nsAString& aNewValue,
+                                            bool aNotify)
+{
+  if (!HTMLPictureElement::IsPictureEnabled()) {
+    return;
+  }
+
+  MOZ_ASSERT(IsPreviousSibling(aSourceNode, this),
+             "Should not be getting notifications for non-previous-siblings");
+
+  // This always triggers the image update steps per the spec, even if
+  // we are not switching to/from this source
+  QueueImageLoadTask();
 }
 
 void
 HTMLImageElement::PictureSourceAdded(nsIContent *aSourceNode)
 {
-  // If the source node is previous to our current one, or ourselves if we have
+  if (!HTMLPictureElement::IsPictureEnabled()) {
-  // no responsive source, try to use it as a responsive source.
+    return;
-  nsIContent *currentSrc = mResponsiveSelector ? mResponsiveSelector->Content()
-                                               : AsContent();
-
-  if (HTMLPictureElement::IsPictureEnabled() &&
-      IsPreviousSibling(aSourceNode, currentSrc) &&
-      TryCreateResponsiveSelector(aSourceNode, nullptr, nullptr)) {
-    LoadSelectedImage(false, true);
   }
+
+  QueueImageLoadTask();
 }
 
 void
 HTMLImageElement::PictureSourceRemoved(nsIContent *aSourceNode)
 {
-  // If this is our current source, we'll need to find another one or leave
+  if (!HTMLPictureElement::IsPictureEnabled()) {
-  // responsive mode.
+    return;
-  if (mResponsiveSelector && mResponsiveSelector->Content() == aSourceNode) {
-    MaybeUpdateResponsiveSelector(aSourceNode, true);
-    LoadSelectedImage(false, true);
   }
+
+  QueueImageLoadTask();
 }
 
-bool
+void
-HTMLImageElement::MaybeUpdateResponsiveSelector(nsIContent *aCurrentSource,
+HTMLImageElement::UpdateResponsiveSource()
-                                                bool aSourceRemoved)
 {
-  nsIContent *thisContent = AsContent();
-
-  if (!aCurrentSource && mResponsiveSelector) {
-    aCurrentSource = mResponsiveSelector->Content();
-  }
-
-  // If we have a source with candidates, no update is needed unless it is being
-  // removed
-  if (aCurrentSource && !aSourceRemoved &&
-      mResponsiveSelector->NumCandidates()) {
-    return false;
-  }
-
-  // Otherwise, invalidate
-  bool hadSelector = !!mResponsiveSelector;
-  mResponsiveSelector = nullptr;
-
   if (!IsSrcsetEnabled()) {
-    return hadSelector;
+    mResponsiveSelector = nullptr;
+    return;
   }
 
-  // See if there's another source node we could use.
+  nsIContent *currentSource =
+    mResponsiveSelector ? mResponsiveSelector->Content() : nullptr;
   bool pictureEnabled = HTMLPictureElement::IsPictureEnabled();
-  nsIContent *nextSource = nullptr;
+  nsINode *parent = pictureEnabled ? this->nsINode::GetParentNode() : nullptr;
-  if (pictureEnabled && aCurrentSource && aCurrentSource != thisContent) {
+
-    // If current source is the <img> tag, there is no next candidate. Otherwise,
+  nsINode *candidateSource = nullptr;
-    // it's the next sibling of the current source.
+  if (parent && parent->Tag() == nsGkAtoms::picture) {
-    MOZ_ASSERT(IsPreviousSibling(aCurrentSource, thisContent) &&
+    // Walk source nodes previous to ourselves
-               thisContent->GetParentNode()->Tag() == nsGkAtoms::picture);
+    candidateSource = parent->GetFirstChild();
-    nextSource = aCurrentSource->GetNextSibling();
+  } else {
-  } else if (!aCurrentSource) {
+    candidateSource = this;
-    // If no current source at all, start from the first possible source, which
-    // is the first node of the <picture> element or ourselves if we're not a
-    // picture
-    nsINode *parent = pictureEnabled ? thisContent->GetParentNode() : nullptr;
-    if (parent && parent->Tag() == nsGkAtoms::picture) {
-      nextSource = parent->GetFirstChild();
-    } else {
-      nextSource = thisContent;
-    }
   }
 
-  while (nextSource) {
+  while (candidateSource) {
-    if (nextSource == thisContent) {
+    if (candidateSource == currentSource) {
-      // We are the last possible source, so stop searching if we match or
+      // found no better source before current, re-run selection on
-      // not
+      // that and keep it if it's still usable.
-      TryCreateResponsiveSelector(nextSource);
+      mResponsiveSelector->SelectImage(true);
+      if (mResponsiveSelector->NumCandidates()) {
+        break;
+      }
+
+      // no longer valid
+      mResponsiveSelector = nullptr;
+      if (candidateSource == this) {
+        // No further possibilities
+        break;
+      }
+    } else if (candidateSource == this) {
+      // We are the last possible source
+      if (!TryCreateResponsiveSelector(candidateSource->AsContent())) {
+        // Failed to find any source
+        mResponsiveSelector = nullptr;
+      }
       break;
-    } else if (nextSource->Tag() == nsGkAtoms::source &&
+    } else if (candidateSource->Tag() == nsGkAtoms::source &&
-               TryCreateResponsiveSelector(nextSource)) {
+               TryCreateResponsiveSelector(candidateSource->AsContent())) {
-      // If this led to a valid source, stop
+      // This led to a valid source, stop
       break;
     }
-
+    candidateSource = candidateSource->GetNextSibling();
-    nextSource = nextSource->GetNextSibling();
   }
 
-  // State changed unless we didn't make a selector and didn't start with one
+  if (!candidateSource) {
-  return mResponsiveSelector || hadSelector;
+    // Ran out of siblings without finding ourself, e.g. XBL magic.
+    mResponsiveSelector = nullptr;
+  }
 }
 
 bool
@@ -972,7 +1060,7 @@ HTMLImageElement::TryCreateResponsiveSelector(nsIContent *aSourceNode,
   if (isSourceTag) {
     DebugOnly<nsINode *> parent(nsINode::GetParentNode());
     MOZ_ASSERT(parent && parent->Tag() == nsGkAtoms::picture);
-    MOZ_ASSERT(IsPreviousSibling(aSourceNode, AsContent()));
+    MOZ_ASSERT(IsPreviousSibling(aSourceNode, this));
     MOZ_ASSERT(pictureEnabled);
 
     HTMLSourceElement *src = static_cast<HTMLSourceElement*>(aSourceNode);
@@ -981,7 +1069,7 @@ HTMLImageElement::TryCreateResponsiveSelector(nsIContent *aSourceNode,
     }
   } else if (aSourceNode->Tag() == nsGkAtoms::img) {
     // Otherwise this is the <img> tag itself
-    MOZ_ASSERT(aSourceNode == AsContent());
+    MOZ_ASSERT(aSourceNode == this);
   }
 
   // Skip if has no srcset or an empty srcset
@@ -999,7 +1087,7 @@ HTMLImageElement::TryCreateResponsiveSelector(nsIContent *aSourceNode,
 
 
   // Try to parse
-  nsRefPtr<ResponsiveImageSelector> sel = new ResponsiveImageSelector(this);
+  nsRefPtr<ResponsiveImageSelector> sel = new ResponsiveImageSelector(aSourceNode);
   if (!sel->SetCandidatesFromSourceSet(srcset)) {
     // No possible candidates, don't need to bother parsing sizes
     return false;
@@ -1015,7 +1103,7 @@ HTMLImageElement::TryCreateResponsiveSelector(nsIContent *aSourceNode,
 
   // If this is the <img> tag, also pull in src as the default source
   if (!isSourceTag) {
-    MOZ_ASSERT(aSourceNode == AsContent());
+    MOZ_ASSERT(aSourceNode == this);
     nsAutoString src;
     if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src) && !src.IsEmpty()) {
       sel->SetDefaultSource(src);

content/html/content/src/HTMLImageElement.h

@@ -26,6 +26,8 @@ class HTMLImageElement MOZ_FINAL : public nsGenericHTMLElement,
                                    public nsIDOMHTMLImageElement
 {
   friend class HTMLSourceElement;
+  friend class HTMLPictureElement;
+  friend class ImageLoadTask;
 public:
   explicit HTMLImageElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo);
 
@@ -43,6 +45,8 @@ public:
   // nsIDOMHTMLImageElement
   NS_DECL_NSIDOMHTMLIMAGEELEMENT
 
+  NS_IMPL_FROMCONTENT_HTML_WITH_TAG(HTMLImageElement, img)
+
   // override from nsImageLoadingContent
   CORSMode GetCORSMode();
 
@@ -197,6 +201,9 @@ public:
 protected:
   virtual ~HTMLImageElement();
 
+  // Queues a task to run LoadSelectedImage pending stable state
+  void QueueImageLoadTask();
+
   // Resolve and load the current mResponsiveSelector (responsive mode) or src
   // attr image.
   nsresult LoadSelectedImage(bool aForce, bool aNotify);
@@ -206,13 +213,25 @@ protected:
                                   const nsAString& aNewValue, bool aNotify);
   void PictureSourceSizesChanged(nsIContent *aSourceNode,
                                  const nsAString& aNewValue, bool aNotify);
+  void PictureSourceMediaChanged(nsIContent *aSourceNode,
+                                 const nsAString& aNewValue, bool aNotify);
 
   void PictureSourceAdded(nsIContent *aSourceNode);
   // This should be called prior to the unbind, such that nextsibling works
   void PictureSourceRemoved(nsIContent *aSourceNode);
 
-  bool MaybeUpdateResponsiveSelector(nsIContent *aCurrentSource = nullptr,
+  // Re-evaluates all source nodes (picture <source>,<img>) and finds
-                                     bool aSourceRemoved = false);
+  // the best source set for mResponsiveSelector. If a better source
+  // is found, creates a new selector and feeds the source to it. If
+  // the current ResponsiveSelector is not changed, runs
+  // SelectImage(true) to re-evaluate its candidates.
+  //
+  // Because keeping the existing selector is the common case (and we
+  // often do no-op reselections), this does not re-parse values for
+  // the existing mResponsiveSelector, meaning you need to update its
+  // parameters as appropriate before calling (or null it out to force
+  // recreation)
+  void UpdateResponsiveSource();
 
   // Given a <source> node that is a previous sibling *or* ourselves, try to
   // create a ResponsiveSelector.
@@ -247,6 +266,8 @@ protected:
 private:
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     nsRuleData* aData);
+
+  nsCOMPtr<nsIRunnable> mPendingImageLoadTask;
 };
 
 } // namespace dom

content/html/content/src/HTMLPictureElement.cpp

@@ -40,6 +40,28 @@ NS_IMPL_ISUPPORTS_INHERITED(HTMLPictureElement, nsGenericHTMLElement,
 
 NS_IMPL_ELEMENT_CLONE(HTMLPictureElement)
 
+void
+HTMLPictureElement::RemoveChildAt(uint32_t aIndex, bool aNotify)
+{
+  // Find all img siblings after this <source> to notify them of its demise
+  nsCOMPtr<nsINode> child = GetChildAt(aIndex);
+  nsCOMPtr<nsIContent> nextSibling;
+  if (child && child->Tag() == nsGkAtoms::source) {
+    nextSibling = child->GetNextSibling();
+  }
+
+  nsGenericHTMLElement::RemoveChildAt(aIndex, aNotify);
+
+  if (nextSibling && nextSibling->GetParentNode() == this) {
+    do {
+      HTMLImageElement* img = HTMLImageElement::FromContent(nextSibling);
+      if (img) {
+        img->PictureSourceRemoved(child->AsContent());
+      }
+    } while ( (nextSibling = nextSibling->GetNextSibling()) );
+  }
+}
+
 bool
 HTMLPictureElement::IsPictureEnabled()
 {

content/html/content/src/HTMLPictureElement.h

@@ -29,6 +29,7 @@ public:
   NS_DECL_NSIDOMHTMLPICTUREELEMENT
 
   virtual nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult) const MOZ_OVERRIDE;
+  virtual void RemoveChildAt(uint32_t aIndex, bool aNotify) MOZ_OVERRIDE;
 
   static bool IsPictureEnabled();
 

content/html/content/src/HTMLSourceElement.cpp

@@ -63,9 +63,10 @@ HTMLSourceElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
   // responsive parameter changes
   nsINode *parent = nsINode::GetParentNode();
   if (aNameSpaceID == kNameSpaceID_None &&
-      (aName == nsGkAtoms::srcset || aName == nsGkAtoms::sizes) &&
+      (aName == nsGkAtoms::srcset ||
-      parent && parent->Tag() == nsGkAtoms::picture && MatchesCurrentMedia()) {
+       aName == nsGkAtoms::sizes ||
-
+       aName == nsGkAtoms::media) &&
+      parent && parent->Tag() == nsGkAtoms::picture) {
     nsString strVal = aValue ? aValue->GetStringValue() : EmptyString();
     // Find all img siblings after this <source> and notify them of the change
     nsCOMPtr<nsINode> sibling = AsContent();
@@ -76,6 +77,8 @@ HTMLSourceElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
           img->PictureSourceSrcsetChanged(AsContent(), strVal, aNotify);
         } else if (aName == nsGkAtoms::sizes) {
           img->PictureSourceSizesChanged(AsContent(), strVal, aNotify);
+        } else if (aName == nsGkAtoms::media) {
+          img->PictureSourceMediaChanged(AsContent(), strVal, aNotify);
         }
       }
     }
@@ -137,24 +140,6 @@ HTMLSourceElement::BindToTree(nsIDocument *aDocument,
   return NS_OK;
 }
 
-void
-HTMLSourceElement::UnbindFromTree(bool aDeep, bool aNullParent)
-{
-  nsINode *parent = nsINode::GetParentNode();
-  if (parent && parent->Tag() == nsGkAtoms::picture) {
-    // Find all img siblings after this <source> and notify them of our demise
-    nsCOMPtr<nsINode> sibling = AsContent();
-    while ( (sibling = sibling->GetNextSibling()) ) {
-      if (sibling->Tag() == nsGkAtoms::img) {
-        HTMLImageElement *img = static_cast<HTMLImageElement*>(sibling.get());
-        img->PictureSourceRemoved(AsContent());
-      }
-    }
-  }
-
-  nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
-}
-
 JSObject*
 HTMLSourceElement::WrapNode(JSContext* aCx)
 {

content/html/content/src/HTMLSourceElement.h

@@ -39,7 +39,6 @@ public:
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers) MOZ_OVERRIDE;
-  virtual void UnbindFromTree(bool aDeep, bool aNullParent) MOZ_OVERRIDE;
 
   // If this element's media attr matches for its owner document.  Returns true
   // if no media attr was set.