Reland #2 "[LCP] Add animated image support"

This is a manual reland of
https://chromium-review.googlesource.com/c/chromium/src/+/3247449

The difference from the previous reland is that the browser tests now
include 2 separate timeouts and a double rAF, to ensure that the
presentation timestamp taken is far enough from both the time the first
frame is sent as well as from the time the second frame is sent.
More importantly, the test now actually is looking at the UKM metric,
rather than at the histogram.

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

Bug: 1260953
Change-Id: I34070bd90a74ed44281da63b547f13d9669f389b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3250690
Reviewed-by: Nicolás Peña Moreno <[email protected]>
Commit-Queue: Yoav Weiss <[email protected]>
Cr-Commit-Position: refs/heads/main@{#936516}
NOKEYCHECK=True
GitOrigin-RevId: 0955a5d72e1eb06e110395a376c8f254473818a6

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 gfx::Rect& 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()) {
-      gfx::RectF 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()) {
+        gfx::RectF 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;
-
   gfx::RectF 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

@@ -57,7 +57,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_;
 };
@@ -89,7 +92,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);
   }
 
@@ -134,14 +137,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, ToGfxSize(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, ToGfxSize(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(
@@ -385,8 +396,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_);
 }

blink/renderer/core/timing/largest_contentful_paint.h

@@ -22,6 +22,7 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry {
                          base::TimeDelta render_time,
                          uint64_t size,
                          base::TimeDelta load_time,
+                         base::TimeDelta first_animated_frame_time,
                          const AtomicString& id,
                          const String& url,
                          Element*);
@@ -35,6 +36,9 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry {
     return render_time_.InMillisecondsF();
   }
   DOMHighResTimeStamp loadTime() const { return load_time_.InMillisecondsF(); }
+  DOMHighResTimeStamp firstAnimatedFrameTime() const {
+    return first_animated_frame_time_.InMillisecondsF();
+  }
   const AtomicString& id() const { return id_; }
   const String& url() const { return url_; }
   Element* element() const;
@@ -47,6 +51,7 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry {
   uint64_t size_;
   base::TimeDelta render_time_;
   base::TimeDelta load_time_;
+  base::TimeDelta first_animated_frame_time_;
   AtomicString id_;
   String url_;
   WeakMember<Element> element_;