Show console msg when explicitly lazyloaded img doesn't have dimensions

When lazyload is triggered by developer opt-in using loadin=lazy, img
dimensions are expected to be provided as well, since that will have
better performance. If dimensions is not provided via attributes or
inline style, this CL shows a console message and also records UseCounter

Bug: 973124
Change-Id: I159e5d4e1c0e32f32911163463dbcd600ff80f7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670063
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671954}

third_party/blink/public/mojom/web_feature/web_feature.mojom

@@ -2322,6 +2322,7 @@ enum WebFeature {
   kLazyLoadFrameLoadingAttributeLazy = 2933,
   kLazyLoadImageLoadingAttributeEager = 2934,
   kLazyLoadImageLoadingAttributeLazy = 2935,
+  kLazyLoadImageMissingDimensionsForLazy = 2936,
 
   // Add new features immediately above this line. Don't change assigned
   // numbers of any item, and don't reuse removed slots.

third_party/blink/renderer/core/css/resolver/element_style_resources.cc

@@ -223,8 +223,7 @@ void ElementStyleResources::LoadPendingImages(ComputedStyle* style) {
                                  image_request_optimization);
             if (new_image && new_image->IsLazyloadPossiblyDeferred()) {
               LazyLoadImageObserver::StartMonitoring(
-                  pseudo_element_ ? pseudo_element_ : element_.Get(),
-                  false /* is_for_intervention */);
+                  pseudo_element_ ? pseudo_element_ : element_.Get());
             }
             background_layer->SetImage(new_image);
           }

third_party/blink/renderer/core/html/lazy_load_image_observer.cc

@@ -21,6 +21,7 @@
 #include "third_party/blink/renderer/core/intersection_observer/intersection_observer.h"
 #include "third_party/blink/renderer/core/intersection_observer/intersection_observer_entry.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
+#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
 
 namespace blink {
 
@@ -59,10 +60,10 @@ Document* GetRootDocumentOrNull(Element* element) {
 }  // namespace
 
 void LazyLoadImageObserver::StartMonitoring(Element* element,
-                                            bool is_for_intervention) {
+                                            DeferralMessage deferral_message) {
   if (Document* document = GetRootDocumentOrNull(element)) {
     document->EnsureLazyLoadImageObserver().StartMonitoringNearViewport(
-        document, element, is_for_intervention);
+        document, element, deferral_message);
   }
 }
 
@@ -97,7 +98,7 @@ LazyLoadImageObserver::LazyLoadImageObserver() = default;
 void LazyLoadImageObserver::StartMonitoringNearViewport(
     Document* root_document,
     Element* element,
-    bool is_for_intervention) {
+    DeferralMessage deferral_message) {
   DCHECK(RuntimeEnabledFeatures::LazyImageLoadingEnabled());
 
   if (!lazy_load_intersection_observer_) {
@@ -110,13 +111,26 @@ void LazyLoadImageObserver::StartMonitoringNearViewport(
   }
   lazy_load_intersection_observer_->observe(element);
 
-  if (is_for_intervention && !is_load_event_deferred_intervention_shown_) {
+  if (deferral_message == DeferralMessage::kLoadEventsDeferred &&
+      !is_load_event_deferred_intervention_shown_) {
     is_load_event_deferred_intervention_shown_ = true;
     root_document->AddConsoleMessage(ConsoleMessage::Create(
         mojom::ConsoleMessageSource::kIntervention,
         mojom::ConsoleMessageLevel::kInfo,
         "Images loaded lazily and replaced with placeholders. Load events are "
-        "deferred. See https://crbug.com/846170"));
+        "deferred. See https://crbug.com/954323"));
+  }
+  if (deferral_message == DeferralMessage::kMissingDimensionForLazy &&
+      !is_missing_dimension_intervention_shown_) {
+    is_missing_dimension_intervention_shown_ = true;
+    root_document->AddConsoleMessage(ConsoleMessage::Create(
+        mojom::ConsoleMessageSource::kIntervention,
+        mojom::ConsoleMessageLevel::kInfo,
+        "An <img> element was lazyloaded with loading=lazy, but had no "
+        "dimensions specified. Specifying dimensions improves performance. See "
+        "https://crbug.com/954323"));
+    UseCounter::Count(root_document,
+                      WebFeature::kLazyLoadImageMissingDimensionsForLazy);
   }
 }
 

third_party/blink/renderer/core/html/lazy_load_image_observer.h

@@ -21,6 +21,12 @@ class Visitor;
 class LazyLoadImageObserver final
     : public GarbageCollected<LazyLoadImageObserver> {
  public:
+  enum class DeferralMessage {
+    kNone,
+    kLoadEventsDeferred,
+    kMissingDimensionForLazy
+  };
+
   struct VisibleLoadTimeMetrics {
     // Keeps track of whether the image was initially intersecting the viewport.
     bool is_initially_intersecting = false;
@@ -35,7 +41,9 @@ class LazyLoadImageObserver final
 
   LazyLoadImageObserver();
 
-  static void StartMonitoring(Element*, bool is_for_intervention);
+  static void StartMonitoring(
+      Element*,
+      DeferralMessage deferral_message = DeferralMessage::kNone);
   static void StopMonitoring(Element*);
 
   static void StartTrackingVisibilityMetrics(HTMLImageElement*);
@@ -44,9 +52,7 @@ class LazyLoadImageObserver final
   void Trace(Visitor*);
 
  private:
-  void StartMonitoringNearViewport(Document*,
-                                   Element*,
-                                   bool is_for_intervention);
+  void StartMonitoringNearViewport(Document*, Element*, DeferralMessage);
   void LoadIfNearViewport(const HeapVector<Member<IntersectionObserverEntry>>&);
 
   void StartMonitoringVisibility(Document*, HTMLImageElement*);
@@ -64,6 +70,7 @@ class LazyLoadImageObserver final
 
   // Used to show the intervention console message one time only.
   bool is_load_event_deferred_intervention_shown_ = false;
+  bool is_missing_dimension_intervention_shown_ = false;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc

@@ -36,7 +36,12 @@ namespace {
 
 const char* kLazyLoadEventsDeferredMessage =
     "Images loaded lazily and replaced with placeholders. Load events are "
-    "deferred. See https://crbug.com/846170";
+    "deferred. See https://crbug.com/954323";
+
+const char* kLazyLoadMissingDimensionMessage =
+    "An <img> element was lazyloaded with loading=lazy, but had no dimensions "
+    "specified. Specifying dimensions improves performance. See "
+    "https://crbug.com/954323";
 
 Vector<char> ReadTestImage() {
   return test::ReadFromFile(test::CoreTestDataPath("notifications/500x500.png"))
@@ -88,6 +93,9 @@ class LazyLoadImagesSimTest : public ::testing::WithParamInterface<bool>,
     }
     EXPECT_TRUE(is_background_image_found);
     EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+    EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+    EXPECT_FALSE(GetDocument().IsUseCounted(
+        WebFeature::kLazyLoadImageMissingDimensionsForLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
         WebFeature::kLazyLoadImageLoadingAttributeLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -138,6 +146,9 @@ class LazyLoadImagesSimTest : public ::testing::WithParamInterface<bool>,
       }
     }
     EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+    EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+    EXPECT_FALSE(GetDocument().IsUseCounted(
+        WebFeature::kLazyLoadImageMissingDimensionsForLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
         WebFeature::kLazyLoadImageLoadingAttributeLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -178,6 +189,9 @@ class LazyLoadImagesSimTest : public ::testing::WithParamInterface<bool>,
     }
     EXPECT_EQ(is_lazyload_image_enabled,
               ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+    EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+    EXPECT_FALSE(GetDocument().IsUseCounted(
+        WebFeature::kLazyLoadImageMissingDimensionsForLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
         WebFeature::kLazyLoadImageLoadingAttributeLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -556,6 +570,19 @@ TEST_P(LazyLoadImagesParamsTest, NearViewport) {
   EXPECT_TRUE(ConsoleMessages().Contains("lazy onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("auto onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("unset onload"));
+
+  switch (std::get<LazyImageLoadingFeatureStatus>(GetParam())) {
+    case LazyImageLoadingFeatureStatus::kEnabledAutomatic:
+    case LazyImageLoadingFeatureStatus::
+        kEnabledAutomaticRestrictedAndDataSaverOn:
+      EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+      EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+      EXPECT_TRUE(GetDocument().IsUseCounted(
+          WebFeature::kLazyLoadImageMissingDimensionsForLazy));
+      break;
+    default:
+      break;
+  }
 }
 
 TEST_P(LazyLoadImagesParamsTest, FarFromViewport) {
@@ -709,6 +736,19 @@ TEST_P(LazyLoadImagesParamsTest, FarFromViewport) {
   EXPECT_TRUE(ConsoleMessages().Contains("lazy onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("auto onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("unset onload"));
+
+  switch (std::get<LazyImageLoadingFeatureStatus>(GetParam())) {
+    case LazyImageLoadingFeatureStatus::kEnabledAutomatic:
+    case LazyImageLoadingFeatureStatus::
+        kEnabledAutomaticRestrictedAndDataSaverOn:
+      EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+      EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+      EXPECT_TRUE(GetDocument().IsUseCounted(
+          WebFeature::kLazyLoadImageMissingDimensionsForLazy));
+      break;
+    default:
+      break;
+  }
 }
 
 INSTANTIATE_TEST_SUITE_P(
@@ -824,6 +864,9 @@ class LazyLoadAutomaticImagesTest : public SimTest {
     EXPECT_TRUE(ConsoleMessages().Contains("main body onload"));
     EXPECT_TRUE(ConsoleMessages().Contains("image onload"));
     EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+    EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+    EXPECT_FALSE(GetDocument().IsUseCounted(
+        WebFeature::kLazyLoadImageMissingDimensionsForLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
         WebFeature::kLazyLoadImageLoadingAttributeLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -849,6 +892,9 @@ class LazyLoadAutomaticImagesTest : public SimTest {
     EXPECT_TRUE(ConsoleMessages().Contains("main body onload"));
     EXPECT_TRUE(ConsoleMessages().Contains("image onload"));
     EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+    EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+    EXPECT_FALSE(GetDocument().IsUseCounted(
+        WebFeature::kLazyLoadImageMissingDimensionsForLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
         WebFeature::kLazyLoadImageLoadingAttributeLazy));
     EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -885,6 +931,9 @@ TEST_F(LazyLoadAutomaticImagesTest, AttributeChangedFromLazyToEager) {
   EXPECT_TRUE(ConsoleMessages().Contains("main body onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("image onload"));
   EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+  EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+  EXPECT_TRUE(GetDocument().IsUseCounted(
+      WebFeature::kLazyLoadImageMissingDimensionsForLazy));
   EXPECT_TRUE(GetDocument().IsUseCounted(
       WebFeature::kLazyLoadImageLoadingAttributeLazy));
   EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -913,6 +962,9 @@ TEST_F(LazyLoadAutomaticImagesTest, AttributeChangedFromAutoToEager) {
   EXPECT_TRUE(ConsoleMessages().Contains("main body onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("image onload"));
   EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+  EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+  EXPECT_FALSE(GetDocument().IsUseCounted(
+      WebFeature::kLazyLoadImageMissingDimensionsForLazy));
   EXPECT_FALSE(GetDocument().IsUseCounted(
       WebFeature::kLazyLoadImageLoadingAttributeLazy));
   EXPECT_FALSE(GetDocument().IsUseCounted(
@@ -941,6 +993,9 @@ TEST_F(LazyLoadAutomaticImagesTest, AttributeChangedFromUnsetToEager) {
   EXPECT_TRUE(ConsoleMessages().Contains("main body onload"));
   EXPECT_TRUE(ConsoleMessages().Contains("image onload"));
   EXPECT_TRUE(ConsoleMessages().Contains(kLazyLoadEventsDeferredMessage));
+  EXPECT_FALSE(ConsoleMessages().Contains(kLazyLoadMissingDimensionMessage));
+  EXPECT_FALSE(GetDocument().IsUseCounted(
+      WebFeature::kLazyLoadImageMissingDimensionsForLazy));
   EXPECT_FALSE(GetDocument().IsUseCounted(
       WebFeature::kLazyLoadImageLoadingAttributeLazy));
   EXPECT_FALSE(GetDocument().IsUseCounted(

third_party/blink/renderer/core/loader/image_loader.cc

@@ -502,9 +502,16 @@ void ImageLoader::UpdateImageState(ImageResourceContent* new_image_content) {
     image_complete_ = false;
     if (lazy_image_load_state_ == LazyImageLoadState::kDeferred) {
       if (auto* html_image = ToHTMLImageElementOrNull(GetElement())) {
-        LazyLoadImageObserver::StartMonitoring(
-            html_image,
-            GetLoadingAttrValue(*html_image) != LoadingAttrValue::kLazy);
+        using DeferralMessage = LazyLoadImageObserver::DeferralMessage;
+        LoadingAttrValue loading_attr = GetLoadingAttrValue(*html_image);
+        DCHECK(loading_attr != LoadingAttrValue::kEager);
+        auto deferral_message = DeferralMessage::kNone;
+        if (loading_attr == LoadingAttrValue::kAuto) {
+          deferral_message = DeferralMessage::kLoadEventsDeferred;
+        } else if (!was_fully_deferred_) {
+          deferral_message = DeferralMessage::kMissingDimensionForLazy;
+        }
+        LazyLoadImageObserver::StartMonitoring(html_image, deferral_message);
       }
     }
   }

tools/metrics/histograms/enums.xml

@@ -23718,6 +23718,7 @@ Called by update_net_error_codes.py.-->
   <int value="2933" label="LazyLoadFrameLoadingAttributeLazy"/>
   <int value="2934" label="LazyLoadImageLoadingAttributeEager"/>
   <int value="2935" label="LazyLoadImageLoadingAttributeLazy"/>
+  <int value="2936" label="LazyLoadImageMissingDimensionsForLazy"/>
 </enum>
 
 <enum name="FeaturePolicyAllowlistType">