Reland "[LCP] Add animated image support"

This is a reland of b7d510c06e0436cfb4bd7260175cd460b949225c

Original change's description:
> [LCP] Add animated image support
>
> This CL adds support for better handling of animated images in LCP:
> * A new attribute is exposing the first animated frame's paint time
> (behind a flag).
> * `startTime` is not changed.
> * The PageLoadMetrics reported for LCP are set to that first frame paint
> time for animated images (behind another flag).
> * Entries are not emitted until the image is loaded.
>
> Relevant spec issue:
> w3c/largest-contentful-paint#83
>
> Change-Id: I6bb01eacb4f200f9c032ffcfcd9a1a41126a7773
> Bug: 1260953
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3226157
> Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
> Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#935133}

Bug: 1260953
Change-Id: I5eaaf0cfd1daa7fb905e68aed4994cb931dc7750
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3247449
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#935635}
NOKEYCHECK=True
GitOrigin-RevId: 2ebdf0d26ea55271942e6b5ed585a08016a3e80b

blink/common/features.cc

@@ -1101,5 +1101,10 @@ const base::Feature kDeprecationWillLogToConsole{
 const base::Feature kDeprecationWillLogToDevToolsIssue{
     "DeprecationWillLogToDevToolsIssue", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables reporting and web-exposure (respectively) of the time the first frame
+// of an animated image was painted.
+const base::Feature kLCPAnimatedImagesReporting{
+    "LCPAnimatedImagesReporting", base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace features
 }  // namespace blink

blink/public/common/features.h

@@ -518,6 +518,8 @@ BLINK_COMMON_EXPORT extern const base::Feature kDeprecationWillLogToConsole;
 BLINK_COMMON_EXPORT extern const base::Feature
     kDeprecationWillLogToDevToolsIssue;
 
+BLINK_COMMON_EXPORT extern const base::Feature kLCPAnimatedImagesReporting;
+
 }  // namespace features
 }  // namespace blink
 

blink/renderer/core/loader/resource/image_resource_content.cc

@@ -626,7 +626,12 @@ ResourceStatus ImageResourceContent::GetContentStatus() const {
   return content_status_;
 }
 
-// TODO(hiroshige): Consider removing the following methods, or stoping
+bool ImageResourceContent::IsAnimatedImageWithPaintedFirstFrame() const {
+  return (image_ && !image_->IsNull() && image_->MaybeAnimated() &&
+          image_->CurrentFrameIsComplete());
+}
+
+// TODO(hiroshige): Consider removing the following methods, or stopping
 // redirecting to ImageResource.
 const KURL& ImageResourceContent::Url() const {
   return info_->Url();

blink/renderer/core/loader/resource/image_resource_content.h

@@ -109,6 +109,7 @@ class CORE_EXPORT ImageResourceContent final
   bool IsLoading() const;
   bool ErrorOccurred() const;
   bool LoadFailedOrCanceled() const;
+  bool IsAnimatedImageWithPaintedFirstFrame() const;
 
   // Redirecting methods to Resource.
   const KURL& Url() const;

blink/renderer/core/paint/image_paint_timing_detector.cc

@@ -53,6 +53,11 @@ uint64_t DownScaleIfIntrinsicSizeIsSmaller(
   return visual_size;
 }
 
+bool ShouldReportAnimatedImages() {
+  return (RuntimeEnabledFeatures::LCPAnimatedImagesWebExposedEnabled() ||
+          base::FeatureList::IsEnabled(features::kLCPAnimatedImagesReporting));
+}
+
 }  // namespace
 
 static bool LargeImageFirst(const base::WeakPtr<ImageRecord>& a,
@@ -101,6 +106,7 @@ void ImagePaintTimingDetector::ReportCandidateToTrace(
   DCHECK(!largest_image_record.paint_time.is_null());
   auto value = std::make_unique<TracedValue>();
   PopulateTraceValue(*value, largest_image_record);
+  // TODO(yoav): Report first animated frame times as well.
   TRACE_EVENT_MARK_WITH_TIMESTAMP2("loading", "LargestImagePaint::Candidate",
                                    largest_image_record.paint_time, "data",
                                    std::move(value), "frame",
@@ -123,20 +129,29 @@ void ImagePaintTimingDetector::ReportNoCandidateToTrace() {
 ImageRecord* ImagePaintTimingDetector::UpdateCandidate() {
   ImageRecord* largest_image_record =
       records_manager_.FindLargestPaintCandidate();
-  const base::TimeTicks time = largest_image_record
-                                   ? largest_image_record->paint_time
-                                   : base::TimeTicks();
+  base::TimeTicks time = largest_image_record ? largest_image_record->paint_time
+                                              : base::TimeTicks();
+  // This doesn't use ShouldReportAnimatedImages(), as it should only update the
+  // record when the base::Feature is enabled, regardless of the runtime-enabled
+  // flag.
+  if (base::FeatureList::IsEnabled(features::kLCPAnimatedImagesReporting) &&
+      largest_image_record &&
+      !largest_image_record->first_animated_frame_time.is_null()) {
+    time = largest_image_record->first_animated_frame_time;
+  }
   const uint64_t size =
       largest_image_record ? largest_image_record->first_size : 0;
   PaintTimingDetector& detector = frame_view_->GetPaintTimingDetector();
+  // Calling NotifyIfChangedLargestImagePaint only has an impact on
+  // PageLoadMetrics, and not on the web exposed metrics.
+  //
   // Two different candidates are rare to have the same time and size.
   // So when they are unchanged, the candidate is considered unchanged.
   bool changed = detector.NotifyIfChangedLargestImagePaint(
       time, size, records_manager_.LargestRemovedImagePaintTime(),
       records_manager_.LargestRemovedImageSize());
   if (changed) {
-    if (!time.is_null()) {
-      DCHECK(largest_image_record->loaded);
+    if (!time.is_null() && largest_image_record->loaded) {
       ReportCandidateToTrace(*largest_image_record);
     } else {
       ReportNoCandidateToTrace();
@@ -166,7 +181,7 @@ void ImagePaintTimingDetector::NotifyImageRemoved(
     const LayoutObject& object,
     const ImageResourceContent* cached_image) {
   RecordId record_id = std::make_pair(&object, cached_image);
-  records_manager_.RemoveImageFinishedRecord(record_id);
+  records_manager_.RemoveImageTimeRecords(record_id);
   records_manager_.RemoveInvisibleRecordIfNeeded(record_id);
   if (!records_manager_.IsRecordedVisibleImage(record_id))
     return;
@@ -215,7 +230,13 @@ void ImageRecordsManager::AssignPaintTimeToRegisteredQueuedRecords(
     }
     if (record->frame_index > last_queued_frame_index)
       break;
-    record->paint_time = timestamp;
+    if (record->loaded) {
+      record->paint_time = timestamp;
+    }
+    if (record->queue_animated_paint) {
+      record->first_animated_frame_time = timestamp;
+      record->queue_animated_paint = false;
+    }
     images_queued_for_paint_time_.pop_front();
   }
 }
@@ -228,6 +249,7 @@ void ImagePaintTimingDetector::RecordImage(
     const StyleFetchedImage* style_image,
     const IntRect& image_border) {
   Node* node = object.GetNode();
+
   if (!node)
     return;
 
@@ -269,25 +291,28 @@ void ImagePaintTimingDetector::RecordImage(
     return;
   }
 
-  if (is_recorded_visible_image &&
-      !records_manager_.IsVisibleImageLoaded(record_id) &&
-      cached_image.IsLoaded()) {
-    records_manager_.OnImageLoaded(record_id, frame_index_, style_image);
-    need_update_timing_at_frame_end_ = true;
-    if (absl::optional<PaintTimingVisualizer>& visualizer =
-            frame_view_->GetPaintTimingDetector().Visualizer()) {
-      FloatRect mapped_visual_rect =
-          frame_view_->GetPaintTimingDetector().CalculateVisualRect(
-              image_border, current_paint_chunk_properties);
-      visualizer->DumpImageDebuggingRect(object, mapped_visual_rect,
-                                         cached_image);
+  if (is_recorded_visible_image) {
+    if (ShouldReportAnimatedImages() &&
+        cached_image.IsAnimatedImageWithPaintedFirstFrame()) {
+      need_update_timing_at_frame_end_ |=
+          records_manager_.OnFirstAnimatedFramePainted(record_id, frame_index_);
+    }
+    if (!records_manager_.IsVisibleImageLoaded(record_id) &&
+        cached_image.IsLoaded()) {
+      records_manager_.OnImageLoaded(record_id, frame_index_, style_image);
+      need_update_timing_at_frame_end_ = true;
+      if (absl::optional<PaintTimingVisualizer>& visualizer =
+              frame_view_->GetPaintTimingDetector().Visualizer()) {
+        FloatRect mapped_visual_rect =
+            frame_view_->GetPaintTimingDetector().CalculateVisualRect(
+                image_border, current_paint_chunk_properties);
+        visualizer->DumpImageDebuggingRect(object, mapped_visual_rect,
+                                           cached_image);
+      }
     }
     return;
   }
 
-  if (is_recorded_visible_image)
-    return;
-
   FloatRect mapped_visual_rect =
       frame_view_->GetPaintTimingDetector().CalculateVisualRect(
           image_border, current_paint_chunk_properties);
@@ -299,6 +324,11 @@ void ImagePaintTimingDetector::RecordImage(
   } else {
     records_manager_.RecordVisible(record_id, rect_size, image_border,
                                    mapped_visual_rect);
+    if (ShouldReportAnimatedImages() &&
+        cached_image.IsAnimatedImageWithPaintedFirstFrame()) {
+      need_update_timing_at_frame_end_ |=
+          records_manager_.OnFirstAnimatedFramePainted(record_id, frame_index_);
+    }
     if (cached_image.IsLoaded()) {
       records_manager_.OnImageLoaded(record_id, frame_index_, style_image);
       need_update_timing_at_frame_end_ = true;
@@ -366,6 +396,19 @@ void ImagePaintTimingDetector::ReportLargestIgnoredImage() {
 ImageRecordsManager::ImageRecordsManager(LocalFrameView* frame_view)
     : size_ordered_set_(&LargeImageFirst), frame_view_(frame_view) {}
 
+bool ImageRecordsManager::OnFirstAnimatedFramePainted(
+    const RecordId& record_id,
+    unsigned current_frame_index) {
+  base::WeakPtr<ImageRecord> record = FindVisibleRecord(record_id);
+  DCHECK(record);
+  if (record->first_animated_frame_time.is_null()) {
+    record->queue_animated_paint = true;
+    QueueToMeasurePaintTime(record, current_frame_index);
+    return true;
+  }
+  return false;
+}
+
 void ImageRecordsManager::OnImageLoaded(const RecordId& record_id,
                                         unsigned current_frame_index,
                                         const StyleFetchedImage* style_image) {

blink/renderer/core/paint/image_paint_timing_detector.h

@@ -56,7 +56,10 @@ class ImageRecord : public base::SupportsWeakPtr<ImageRecord> {
   // The time of the first paint after fully loaded. 0 means not painted yet.
   base::TimeTicks paint_time = base::TimeTicks();
   base::TimeTicks load_time = base::TimeTicks();
+  base::TimeTicks first_animated_frame_time = base::TimeTicks();
   bool loaded = false;
+  // An animated frame is queued for paint timing.
+  bool queue_animated_paint = false;
   // LCP rect information, only populated when tracing is enabled.
   std::unique_ptr<LCPRectInfo> lcp_rect_info_;
 };
@@ -88,7 +91,7 @@ class CORE_EXPORT ImageRecordsManager {
     invisible_images_.erase(record_id);
   }
 
-  inline void RemoveImageFinishedRecord(const RecordId& record_id) {
+  inline void RemoveImageTimeRecords(const RecordId& record_id) {
     image_finished_times_.erase(record_id);
   }
 
@@ -133,14 +136,17 @@ class CORE_EXPORT ImageRecordsManager {
     // not currently the case. If we plumb some information from
     // ImageResourceContent we may be able to ensure that this call does not
     // require the Contains() check, which would save time.
-    if (!image_finished_times_.Contains(record_id))
+    if (!image_finished_times_.Contains(record_id)) {
       image_finished_times_.insert(record_id, base::TimeTicks::Now());
+    }
   }
 
   inline bool IsVisibleImageLoaded(const RecordId& record_id) const {
     DCHECK(visible_images_.Contains(record_id));
     return visible_images_.at(record_id)->loaded;
   }
+  bool OnFirstAnimatedFramePainted(const RecordId&,
+                                   unsigned current_frame_index);
   void OnImageLoaded(const RecordId&,
                      unsigned current_frame_index,
                      const StyleFetchedImage*);

blink/renderer/core/paint/largest_contentful_paint_calculator.cc

@@ -22,7 +22,7 @@ LargestContentfulPaintCalculator::LargestContentfulPaintCalculator(
     WindowPerformance* window_performance)
     : window_performance_(window_performance) {}
 
-void LargestContentfulPaintCalculator::UpdateLargestContentPaintIfNeeded(
+void LargestContentfulPaintCalculator::UpdateLargestContentfulPaintIfNeeded(
     const TextRecord* largest_text,
     const ImageRecord* largest_image) {
   uint64_t text_size = largest_text ? largest_text->first_size : 0u;
@@ -72,9 +72,12 @@ void LargestContentfulPaintCalculator::UpdateLargestContentfulImage(
       image_element ? image_element->GetIdAttribute() : AtomicString();
   window_performance_->OnLargestContentfulPaintUpdated(
       expose_paint_time_to_api ? largest_image->paint_time : base::TimeTicks(),
-      largest_image->first_size, largest_image->load_time, image_id, image_url,
-      image_element);
+      largest_image->first_size, largest_image->load_time,
+      expose_paint_time_to_api ? largest_image->first_animated_frame_time
+                               : base::TimeTicks(),
+      image_id, image_url, image_element);
 
+  // TODO: update trace value with animated frame data
   if (LocalDOMWindow* window = window_performance_->DomWindow()) {
     TRACE_EVENT_MARK_WITH_TIMESTAMP2(kTraceCategories, kLCPCandidate,
                                      largest_image->paint_time, "data",
@@ -100,7 +103,7 @@ void LargestContentfulPaintCalculator::UpdateLargestContentfulText(
       text_element ? text_element->GetIdAttribute() : AtomicString();
   window_performance_->OnLargestContentfulPaintUpdated(
       largest_text.paint_time, largest_text.first_size, base::TimeTicks(),
-      text_id, g_empty_string, text_element);
+      base::TimeTicks(), text_id, g_empty_string, text_element);
 
   if (LocalDOMWindow* window = window_performance_->DomWindow()) {
     TRACE_EVENT_MARK_WITH_TIMESTAMP2(kTraceCategories, kLCPCandidate,

blink/renderer/core/paint/largest_contentful_paint_calculator.h

@@ -23,8 +23,8 @@ class CORE_EXPORT LargestContentfulPaintCalculator final
   LargestContentfulPaintCalculator& operator=(
       const LargestContentfulPaintCalculator&) = delete;
 
-  void UpdateLargestContentPaintIfNeeded(const TextRecord* largest_text,
-                                         const ImageRecord* largest_image);
+  void UpdateLargestContentfulPaintIfNeeded(const TextRecord* largest_text,
+                                            const ImageRecord* largest_image);
 
   void Trace(Visitor* visitor) const;
 

blink/renderer/core/paint/paint_timing_detector.cc

@@ -33,6 +33,7 @@
 #include "third_party/blink/renderer/platform/graphics/paint/property_tree_state.h"
 #include "third_party/blink/renderer/platform/graphics/paint/scoped_paint_chunk_properties.h"
 #include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 
@@ -122,14 +123,23 @@ void PaintTimingDetector::NotifyBackgroundImagePaint(
   LocalFrameView* frame_view = object->GetFrameView();
   if (!frame_view)
     return;
-  PaintTimingDetector& detector = frame_view->GetPaintTimingDetector();
-  if (!detector.GetImagePaintTimingDetector())
+
+  ImagePaintTimingDetector* detector =
+      frame_view->GetPaintTimingDetector().GetImagePaintTimingDetector();
+  if (!detector)
     return;
+
   if (!IsBackgroundImageContentful(*object, image))
     return;
-  detector.GetImagePaintTimingDetector()->RecordImage(
-      *object, image.Size(), *style_image.CachedImage(),
-      current_paint_chunk_properties, &style_image, image_border);
+
+  ImageResourceContent* cached_image = style_image.CachedImage();
+  DCHECK(cached_image);
+  // TODO(yoav): |image| and |cached_image.GetImage()| are not the same here in
+  // the case of SVGs. Figure out why and if we can remove this footgun.
+
+  detector->RecordImage(*object, image.Size(), *cached_image,
+                        current_paint_chunk_properties, &style_image,
+                        image_border);
 }
 
 // static
@@ -144,12 +154,13 @@ void PaintTimingDetector::NotifyImagePaint(
   LocalFrameView* frame_view = object.GetFrameView();
   if (!frame_view)
     return;
-  PaintTimingDetector& detector = frame_view->GetPaintTimingDetector();
-  if (!detector.GetImagePaintTimingDetector())
+  ImagePaintTimingDetector* detector =
+      frame_view->GetPaintTimingDetector().GetImagePaintTimingDetector();
+  if (!detector)
     return;
-  detector.GetImagePaintTimingDetector()->RecordImage(
-      object, intrinsic_size, cached_image, current_paint_chunk_properties,
-      nullptr, image_border);
+
+  detector->RecordImage(object, intrinsic_size, cached_image,
+                        current_paint_chunk_properties, nullptr, image_border);
 }
 
 void PaintTimingDetector::NotifyImageFinished(
@@ -386,8 +397,8 @@ void PaintTimingDetector::UpdateLargestContentfulPaintCandidate() {
     largest_image_record = image_timing_detector->UpdateCandidate();
   }
 
-  lcp_calculator->UpdateLargestContentPaintIfNeeded(largest_text_record,
-                                                    largest_image_record);
+  lcp_calculator->UpdateLargestContentfulPaintIfNeeded(largest_text_record,
+                                                       largest_image_record);
 }
 
 void PaintTimingDetector::ReportIgnoredContent() {

blink/renderer/core/timing/largest_contentful_paint.cc

@@ -11,17 +11,20 @@
 
 namespace blink {
 
-LargestContentfulPaint::LargestContentfulPaint(double start_time,
-                                               base::TimeDelta render_time,
-                                               uint64_t size,
-                                               base::TimeDelta load_time,
-                                               const AtomicString& id,
-                                               const String& url,
-                                               Element* element)
+LargestContentfulPaint::LargestContentfulPaint(
+    double start_time,
+    base::TimeDelta render_time,
+    uint64_t size,
+    base::TimeDelta load_time,
+    base::TimeDelta first_animated_frame_time,
+    const AtomicString& id,
+    const String& url,
+    Element* element)
     : PerformanceEntry(g_empty_atom, start_time, start_time),
       size_(size),
       render_time_(render_time),
       load_time_(load_time),
+      first_animated_frame_time_(first_animated_frame_time),
       id_(id),
       url_(url),
       element_(element) {}
@@ -53,6 +56,8 @@ void LargestContentfulPaint::BuildJSONValue(V8ObjectBuilder& builder) const {
   builder.Add("size", size_);
   builder.Add("renderTime", render_time_.InMillisecondsF());
   builder.Add("loadTime", load_time_.InMillisecondsF());
+  builder.Add("firstAnimatedFrameTime",
+              first_animated_frame_time_.InMillisecondsF());
   builder.Add("id", id_);
   builder.Add("url", url_);
 }