Backed out changeset a77ccd1c1659 (bug 1687358) for browser_lazyload_telemetry.js failures CLOSED TREE

dom/base/DOMIntersectionObserver.cpp

@@ -152,18 +152,7 @@ static void LazyLoadCallback(
     MOZ_ASSERT(entry->Target()->IsHTMLElement(nsGkAtoms::img));
     if (entry->IsIntersecting()) {
       static_cast<HTMLImageElement*>(entry->Target())
-          ->StopLazyLoadingAndStartLoadIfNeeded(true);
+          ->StopLazyLoadingAndStartLoadIfNeeded();
-    }
-  }
-}
-
-static void LazyLoadCallbackReachViewport(
-    const Sequence<OwningNonNull<DOMIntersectionObserverEntry>>& aEntries) {
-  for (const auto& entry : aEntries) {
-    MOZ_ASSERT(entry->Target()->IsHTMLElement(nsGkAtoms::img));
-    if (entry->IsIntersecting()) {
-      static_cast<HTMLImageElement*>(entry->Target())
-          ->LazyLoadImageReachedViewport();
     }
   }
 }
@@ -200,14 +189,6 @@ DOMIntersectionObserver::CreateLazyLoadObserver(Document& aDocument) {
   return observer.forget();
 }
 
-already_AddRefed<DOMIntersectionObserver>
-DOMIntersectionObserver::CreateLazyLoadObserverViewport(Document& aDocument) {
-  RefPtr<DOMIntersectionObserver> observer =
-      new DOMIntersectionObserver(aDocument, LazyLoadCallbackReachViewport);
-  observer->mThresholds.AppendElement(std::numeric_limits<double>::min());
-  return observer.forget();
-}
-
 bool DOMIntersectionObserver::SetRootMargin(const nsACString& aString) {
   return Servo_IntersectionObserverRootMargin_Parse(&aString, &mRootMargin);
 }

dom/base/DOMIntersectionObserver.h

@@ -127,8 +127,6 @@ class DOMIntersectionObserver final : public nsISupports,
 
   static already_AddRefed<DOMIntersectionObserver> CreateLazyLoadObserver(
       Document&);
-  static already_AddRefed<DOMIntersectionObserver>
-  CreateLazyLoadObserverViewport(Document&);
 
  protected:
   void Connect();

dom/base/Document.cpp

@@ -1403,10 +1403,6 @@ Document::Document(const char* aContentType)
       mOnloadBlockCount(0),
       mAsyncOnloadBlockCount(0),
       mWriteLevel(0),
-      mLazyLoadImageCount(0),
-      mLazyLoadImageStarted(0),
-      mLazyLoadImageReachViewportLoading(0),
-      mLazyLoadImageReachViewportLoaded(0),
       mContentEditableCount(0),
       mEditingState(EditingState::eOff),
       mCompatMode(eCompatibility_FullStandards),
@@ -2378,7 +2374,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(Document)
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOnloadBlocker)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLazyLoadImageObserver)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLazyLoadImageObserverViewport)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDOMImplementation)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mImageMaps)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOrientationPendingPromise)
@@ -2501,7 +2496,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Document)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSecurityInfo)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDisplayDocument)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLazyLoadImageObserver)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mLazyLoadImageObserverViewport)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mFontFaceSet)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mReadyForIdle)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocumentL10n)
@@ -15415,25 +15409,6 @@ void Document::ReportDocumentUseCounters() {
             (" > %s\n", Telemetry::GetHistogramName(id)));
     Telemetry::Accumulate(id, 1);
   }
-
-  ReportDocumentLazyLoadCounters();
-}
-
-void Document::ReportDocumentLazyLoadCounters() {
-  if (!mLazyLoadImageCount) {
-    return;
-  }
-  Telemetry::Accumulate(Telemetry::LAZYLOAD_IMAGE_TOTAL, mLazyLoadImageCount);
-  Telemetry::Accumulate(Telemetry::LAZYLOAD_IMAGE_STARTED,
-                        mLazyLoadImageStarted);
-  Telemetry::Accumulate(Telemetry::LAZYLOAD_IMAGE_NOT_VIEWPORT,
-                        mLazyLoadImageStarted -
-                            mLazyLoadImageReachViewportLoading -
-                            mLazyLoadImageReachViewportLoaded);
-  Telemetry::Accumulate(Telemetry::LAZYLOAD_IMAGE_VIEWPORT_LOADING,
-                        mLazyLoadImageReachViewportLoading);
-  Telemetry::Accumulate(Telemetry::LAZYLOAD_IMAGE_VIEWPORT_LOADED,
-                        mLazyLoadImageReachViewportLoaded);
 }
 
 void Document::SendPageUseCounters() {
@@ -15562,22 +15537,6 @@ DOMIntersectionObserver& Document::EnsureLazyLoadImageObserver() {
   return *mLazyLoadImageObserver;
 }
 
-DOMIntersectionObserver& Document::EnsureLazyLoadImageObserverViewport() {
-  if (!mLazyLoadImageObserverViewport) {
-    mLazyLoadImageObserverViewport =
-        DOMIntersectionObserver::CreateLazyLoadObserverViewport(*this);
-  }
-  return *mLazyLoadImageObserverViewport;
-}
-
-void Document::IncLazyLoadImageReachViewport(bool aLoading) {
-  if (aLoading) {
-    ++mLazyLoadImageReachViewportLoading;
-  } else {
-    ++mLazyLoadImageReachViewportLoaded;
-  }
-}
-
 void Document::NotifyLayerManagerRecreated() {
   EnumerateActivityObservers(NotifyActivityChanged);
   EnumerateSubDocuments([](Document& aSubDoc) {

dom/base/Document.h

@@ -3616,9 +3616,6 @@ class Document : public nsINode,
   // effect once per document, and so is called during document destruction.
   void ReportDocumentUseCounters();
 
-  // Report how lazyload performs for this document.
-  void ReportDocumentLazyLoadCounters();
-
   // Sends page use counters to the parent process to accumulate against the
   // top-level document.  Must be called while we still have access to our
   // WindowContext.  This method has an effect each time it is called, and we
@@ -3732,18 +3729,7 @@ class Document : public nsINode,
   DOMIntersectionObserver* GetLazyLoadImageObserver() {
     return mLazyLoadImageObserver;
   }
-  DOMIntersectionObserver* GetLazyLoadImageObserverViewport() {
-    return mLazyLoadImageObserverViewport;
-  }
   DOMIntersectionObserver& EnsureLazyLoadImageObserver();
-  DOMIntersectionObserver& EnsureLazyLoadImageObserverViewport();
-  void IncLazyLoadImageCount() { ++mLazyLoadImageCount; }
-  void DecLazyLoadImageCount() {
-    MOZ_DIAGNOSTIC_ASSERT(mLazyLoadImageCount > 0);
-    --mLazyLoadImageCount;
-  }
-  void IncLazyLoadImageStarted() { ++mLazyLoadImageStarted; }
-  void IncLazyLoadImageReachViewport(bool aLoading);
 
   // Dispatch a runnable related to the document.
   nsresult Dispatch(TaskCategory aCategory,
@@ -4728,19 +4714,6 @@ class Document : public nsINode,
   // finishes processing that script.
   uint32_t mWriteLevel;
 
-  // The amount of images that have `loading="lazy"` on the page or have loaded
-  // lazily already.
-  uint32_t mLazyLoadImageCount;
-  // Number of lazy-loaded images that we've started loading as a result of
-  // triggering the lazy-load observer threshold.
-  uint32_t mLazyLoadImageStarted;
-  // Number of lazy-loaded images that reached the viewport which were not done
-  // loading when they did so.
-  uint32_t mLazyLoadImageReachViewportLoading;
-  // Number of lazy-loaded images that reached the viewport and were done
-  // loading when they did so.
-  uint32_t mLazyLoadImageReachViewportLoaded;
-
   uint32_t mContentEditableCount;
   EditingState mEditingState;
 
@@ -5036,8 +5009,6 @@ class Document : public nsINode,
   nsTHashtable<nsPtrHashKey<DOMIntersectionObserver>> mIntersectionObservers;
 
   RefPtr<DOMIntersectionObserver> mLazyLoadImageObserver;
-  // Used to measure how effective the lazyload thresholds are.
-  RefPtr<DOMIntersectionObserver> mLazyLoadImageObserverViewport;
 
   // Stack of top layer elements.
   nsTArray<nsWeakPtr> mTopLayer;

dom/html/HTMLImageElement.cpp

@@ -352,7 +352,7 @@ nsresult HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                static_cast<HTMLImageElement::Loading>(
                    aOldValue->GetEnumValue()) == Loading::Lazy &&
                !ImageState().HasState(NS_EVENT_STATE_LOADING)) {
-      StopLazyLoadingAndStartLoadIfNeeded(false);
+      StopLazyLoadingAndStartLoadIfNeeded();
     }
   } else if (aName == nsGkAtoms::src && !aValue) {
     // NOTE: regular src value changes are handled in AfterMaybeChangeAttr, so
@@ -653,18 +653,8 @@ EventStates HTMLImageElement::IntrinsicState() const {
 void HTMLImageElement::NodeInfoChanged(Document* aOldDoc) {
   nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
 
-  // Unlike the LazyLoadImageObserver, the intersection observer
-  // for the viewport could contain the element even if
-  // it's not lazy-loading. For instance, the element has
-  // started to load, but haven't reached to the viewport.
-  // So here we always try to unobserve it.
-  if (auto* observer = aOldDoc->GetLazyLoadImageObserverViewport()) {
-    observer->Unobserve(*this);
-  }
-
   if (mLazyLoading) {
     aOldDoc->GetLazyLoadImageObserver()->Unobserve(*this);
-    aOldDoc->DecLazyLoadImageCount();
     mLazyLoading = false;
     SetLazyLoading();
   }
@@ -1266,9 +1256,10 @@ void HTMLImageElement::SetLazyLoading() {
     return;
   }
 
-  doc->EnsureLazyLoadImageObserver().Observe(*this);
+  // There (maybe) is a race condition that we have no LazyLoadImageObserver
-  doc->EnsureLazyLoadImageObserverViewport().Observe(*this);
+  // when the root document has been removed from the docshell.
-  doc->IncLazyLoadImageCount();
+  // In the case we don't need to worry about lazy-loading.
+  OwnerDoc()->EnsureLazyLoadImageObserver().Observe(*this);
   mLazyLoading = true;
   UpdateImageState(true);
 }
@@ -1288,28 +1279,13 @@ void HTMLImageElement::StartLoadingIfNeeded() {
   }
 }
 
-void HTMLImageElement::StopLazyLoadingAndStartLoadIfNeeded(
+void HTMLImageElement::StopLazyLoadingAndStartLoadIfNeeded() {
-    bool aFromIntersectionObserver) {
   if (!mLazyLoading) {
     return;
   }
   mLazyLoading = false;
-  Document* doc = OwnerDoc();
+  OwnerDoc()->GetLazyLoadImageObserver()->Unobserve(*this);
-  doc->GetLazyLoadImageObserver()->Unobserve(*this);
   StartLoadingIfNeeded();
-
-  if (aFromIntersectionObserver) {
-    doc->IncLazyLoadImageStarted();
-  } else {
-    doc->DecLazyLoadImageCount();
-    doc->GetLazyLoadImageObserverViewport()->Unobserve(*this);
-  }
-}
-
-void HTMLImageElement::LazyLoadImageReachedViewport() {
-  Document* doc = OwnerDoc();
-  doc->GetLazyLoadImageObserverViewport()->Unobserve(*this);
-  doc->IncLazyLoadImageReachViewport(!Complete());
 }
 
 }  // namespace mozilla::dom

dom/html/HTMLImageElement.h

@@ -261,8 +261,7 @@ class HTMLImageElement final : public nsGenericHTMLElement,
       const nsAString& aTypeAttr, const nsAString& aMediaAttr,
       nsAString& aResult);
 
-  void StopLazyLoadingAndStartLoadIfNeeded(bool aFromIntersectionObserver);
+  void StopLazyLoadingAndStartLoadIfNeeded();
-  void LazyLoadImageReachedViewport();
 
  protected:
   virtual ~HTMLImageElement();

layout/base/tests/browser.ini

@@ -22,7 +22,3 @@ skip-if = (verify && (os == 'mac')) # bug 1627874
 support-files =
   !/gfx/layers/apz/test/mochitest/apz_test_native_event_utils.js
   !/browser/base/content/test/forms/head.js
-[browser_lazyload_telemetry.js]
-support-files =
-  file_lazyload_telemetry.html
-  image_rgrg-256x256.png

layout/base/tests/browser_lazyload_telemetry.js

@@ -1,61 +0,0 @@
-const baseURL = getRootDirectory(gTestPath).replace(
-  "chrome://mochitests/content",
-  "http://example.com"
-);
-
-const testFileURL = `${baseURL}file_lazyload_telemetry.html`;
-
-const { TelemetryTestUtils } = ChromeUtils.import(
-  "resource://testing-common/TelemetryTestUtils.jsm"
-);
-
-function dataIsReported() {
-  const snapshot = Services.telemetry.getSnapshotForHistograms("main", false)
-    .content;
-  return (
-    snapshot.LAZYLOAD_IMAGE_TOTAL && snapshot.LAZYLOAD_IMAGE_TOTAL.values[4]
-  );
-}
-
-add_task(async function testTelemetryCollection() {
-  Services.telemetry.getHistogramById("LAZYLOAD_IMAGE_TOTAL").clear();
-  Services.telemetry.getHistogramById("LAZYLOAD_IMAGE_STARTED").clear();
-  Services.telemetry.getHistogramById("LAZYLOAD_IMAGE_NOT_VIEWPORT").clear();
-  Services.telemetry
-    .getHistogramById("LAZYLOAD_IMAGE_VIEWPORT_LOADING")
-    .clear();
-  Services.telemetry.getHistogramById("LAZYLOAD_IMAGE_VIEWPORT_LOADED").clear();
-
-  const testTab = await BrowserTestUtils.openNewForegroundTab(
-    gBrowser,
-    testFileURL,
-    true
-  );
-
-  await SpecialPowers.spawn(testTab.linkedBrowser.browsingContext, [], () => {
-    const image2 = content.document.getElementById("image2");
-    image2.scrollIntoView();
-  });
-
-  gBrowser.removeTab(testTab);
-
-  await BrowserTestUtils.waitForCondition(dataIsReported);
-
-  const snapshot = Services.telemetry.getSnapshotForHistograms("main", false)
-    .content;
-
-  // Ensures we have 4 lazyload images.
-  is(snapshot.LAZYLOAD_IMAGE_TOTAL.values[4], 1);
-  // All 4 images should be lazy-loaded.
-  is(snapshot.LAZYLOAD_IMAGE_STARTED.values[4], 1);
-  // The last image didn't reach to the viewport.
-  is(snapshot.LAZYLOAD_IMAGE_NOT_VIEWPORT.values[1], 1);
-  // The sum of the images that reached to the viewport
-  // should be three. This includes all images except
-  // the last one.
-  is(
-    snapshot.LAZYLOAD_IMAGE_VIEWPORT_LOADING.sum +
-      snapshot.LAZYLOAD_IMAGE_VIEWPORT_LOADED.sum,
-    3
-  );
-});

layout/base/tests/file_lazyload_telemetry.html

@@ -1,9 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-  <body>
-    <img id="image1" src="image_rgrg-256x256.png" loading="lazy" style="display:block; width:100px; height:50vh">
-    <img id="image2" src="image_rgrg-256x256.png" loading="lazy" style="display:block; width:100px; height:50vh">
-    <img id="image3" src="image_rgrg-256x256.png" loading="lazy" style="display:block; width:100px; height:50vh">
-    <img id="image4" src="image_rgrg-256x256.png" loading="lazy" style="display:block; width:100px; height:50vh">
-  </body>
-</html>

toolkit/components/telemetry/Histograms.json

@@ -4733,66 +4733,6 @@
     "releaseChannelCollection": "opt-out",
     "description": "Number of tabs opened across all windows, collected at most every 5 minutes whenever the user interacts with the browser in the following ways: open tab/window, page load."
   },
-  "LAZYLOAD_IMAGE_TOTAL": {
-    "record_in_processes": ["main", "content"],
-    "products": ["firefox"],
-    "alert_emails": ["sefeng@mozilla.com"],
-    "bug_numbers": [1687358],
-    "expires_in_version": "never",
-    "kind": "exponential",
-    "high": 2000,
-    "n_buckets": 100,
-    "releaseChannelCollection": "opt-out",
-    "description": "The amount of images that have `loading='lazy'` on the page or have loaded lazily already."
-  },
-  "LAZYLOAD_IMAGE_STARTED": {
-    "record_in_processes": ["main", "content"],
-    "products": ["firefox"],
-    "alert_emails": ["sefeng@mozilla.com"],
-    "bug_numbers": [1687358],
-    "expires_in_version": "never",
-    "kind": "exponential",
-    "high": 2000,
-    "n_buckets": 100,
-    "releaseChannelCollection": "opt-out",
-    "description": "Number of lazy-loaded images that we've started loading as a result of triggering the lazy-load observer threshold."
-  },
-  "LAZYLOAD_IMAGE_NOT_VIEWPORT": {
-    "record_in_processes": ["main", "content"],
-    "products": ["firefox"],
-    "alert_emails": ["sefeng@mozilla.com"],
-    "bug_numbers": [1687358],
-    "expires_in_version": "never",
-    "kind": "exponential",
-    "high": 2000,
-    "n_buckets": 100,
-    "releaseChannelCollection": "opt-out",
-    "description": "Number of lazy-loaded images that were started to load but didn't reach to the viewport eventually"
-  },
-  "LAZYLOAD_IMAGE_VIEWPORT_LOADING": {
-    "record_in_processes": ["main", "content"],
-    "products": ["firefox"],
-    "alert_emails": ["sefeng@mozilla.com"],
-    "bug_numbers": [1687358],
-    "expires_in_version": "never",
-    "kind": "exponential",
-    "high": 2000,
-    "n_buckets": 100,
-    "releaseChannelCollection": "opt-out",
-    "description": "Number of lazy-loaded images that reached the viewport which were not done loading when they did so."
-  },
-  "LAZYLOAD_IMAGE_VIEWPORT_LOADED": {
-    "record_in_processes": ["main", "content"],
-    "products": ["firefox"],
-    "alert_emails": ["sefeng@mozilla.com"],
-    "bug_numbers": [1687358],
-    "expires_in_version": "never",
-    "kind": "exponential",
-    "high": 2000,
-    "n_buckets": 100,
-    "releaseChannelCollection": "opt-out",
-    "description": "Number of lazy-loaded images that reached the viewport and were done loading when they did so."
-  },
   "LOADED_TAB_COUNT": {
     "record_in_processes": ["main"],
     "products": ["firefox"],