Bug 1709577 - Check list of available images before deciding to defer a lazy load. r=edgar

As per https://html.spec.whatwg.org/#updating-the-image-data step 6.

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

dom/base/DOMIntersectionObserver.cpp

@@ -152,7 +152,8 @@ static void LazyLoadCallback(
     MOZ_ASSERT(entry->Target()->IsHTMLElement(nsGkAtoms::img));
     if (entry->IsIntersecting()) {
       static_cast<HTMLImageElement*>(entry->Target())
-          ->StopLazyLoadingAndStartLoadIfNeeded(true);
+          ->StopLazyLoading(HTMLImageElement::FromIntersectionObserver::Yes,
+                            HTMLImageElement::StartLoading::Yes);
     }
   }
 }

dom/base/nsContentUtils.cpp

@@ -9717,6 +9717,19 @@ void nsContentUtils::AppendNativeAnonymousChildren(const nsIContent* aContent,
   }
 }
 
+bool nsContentUtils::IsImageAvailable(nsIContent* aLoadingNode, nsIURI* aURI,
+                                      nsIPrincipal* aDefaultTriggeringPrincipal,
+                                      CORSMode aCORSMode) {
+  nsCOMPtr<nsIPrincipal> triggeringPrincipal;
+  QueryTriggeringPrincipal(aLoadingNode, aDefaultTriggeringPrincipal,
+                           getter_AddRefs(triggeringPrincipal));
+  MOZ_ASSERT(triggeringPrincipal);
+
+  Document* doc = aLoadingNode->OwnerDoc();
+  imgLoader* imgLoader = GetImgLoaderForDocument(doc);
+  return imgLoader->IsImageAvailable(aURI, triggeringPrincipal, aCORSMode, doc);
+}
+
 /* static */
 bool nsContentUtils::QueryTriggeringPrincipal(
     nsIContent* aLoadingNode, nsIPrincipal* aDefaultPrincipal,

dom/base/nsContentUtils.h

@@ -3085,6 +3085,14 @@ class nsContentUtils {
                                     aTriggeringPrincipal);
   }
 
+  // Returns whether the image for the given URI and triggering principal is
+  // already available. Ideally this should exactly match the "list of available
+  // images" in the HTML spec, but our implementation of that at best only
+  // resembles it.
+  static bool IsImageAvailable(nsIContent*, nsIURI*,
+                               nsIPrincipal* aDefaultTriggeringPrincipal,
+                               mozilla::CORSMode);
+
   /**
    * Returns the content policy type that should be used for loading images
    * for displaying in the UI.  The sources of such images can be <xul:image>,

dom/html/HTMLImageElement.cpp

@@ -88,15 +88,7 @@ class ImageLoadTask final : public MicroTaskRunnable {
     if (mElement->mPendingImageLoadTask == this) {
       mElement->mPendingImageLoadTask = nullptr;
       mElement->mUseUrgentStartForChannel = mUseUrgentStartForChannel;
-      // Defer loading this image if loading="lazy" was set after this microtask
-      // was queued.
-      // NOTE: Using ShouldLoadImage() will violate the HTML standard spec
-      // because ShouldLoadImage() checks the document active state which should
-      // have done just once before this queue is created as per the spec, so
-      // we just check the lazy loading state here.
-      if (!mElement->IsLazyLoading()) {
-        mElement->LoadSelectedImage(true, true, mAlwaysLoad);
-      }
+      mElement->LoadSelectedImage(true, true, mAlwaysLoad);
     }
     mDocument->UnblockOnload(false);
   }
@@ -342,17 +334,13 @@ nsresult HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
 
   bool forceReload = false;
 
-  if (aName == nsGkAtoms::loading) {
-    if (aValue &&
-        static_cast<HTMLImageElement::Loading>(aValue->GetEnumValue()) ==
-            Loading::Lazy &&
-        !ImageState().HasState(NS_EVENT_STATE_LOADING)) {
+  if (aName == nsGkAtoms::loading &&
+      !ImageState().HasState(NS_EVENT_STATE_LOADING)) {
+    if (aValue && Loading(aValue->GetEnumValue()) == Loading::Lazy) {
       SetLazyLoading();
     } else if (aOldValue &&
-               static_cast<HTMLImageElement::Loading>(
-                   aOldValue->GetEnumValue()) == Loading::Lazy &&
-               !ImageState().HasState(NS_EVENT_STATE_LOADING)) {
-      StopLazyLoadingAndStartLoadIfNeeded(false);
+               Loading(aOldValue->GetEnumValue()) == Loading::Lazy) {
+      StopLazyLoading(FromIntersectionObserver::No, StartLoading::Yes);
     }
   } else if (aName == nsGkAtoms::src && !aValue) {
     // NOTE: regular src value changes are handled in AfterMaybeChangeAttr, so
@@ -489,8 +477,8 @@ void HTMLImageElement::AfterMaybeChangeAttr(
     // when aNotify is true, and 2) When this function is called by
     // OnAttrSetButNotChanged, SetAttrAndNotify will not subsequently call
     // UpdateState.
-    LoadImage(aValue.String(), true, aNotify, eImageLoadType_Normal,
-              mSrcTriggeringPrincipal);
+    LoadSelectedImage(/* aForce = */ true, aNotify,
+                      /* aAlwaysLoad = */ true);
 
     mNewRequestsWillNeedAnimationReset = false;
   }
@@ -556,8 +544,7 @@ nsresult HTMLImageElement::BindToTree(BindContext& aContext, nsINode& aParent) {
     // in order to react to changes in the environment. See note of
     // https://html.spec.whatwg.org/multipage/embedded-content.html#img-environment-changes
     QueueImageLoadTask(false);
-  } else if (!InResponsiveMode() &&
-             HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
+  } else if (!InResponsiveMode() && HasAttr(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
@@ -749,8 +736,7 @@ nsresult HTMLImageElement::CopyInnerTo(HTMLImageElement* aDest) {
   // doing the image load because we passed in false for aNotify.  But we
   // really do want it to do the load, so set it up to happen once the cloning
   // reaches a stable state.
-  if (!aDest->InResponsiveMode() &&
-      aDest->HasAttr(kNameSpaceID_None, nsGkAtoms::src) &&
+  if (!aDest->InResponsiveMode() && aDest->HasAttr(nsGkAtoms::src) &&
       aDest->ShouldLoadImage()) {
     // Mark channel as urgent-start before load image if the image load is
     // initaiated by a user interaction.
@@ -835,12 +821,12 @@ void HTMLImageElement::QueueImageLoadTask(bool aAlwaysLoad) {
 }
 
 bool HTMLImageElement::HaveSrcsetOrInPicture() {
-  if (HasAttr(kNameSpaceID_None, nsGkAtoms::srcset)) {
+  if (HasAttr(nsGkAtoms::srcset)) {
     return true;
   }
 
-  Element* parent = nsINode::GetParentElement();
-  return (parent && parent->IsHTMLElement(nsGkAtoms::picture));
+  Element* parent = GetParentElement();
+  return parent && parent->IsHTMLElement(nsGkAtoms::picture);
 }
 
 bool HTMLImageElement::InResponsiveMode() {
@@ -904,40 +890,47 @@ nsresult HTMLImageElement::LoadSelectedImage(bool aForce, bool aNotify,
 
   nsresult rv = NS_ERROR_FAILURE;
   nsCOMPtr<nsIURI> selectedSource;
+  nsCOMPtr<nsIPrincipal> triggeringPrincipal;
+  ImageLoadType type = eImageLoadType_Normal;
   if (mResponsiveSelector) {
-    nsCOMPtr<nsIURI> url = mResponsiveSelector->GetSelectedImageURL();
-    nsCOMPtr<nsIPrincipal> triggeringPrincipal =
+    selectedSource = mResponsiveSelector->GetSelectedImageURL();
+    triggeringPrincipal =
         mResponsiveSelector->GetSelectedImageTriggeringPrincipal();
-    selectedSource = url;
-    if (!aAlwaysLoad && SelectedSourceMatchesLast(selectedSource)) {
-      UpdateDensityOnly();
-      return NS_OK;
-    }
-    if (url) {
-      rv = LoadImage(url, aForce, aNotify, eImageLoadType_Imageset,
-                     triggeringPrincipal);
-    }
+    type = eImageLoadType_Imageset;
   } else {
     nsAutoString src;
-    if (!GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
+    if (!GetAttr(nsGkAtoms::src, src) || src.IsEmpty()) {
       CancelImageRequests(aNotify);
       rv = NS_OK;
     } else {
       Document* doc = OwnerDoc();
       StringToURI(src, doc, getter_AddRefs(selectedSource));
-      if (!aAlwaysLoad && SelectedSourceMatchesLast(selectedSource)) {
-        UpdateDensityOnly();
+      if (HaveSrcsetOrInPicture()) {
+        // If we have a srcset attribute or are in a <picture> element, we
+        // always use the Imageset load type, even if we parsed no valid
+        // responsive sources from either, per spec.
+        type = eImageLoadType_Imageset;
+      }
+      triggeringPrincipal = mSrcTriggeringPrincipal;
+    }
+  }
+
+  if (!aAlwaysLoad && SelectedSourceMatchesLast(selectedSource)) {
+    UpdateDensityOnly();
+    return NS_OK;
+  }
+
+  if (selectedSource) {
+    // Before we actually defer the lazy-loading
+    if (mLazyLoading) {
+      if (!nsContentUtils::IsImageAvailable(
+              this, selectedSource, triggeringPrincipal, GetCORSMode())) {
         return NS_OK;
       }
-
-      // If we have a srcset attribute or are in a <picture> element,
-      // we always use the Imageset load type, even if we parsed no
-      // valid responsive sources from either, per spec.
-      rv = LoadImage(src, aForce, aNotify,
-                     HaveSrcsetOrInPicture() ? eImageLoadType_Imageset
-                                             : eImageLoadType_Normal,
-                     mSrcTriggeringPrincipal);
+      StopLazyLoading(FromIntersectionObserver::No, StartLoading::No);
     }
+
+    rv = LoadImage(selectedSource, aForce, aNotify, type, triggeringPrincipal);
   }
   mLastSelectedSource = selectedSource;
   mCurrentDensity = currentDensity;
@@ -1147,7 +1140,7 @@ bool HTMLImageElement::TryCreateResponsiveSelector(Element* aSourceElement) {
 
   // Skip if has no srcset or an empty srcset
   nsString srcset;
-  if (!aSourceElement->GetAttr(kNameSpaceID_None, nsGkAtoms::srcset, srcset)) {
+  if (!aSourceElement->GetAttr(nsGkAtoms::srcset, srcset)) {
     return false;
   }
 
@@ -1164,14 +1157,14 @@ bool HTMLImageElement::TryCreateResponsiveSelector(Element* aSourceElement) {
   }
 
   nsAutoString sizes;
-  aSourceElement->GetAttr(kNameSpaceID_None, nsGkAtoms::sizes, sizes);
+  aSourceElement->GetAttr(nsGkAtoms::sizes, sizes);
   sel->SetSizesFromDescriptor(sizes);
 
   // If this is the <img> tag, also pull in src as the default source
   if (!isSourceTag) {
     MOZ_ASSERT(aSourceElement == this);
     nsAutoString src;
-    if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src) && !src.IsEmpty()) {
+    if (GetAttr(nsGkAtoms::src, src) && !src.IsEmpty()) {
       sel->SetDefaultSource(src, mSrcTriggeringPrincipal);
     }
   }
@@ -1246,7 +1239,7 @@ void HTMLImageElement::MediaFeatureValuesChanged() {
 }
 
 bool HTMLImageElement::ShouldLoadImage() const {
-  return OwnerDoc()->ShouldLoadImages() && !mLazyLoading;
+  return OwnerDoc()->ShouldLoadImages();
 }
 
 void HTMLImageElement::SetLazyLoading() {
@@ -1279,7 +1272,7 @@ void HTMLImageElement::StartLoadingIfNeeded() {
     // Use script runner for the case the adopt is from appendChild.
     // Bug 1076583 - We still behave synchronously in the non-responsive case
     nsContentUtils::AddScriptRunner(
-        (InResponsiveMode())
+        InResponsiveMode()
             ? NewRunnableMethod<bool>(
                   "dom::HTMLImageElement::QueueImageLoadTask", this,
                   &HTMLImageElement::QueueImageLoadTask, true)
@@ -1289,22 +1282,26 @@ void HTMLImageElement::StartLoadingIfNeeded() {
   }
 }
 
-void HTMLImageElement::StopLazyLoadingAndStartLoadIfNeeded(
-    bool aFromIntersectionObserver) {
+void HTMLImageElement::StopLazyLoading(
+    FromIntersectionObserver aFromIntersectionObserver,
+    StartLoading aStartLoading) {
   if (!mLazyLoading) {
     return;
   }
   mLazyLoading = false;
   Document* doc = OwnerDoc();
   doc->GetLazyLoadImageObserver()->Unobserve(*this);
-  StartLoadingIfNeeded();
 
-  if (aFromIntersectionObserver) {
+  if (bool(aFromIntersectionObserver)) {
     doc->IncLazyLoadImageStarted();
   } else {
     doc->DecLazyLoadImageCount();
     doc->GetLazyLoadImageObserverViewport()->Unobserve(*this);
   }
+
+  if (bool(aStartLoading)) {
+    StartLoadingIfNeeded();
+  }
 }
 
 void HTMLImageElement::LazyLoadImageReachedViewport() {

dom/html/HTMLImageElement.h

@@ -264,7 +264,10 @@ class HTMLImageElement final : public nsGenericHTMLElement,
       const nsAString& aTypeAttr, const nsAString& aMediaAttr,
       nsAString& aResult);
 
-  void StopLazyLoadingAndStartLoadIfNeeded(bool aFromIntersectionObserver);
+  enum class FromIntersectionObserver : bool { No, Yes };
+  enum class StartLoading : bool { No, Yes };
+  void StopLazyLoading(FromIntersectionObserver, StartLoading);
+
   void LazyLoadImageReachedViewport();
 
  protected:

image/imgLoader.cpp

@@ -2100,6 +2100,23 @@ static void MakeRequestStaticIfNeeded(
   proxy.forget(aProxyAboutToGetReturned);
 }
 
+bool imgLoader::IsImageAvailable(nsIURI* aURI,
+                                 nsIPrincipal* aTriggeringPrincipal,
+                                 CORSMode aCORSMode, Document* aDocument) {
+  ImageCacheKey key(aURI, aTriggeringPrincipal->OriginAttributesRef(),
+                    aDocument);
+  RefPtr<imgCacheEntry> entry;
+  imgCacheTable& cache = GetCache(key);
+  if (!cache.Get(key, getter_AddRefs(entry)) || !entry) {
+    return false;
+  }
+  RefPtr<imgRequest> request = entry->GetRequest();
+  if (!request) {
+    return false;
+  }
+  return ValidateCORSMode(request, false, aCORSMode, aTriggeringPrincipal);
+}
+
 nsresult imgLoader::LoadImage(
     nsIURI* aURI, nsIURI* aInitialDocumentURI, nsIReferrerInfo* aReferrerInfo,
     nsIPrincipal* aTriggeringPrincipal, uint64_t aRequestContextID,

image/imgLoader.h

@@ -241,6 +241,9 @@ class imgLoader final : public imgILoader,
   imgLoader();
   nsresult Init();
 
+  bool IsImageAvailable(nsIURI*, nsIPrincipal* aTriggeringPrincipal,
+                        mozilla::CORSMode, mozilla::dom::Document*);
+
   [[nodiscard]] nsresult LoadImage(
       nsIURI* aURI, nsIURI* aInitialDocumentURI, nsIReferrerInfo* aReferrerInfo,
       nsIPrincipal* aLoadingPrincipal, uint64_t aRequestContextID,

testing/web-platform/tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-available.html

@@ -0,0 +1,30 @@
+<!doctype html>
+<title>The list of available images gets checked before deciding to make a load lazy</title>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/images.html#update-the-image-data">
+<link rel="help" href="https://html.spec.whatwg.org/multipage/images.html#will-lazy-load-image-steps">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1709577">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<img src="/images/green-256x256.png">
+<div style="height:1000vh;"></div>
+<script>
+promise_test(async t => {
+  await new Promise(resolve => {
+    window.addEventListener("load", resolve);
+  });
+  let nonLazy = document.querySelector("img");
+  assert_equals(nonLazy.width, 256);
+  assert_equals(nonLazy.height, 256);
+
+  let lazy = document.createElement("img");
+  lazy.loading = "lazy";
+  lazy.src = nonLazy.src;
+  document.body.appendChild(lazy);
+
+  await new Promise(resolve => setTimeout(resolve));
+
+  assert_equals(lazy.width, 256, "The list of available images should be checked before delaying the image load");
+  assert_equals(lazy.height, 256, "The list of available images should be checked before delaying the image load");
+});
+</script>

testing/web-platform/tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-base-url-2.html

@@ -46,7 +46,7 @@
     // to it when it sets up the request at parse-time.
     window.history.pushState(1, document.title, 'resources/')
   </script>
-  <img id="below-viewport" src="image.png" loading="lazy"
+  <img id="below-viewport" src="image.png?base-url-2" loading="lazy"
        onload="below_viewport_img.resolve()"
        onerror="below_viewport_img.reject()">
 </body>

testing/web-platform/tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-base-url.html

@@ -45,7 +45,7 @@
 
 <body>
   <div style="height:1000vh"></div>
-  <img id="below-viewport" src="image.png" loading="lazy"
+  <img id="below-viewport" src="image.png?base-url" loading="lazy"
        onload="below_viewport_img.resolve()"
        onerror="below_viewport_img.reject()">
 </body>

testing/web-platform/tests/html/semantics/embedded-content/the-img-element/not-rendered-below-viewport-image-loading-lazy.html

@@ -12,11 +12,11 @@
 <body>
   <!-- These images must not attempt to load when scrolled into the
        viewport -->
-  <img id="display_none" style="display:none;" src="resources/image.png?2" loading="lazy"
+  <img id="display_none" style="display:none;" src="resources/image.png?not-rendered-2" loading="lazy"
        onload="display_none_img.resolve();" onerror="display_none_img.reject();">
-  <img id="attribute_hidden" hidden src="resources/image.png?3" loading="lazy"
+  <img id="attribute_hidden" hidden src="resources/image.png?not-rendered-3" loading="lazy"
        onload="attribute_hidden_img.resolve();" onerror="attribute_hidden_img.reject();">
-  <img id="js_display_none" src="resources/image.png?4" loading="lazy"
+  <img id="js_display_none" src="resources/image.png?not-rendered-4" loading="lazy"
        onload="js_display_none_img.resolve();" onerror="js_display_none_img.reject();">
   <script>
     document.getElementById("js_display_none").style = 'display:none;';