From f64788828d61c24d116b754afc8148511cb32d56 Mon Sep 17 00:00:00 2001
From: Hiroyuki Ikezoe <hikezoe.birchill@mozilla.com>
Date: Sat, 15 Feb 2020 08:45:27 +0000
Subject: [PATCH] Bug 1612649 - Call UpdateImageState in SetDeferredLoad to
 start with `not broken` state for lazy load images. r=emilio

So that we can generate nsImageFrame for lazy load image elements in the first
place.

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

--HG--
rename : layout/reftests/image/moz-broken-matching-1.html => layout/reftests/image/moz-broken-matching-lazy-load.html
extra : moz-landing-system : lando
---
 dom/base/nsImageLoadingContent.cpp            | 10 +++--
 dom/base/nsImageLoadingContent.h              | 19 ++++++----
 dom/html/HTMLImageElement.cpp                 |  2 +-
 dom/html/HTMLImageElement.h                   |  3 --
 .../file_reframe_for_lazy_load_image.html     | 38 +++++++++++++++++++
 layout/generic/test/mochitest.ini             |  3 ++
 .../test_reframe_for_lazy_load_image.html     | 20 ++++++++++
 .../image/moz-broken-matching-lazy-load.html  | 13 +++++++
 layout/reftests/image/reftest.list            |  1 +
 9 files changed, 94 insertions(+), 15 deletions(-)
 create mode 100644 layout/generic/test/file_reframe_for_lazy_load_image.html
 create mode 100644 layout/generic/test/test_reframe_for_lazy_load_image.html
 create mode 100644 layout/reftests/image/moz-broken-matching-lazy-load.html

diff --git a/dom/base/nsImageLoadingContent.cpp b/dom/base/nsImageLoadingContent.cpp
index 40f6034eccfe..d0e76490f2de 100644
--- a/dom/base/nsImageLoadingContent.cpp
+++ b/dom/base/nsImageLoadingContent.cpp
@@ -106,6 +106,7 @@ nsImageLoadingContent::nsImageLoadingContent()
       mSuppressed(false),
       mNewRequestsWillNeedAnimationReset(false),
       mUseUrgentStartForChannel(false),
+      mLazyLoading(false),
       mStateChangerDepth(0),
       mCurrentRequestRegistered(false),
       mPendingRequestRegistered(false),
@@ -1303,9 +1304,12 @@ void nsImageLoadingContent::UpdateImageState(bool aNotify) {
   } else if (mImageBlockingStatus == nsIContentPolicy::REJECT_TYPE) {
     mUserDisabled = true;
   } else if (!mCurrentRequest) {
-    // No current request means error, since we weren't disabled or suppressed
-    mBroken = true;
-    RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
+    if (!mLazyLoading) {
+      // In case of non-lazy loading, no current request means error, since we
+      // weren't disabled or suppressed
+      mBroken = true;
+      RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
+    }
   } else {
     uint32_t currentLoadStatus;
     nsresult rv = mCurrentRequest->GetImageStatus(&currentLoadStatus);
diff --git a/dom/base/nsImageLoadingContent.h b/dom/base/nsImageLoadingContent.h
index dd280fcd8c0a..27610029e00e 100644
--- a/dom/base/nsImageLoadingContent.h
+++ b/dom/base/nsImageLoadingContent.h
@@ -340,13 +340,6 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
 
   friend struct AutoStateChanger;
 
-  /**
-   * UpdateImageState recomputes the current state of this image loading
-   * content and updates what ImageState() returns accordingly.  It will also
-   * fire a ContentStatesChanged() notification as needed if aNotify is true.
-   */
-  void UpdateImageState(bool aNotify);
-
   /**
    * Method to fire an event once we know what's going on with the image load.
    *
@@ -364,6 +357,13 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
   RefPtr<mozilla::AsyncEventDispatcher> mPendingEvent;
 
  protected:
+  /**
+   * UpdateImageState recomputes the current state of this image loading
+   * content and updates what ImageState() returns accordingly.  It will also
+   * fire a ContentStatesChanged() notification as needed if aNotify is true.
+   */
+  void UpdateImageState(bool aNotify);
+
   /**
    * Method to create an nsIURI object from the given string (will
    * handle getting the right charset, base, etc).  You MUST pass in a
@@ -600,7 +600,10 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
    * True if we want to set nsIClassOfService::UrgentStart to the channel to
    * get the response ASAP for better user responsiveness.
    */
-  bool mUseUrgentStartForChannel;
+  bool mUseUrgentStartForChannel : 1;
+
+  // Represents the image is deferred loading until this element gets visible.
+  bool mLazyLoading : 1;
 
  private:
   /* The number of nested AutoStateChangers currently tracking our state. */
diff --git a/dom/html/HTMLImageElement.cpp b/dom/html/HTMLImageElement.cpp
index 653b66a708e8..5f57ce8a7ad6 100644
--- a/dom/html/HTMLImageElement.cpp
+++ b/dom/html/HTMLImageElement.cpp
@@ -114,7 +114,6 @@ HTMLImageElement::HTMLImageElement(
     : nsGenericHTMLElement(std::move(aNodeInfo)),
       mForm(nullptr),
       mInDocResponsiveContent(false),
-      mLazyLoading(false),
       mCurrentDensity(1.0) {
   // We start out broken
   AddStatesSilently(NS_EVENT_STATE_BROKEN);
@@ -1267,6 +1266,7 @@ void HTMLImageElement::SetLazyLoading() {
           OwnerDoc()->GetLazyLoadImageObserver()) {
     lazyLoadObserver->Observe(*this);
     mLazyLoading = true;
+    UpdateImageState(true);
   }
 }
 
diff --git a/dom/html/HTMLImageElement.h b/dom/html/HTMLImageElement.h
index 940196cec474..924c50fddffa 100644
--- a/dom/html/HTMLImageElement.h
+++ b/dom/html/HTMLImageElement.h
@@ -390,9 +390,6 @@ class HTMLImageElement final : public nsGenericHTMLElement,
 
   bool mInDocResponsiveContent;
 
-  // Represents the image is deferred loading until this element gets visible.
-  bool mLazyLoading;
-
   RefPtr<ImageLoadTask> mPendingImageLoadTask;
   nsCOMPtr<nsIPrincipal> mSrcTriggeringPrincipal;
   nsCOMPtr<nsIPrincipal> mSrcsetTriggeringPrincipal;
diff --git a/layout/generic/test/file_reframe_for_lazy_load_image.html b/layout/generic/test/file_reframe_for_lazy_load_image.html
new file mode 100644
index 000000000000..912fbdf3c647
--- /dev/null
+++ b/layout/generic/test/file_reframe_for_lazy_load_image.html
@@ -0,0 +1,38 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>
+  Test for bug 1612649: We don't reframe for lazy load image state changes.
+</title>
+<div id="spacer" style="height: calc(100vh + 100px);"></div>
+<img id="image" loading="lazy" width="20" height="20" alt="this is an image"
+     src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAAG0lEQVR42mP8z0A%2BYKJA76jmUc2jmkc1U0EzACKcASfOgGoMAAAAAElFTkSuQmCC">
+<script>
+const is = opener.is.bind(opener);
+const add_task = opener.add_task;
+const original_finish = opener.SimpleTest.finish;
+const SimpleTest = opener.SimpleTest;
+SimpleTest.finish = function finish() {
+  self.close();
+  original_finish();
+}
+
+const utils = SpecialPowers.DOMWindowUtils;
+
+add_task(async () => {
+  await SimpleTest.promiseFocus();
+
+  const previousConstructCount = utils.framesConstructed;
+
+  const loadPromise =
+      new Promise(resolve => image.addEventListener("load", resolve));
+
+  image.scrollIntoView();
+
+  await loadPromise;
+
+  image.getBoundingClientRect();
+
+  is(previousConstructCount, utils.framesConstructed,
+     "We should not have reframed");
+});
+</script>
diff --git a/layout/generic/test/mochitest.ini b/layout/generic/test/mochitest.ini
index 8428e75f35f8..f6678d8d8e4e 100644
--- a/layout/generic/test/mochitest.ini
+++ b/layout/generic/test/mochitest.ini
@@ -158,3 +158,6 @@ support-files =
 skip-if = debug == true || tsan # the test is slow. tsan: bug 1612707
 [test_grid_track_sizing_algo_002.html]
 skip-if = debug == true || tsan # the test is slow. tsan: bug 1612707
+[test_reframe_for_lazy_load_image.html]
+support-files =
+    file_reframe_for_lazy_load_image.html
diff --git a/layout/generic/test/test_reframe_for_lazy_load_image.html b/layout/generic/test/test_reframe_for_lazy_load_image.html
new file mode 100644
index 000000000000..0c744b1db202
--- /dev/null
+++ b/layout/generic/test/test_reframe_for_lazy_load_image.html
@@ -0,0 +1,20 @@
+<!doctype html>
+<meta charset=utf-8>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+<div id='log'></div>
+<script>
+'use strict';
+SimpleTest.waitForExplicitFinish();
+SpecialPowers.pushPrefEnv(
+  // Open a new document with enabling lazy load since we need to create an
+  // image element with loading="lazy" and alt="something" in the first place.
+  // That's because if the element has `alt` but doesn't have `lazy` we will
+  // generate inline frames due to the `alt`.  Also if we do add the `lazy` and
+  // the `alt` dynamically, it ends up generating reconstruct change hint for
+  // the `alt` change.
+  { set: [["dom.image-lazy-loading.enabled", true]] },
+  () => { window.open('file_reframe_for_lazy_load_image.html'); }
+);
+</script>
+</html>
diff --git a/layout/reftests/image/moz-broken-matching-lazy-load.html b/layout/reftests/image/moz-broken-matching-lazy-load.html
new file mode 100644
index 000000000000..696560ac5c99
--- /dev/null
+++ b/layout/reftests/image/moz-broken-matching-lazy-load.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<style>
+  :-moz-broken {
+    border: 10px solid green;
+  }
+</style>
+<!--
+  We need to wait for an error event since we consider lazy load images are NOT
+  broken initially until loaded.
+-->
+<img loading="lazy" src="nosuch:url" onerror="document.documentElement.className=''">
+</html>
diff --git a/layout/reftests/image/reftest.list b/layout/reftests/image/reftest.list
index cadca6f94014..9008531f4e67 100644
--- a/layout/reftests/image/reftest.list
+++ b/layout/reftests/image/reftest.list
@@ -71,3 +71,4 @@ random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == image-srcset-basic-selec
 == image-resize-percent-width.html image-resize-ref.html
 
 == moz-broken-matching-1.html moz-broken-matching-1-ref.html
+pref(dom.image-lazy-loading.enabled,true) == moz-broken-matching-lazy-load.html moz-broken-matching-1-ref.html