Bug 1817360 - Remove browser.display.show_loading_image_placeholder. r=tnikkel

This pref has been false since forever, completely untested, and I see
no references to it anywhere. I'm pretty sure having a loading image
placeholder wouldn't be web compatible, particularly in the current days
with all the lazy-loading shenanigans etc.

I propose just removing this code, and simplifying surrounding code for
clarity.

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

layout/generic/jar.mn

@@ -4,4 +4,3 @@
 
 toolkit.jar:
    res/broken-image.png                (broken-image.png)
-   res/loading-image.png               (loading-image.png)

layout/generic/nsImageFrame.cpp

@@ -213,7 +213,6 @@ class IconLoad final : public imgINotificationObserver {
   nsTObserverArray<nsImageFrame*> mIconObservers;
 
  public:
-  RefPtr<imgRequestProxy> mLoadingImage;
   RefPtr<imgRequestProxy> mBrokenImage;
 };
 
@@ -303,6 +302,10 @@ bool nsImageFrame::ShouldShowBrokenImageIcon() const {
     return false;
   }
 
+  if (!StaticPrefs::browser_display_show_image_placeholders()) {
+    return false;
+  }
+
   // <img alt=""> is special, and it shouldn't draw the broken image icon,
   // unlike the no-alt attribute or non-empty-alt-attribute case.
   if (auto* image = HTMLImageElement::FromNode(mContent)) {
@@ -1734,8 +1737,7 @@ ImgDrawResult nsImageFrame::DisplayAltFeedback(gfxContext& aRenderingContext,
   }
 
   // Paint the border
-  if (!isLoading ||
-      StaticPrefs::browser_display_show_loading_image_placeholder()) {
+  if (!isLoading) {
     nsRecessedBorder recessedBorder(borderEdgeWidth, PresContext());
 
     // Assert that we're not drawing a border-image here; if we were, we
@@ -1764,18 +1766,14 @@ ImgDrawResult nsImageFrame::DisplayAltFeedback(gfxContext& aRenderingContext,
   aRenderingContext.Clip(NSRectToSnappedRect(
       inner, PresContext()->AppUnitsPerDevPixel(), *drawTarget));
 
-  ImgDrawResult result = ImgDrawResult::NOT_READY;
+  ImgDrawResult result = ImgDrawResult::SUCCESS;
 
   // Check if we should display image placeholders
-  if (!ShouldShowBrokenImageIcon() ||
-      !StaticPrefs::browser_display_show_image_placeholders() ||
-      (isLoading && StaticPrefs::browser_display_show_loading_image_placeholder())) {
-    result = ImgDrawResult::SUCCESS;
-  } else {
+  if (ShouldShowBrokenImageIcon()) {
+    result = ImgDrawResult::NOT_READY;
     nscoord size = nsPresContext::CSSPixelsToAppUnits(ICON_SIZE);
 
-    imgIRequest* request = isLoading ? gIconLoad->mLoadingImage
-                                     : gIconLoad->mBrokenImage;
+    imgIRequest* request = gIconLoad->mBrokenImage;
 
     // If we weren't previously displaying an icon, register ourselves
     // as an observer for load and animation updates and flag that we're
@@ -1919,8 +1917,7 @@ ImgDrawResult nsImageFrame::DisplayAltFeedbackWithoutLayer(
   AutoSaveRestore autoSaveRestore(aBuilder, textDrawResult);
 
   // Paint the border
-  if (!isLoading ||
-      StaticPrefs::browser_display_show_loading_image_placeholder()) {
+  if (!isLoading) {
     nsRecessedBorder recessedBorder(borderEdgeWidth, PresContext());
     // Assert that we're not drawing a border-image here; if we were, we
     // couldn't ignore the ImgDrawResult that PaintBorderWithStyleBorder
@@ -1948,14 +1945,10 @@ ImgDrawResult nsImageFrame::DisplayAltFeedbackWithoutLayer(
   auto wrBounds = wr::ToLayoutRect(bounds);
 
   // Check if we should display image placeholders
-  if (ShouldShowBrokenImageIcon() &&
-      StaticPrefs::browser_display_show_image_placeholders() &&
-      (!isLoading ||
-       StaticPrefs::browser_display_show_loading_image_placeholder())) {
+  if (ShouldShowBrokenImageIcon()) {
     ImgDrawResult result = ImgDrawResult::NOT_READY;
     nscoord size = nsPresContext::CSSPixelsToAppUnits(ICON_SIZE);
-    imgIRequest* request =
-        isLoading ? gIconLoad->mLoadingImage : gIconLoad->mBrokenImage;
+    imgIRequest* request = gIconLoad->mBrokenImage;
 
     // If we weren't previously displaying an icon, register ourselves
     // as an observer for load and animation updates and flag that we're
@@ -2784,35 +2777,15 @@ void nsImageFrame::GetLoadGroup(nsPresContext* aPresContext,
 nsresult nsImageFrame::LoadIcons(nsPresContext* aPresContext) {
   NS_ASSERTION(!gIconLoad, "called LoadIcons twice");
 
-  constexpr auto loadingSrc = u"resource://gre-resources/loading-image.png"_ns;
   constexpr auto brokenSrc = u"resource://gre-resources/broken-image.png"_ns;
-
   gIconLoad = new IconLoad();
-
-  nsresult rv;
-  // create a loader and load the images
-  rv = LoadIcon(loadingSrc, aPresContext,
-                getter_AddRefs(gIconLoad->mLoadingImage));
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  rv = LoadIcon(brokenSrc, aPresContext,
+  return LoadIcon(brokenSrc, aPresContext,
                   getter_AddRefs(gIconLoad->mBrokenImage));
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  return rv;
 }
 
 NS_IMPL_ISUPPORTS(IconLoad, imgINotificationObserver)
 
 void IconLoad::Shutdown() {
-  if (mLoadingImage) {
-    mLoadingImage->CancelAndForgetObserver(NS_ERROR_FAILURE);
-    mLoadingImage = nullptr;
-  }
   if (mBrokenImage) {
     mBrokenImage->CancelAndForgetObserver(NS_ERROR_FAILURE);
     mBrokenImage = nullptr;

modules/libpref/init/StaticPrefList.yaml

@@ -1276,14 +1276,6 @@
   value: true
   mirror: always
 
-# If browser.display.show_image_placeholders is true then this controls whether
-# the loading image placeholder and border is shown or not.
-# TODO(emilio): Consider removing some or most of these image prefs.
-- name: browser.display.show_loading_image_placeholder
-  type: bool
-  value: false
-  mirror: always
-
 # Whether we should always enable focus rings after focus was moved by keyboard.
 #
 # This behavior matches both historical and GTK / Windows focus behavior.