[LargestContentfulPaint] Replace responseEnd with loadTime

This CL replaces responseEnd with loadTime. Before, responseEnd would be
used and would be problematic for memory cached images. We change it to
loadTime, the time at which LayoutObject::ImageNotifyFinished is called.
This time has the advantage of being independent of the memory cache and
being after the image has been appended to the DOM, as shown by a test.

To implement loadTime, we use a new HashMap. This HashMap populates the
timestamps that will be used in ImageRecords. We cannot use the existing
data structures because this happens before we know whether the image
will be added to the set of visible or invisible images, etc.

Bug: 982046, 965505
Change-Id: I50d4f304a35b60409c58c54a5bd912e7d15825e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1710682
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680096}

third_party/blink/renderer/core/layout/layout_object.cc

@@ -3513,9 +3513,15 @@ void LayoutObject::ImageChanged(ImageResourceContent* image,
   ImageChanged(static_cast<WrappedImagePtr>(image), defer);
 }
 
-void LayoutObject::ImageNotifyFinished(ImageResourceContent*) {
+void LayoutObject::ImageNotifyFinished(ImageResourceContent* image) {
   if (AXObjectCache* cache = GetDocument().ExistingAXObjectCache())
     cache->ImageLoaded(this);
+
+  if (RuntimeEnabledFeatures::FirstContentfulPaintPlusPlusEnabled()) {
+    if (LocalFrameView* frame_view = GetFrameView()) {
+      frame_view->GetPaintTimingDetector().NotifyImageFinished(*this, image);
+    }
+  }
 }
 
 Element* LayoutObject::OffsetParent(const Element* base) const {

third_party/blink/renderer/core/paint/image_paint_timing_detector.cc

@@ -169,6 +169,7 @@ void ImagePaintTimingDetector::NotifyImageRemoved(
   if (!is_recording_)
     return;
   RecordId record_id = std::make_pair(&object, cached_image);
+  records_manager_.RemoveImageFinishedRecord(record_id);
   if (!records_manager_.IsRecordedVisibleImage(record_id))
     return;
   records_manager_.RemoveVisibleRecord(record_id);
@@ -280,12 +281,25 @@ void ImagePaintTimingDetector::RecordImage(
   }
 }
 
+void ImagePaintTimingDetector::NotifyImageFinished(
+    const LayoutObject& object,
+    const ImageResourceContent* cached_image) {
+  RecordId record_id = std::make_pair(&object, cached_image);
+  records_manager_.NotifyImageFinished(record_id);
+}
+
 ImageRecordsManager::ImageRecordsManager()
     : size_ordered_set_(&LargeImageFirst) {}
 
 void ImageRecordsManager::OnImageLoaded(const RecordId& record_id,
                                         unsigned current_frame_index) {
   base::WeakPtr<ImageRecord> record = FindVisibleRecord(record_id);
+  DCHECK(record);
+  // TODO(crbug.com/986891): some background images are not being tracked
+  // properly, so we cannot add a DCHECK that |image_finished_times| contains
+  // |record_id|. Once that bug is fixed, we should add that check, as otherwise
+  // we'll be exposing a loadTime of 0.
+  record->load_time = image_finished_times_.at(record_id);
   OnImageLoadedInternal(record, current_frame_index);
 }
 

third_party/blink/renderer/core/paint/image_paint_timing_detector.h

@@ -49,6 +49,7 @@ class ImageRecord : public base::SupportsWeakPtr<ImageRecord> {
   unsigned insertion_index;
   // 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();
   bool loaded = false;
 };
 
@@ -75,6 +76,10 @@ class CORE_EXPORT ImageRecordsManager {
     invisible_images_.erase(&object);
   }
 
+  inline void RemoveImageFinishedRecord(const RecordId& record_id) {
+    image_finished_times_.erase(record_id);
+  }
+
   inline void RemoveVisibleRecord(const RecordId& record_id) {
     base::WeakPtr<ImageRecord> record =
         visible_images_.find(record_id)->value->AsWeakPtr();
@@ -95,6 +100,16 @@ class CORE_EXPORT ImageRecordsManager {
     return invisible_images_.Contains(&object);
   }
 
+  void NotifyImageFinished(const RecordId& record_id) {
+    // TODO(npm): Ideally NotifyImageFinished() would only be called when the
+    // record has not yet been inserted in |image_finished_times_| but that's
+    // 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))
+      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;
@@ -156,6 +171,9 @@ class CORE_EXPORT ImageRecordsManager {
   // |ImageRecord|s waiting for paint time are stored in this queue
   // until they get a swap time.
   Deque<base::WeakPtr<ImageRecord>> images_queued_for_paint_time_;
+  // Map containing timestamps of when LayoutObject::ImageNotifyFinished is
+  // first called.
+  HashMap<RecordId, base::TimeTicks> image_finished_times_;
 
   DISALLOW_COPY_AND_ASSIGN(ImageRecordsManager);
 };
@@ -190,11 +208,7 @@ class CORE_EXPORT ImagePaintTimingDetector final
                    const IntSize& intrinsic_size,
                    const ImageResourceContent&,
                    const PropertyTreeState& current_paint_chunk_properties);
-  void RecordBackgroundImage(
-      const LayoutObject&,
-      const IntSize& intrinsic_size,
-      const ImageResourceContent& cached_image,
-      const PropertyTreeState& current_paint_chunk_properties);
+  void NotifyImageFinished(const LayoutObject&, const ImageResourceContent*);
   void OnPaintFinished();
   void LayoutObjectWillBeDestroyed(const LayoutObject&);
   void NotifyImageRemoved(const LayoutObject&, const ImageResourceContent*);

third_party/blink/renderer/core/paint/image_paint_timing_detector_test.cc

@@ -128,7 +128,10 @@ class ImagePaintTimingDetectorTest
                ->records_manager_.size_ordered_set_.size() +
            GetPaintTimingDetector()
                .GetImagePaintTimingDetector()
-               ->records_manager_.images_queued_for_paint_time_.size();
+               ->records_manager_.images_queued_for_paint_time_.size() +
+           GetPaintTimingDetector()
+               .GetImagePaintTimingDetector()
+               ->records_manager_.image_finished_times_.size();
   }
 
   size_t CountChildFrameRecords() {
@@ -629,7 +632,7 @@ TEST_F(ImagePaintTimingDetectorTest,
   )HTML");
   SetImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
-  EXPECT_EQ(ContainerTotalSize(), 2u);
+  EXPECT_EQ(ContainerTotalSize(), 3u);
 
   GetDocument().getElementById("parent")->RemoveChild(
       GetDocument().getElementById("target"));
@@ -657,7 +660,7 @@ TEST_F(ImagePaintTimingDetectorTest,
   )HTML");
   SetImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
-  EXPECT_EQ(ContainerTotalSize(), 1u);
+  EXPECT_EQ(ContainerTotalSize(), 2u);
   EXPECT_EQ(CountInvisibleRecords(), 1u);
 
   GetDocument().body()->RemoveChild(GetDocument().getElementById("parent"));
@@ -680,7 +683,7 @@ TEST_F(ImagePaintTimingDetectorTest,
     </div>
   )HTML");
   UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
-  EXPECT_EQ(ContainerTotalSize(), 2u);
+  EXPECT_EQ(ContainerTotalSize(), 3u);
 
   GetDocument().getElementById("parent")->RemoveChild(
       GetDocument().getElementById("target"));
@@ -696,7 +699,7 @@ TEST_F(ImagePaintTimingDetectorTest,
   )HTML");
   SetImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhases();
-  EXPECT_EQ(ContainerTotalSize(), 3u);
+  EXPECT_EQ(ContainerTotalSize(), 4u);
 
   GetDocument().getElementById("parent")->RemoveChild(
       GetDocument().getElementById("target"));

third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.cc

@@ -21,6 +21,7 @@ void LargestContentfulPaintCalculator::OnLargestImageUpdated(
     largest_image_->first_size = largest_image->first_size;
     largest_image_->paint_time = largest_image->paint_time;
     largest_image_->cached_image = largest_image->cached_image;
+    largest_image_->load_time = largest_image->load_time;
   }
 
   if (LargestImageSize() > LargestTextSize()) {
@@ -95,7 +96,7 @@ void LargestContentfulPaintCalculator::OnLargestContentfulPaintUpdated(
         image_element ? image_element->GetIdAttribute() : AtomicString();
     window_performance_->OnLargestContentfulPaintUpdated(
         largest_image_->paint_time, largest_image_->first_size,
-        cached_image->LoadResponseEnd(), image_id, image_url, image_element);
+        largest_image_->load_time, image_id, image_url, image_element);
   } else {
     Node* text_node = DOMNodeIds::NodeForId(largest_text_->node_id);
     // |text_node| could be null and |largest_text_| should be ignored in this

third_party/blink/renderer/core/paint/paint_timing_detector.cc

@@ -118,6 +118,13 @@ void PaintTimingDetector::NotifyImagePaint(
       object, intrinsic_size, *cached_image, current_paint_chunk_properties);
 }
 
+void PaintTimingDetector::NotifyImageFinished(
+    const LayoutObject& object,
+    const ImageResourceContent* cached_image) {
+  if (image_paint_timing_detector_)
+    image_paint_timing_detector_->NotifyImageFinished(object, cached_image);
+}
+
 void PaintTimingDetector::LayoutObjectWillBeDestroyed(
     const LayoutObject& object) {
   if (text_paint_timing_detector_)

third_party/blink/renderer/core/paint/paint_timing_detector.h

@@ -50,6 +50,7 @@ class CORE_EXPORT PaintTimingDetector
       const PropertyTreeState& current_paint_chunk_properties);
   inline static void NotifyTextPaint(const IntRect& text_visual_rect);
 
+  void NotifyImageFinished(const LayoutObject&, const ImageResourceContent*);
   void LayoutObjectWillBeDestroyed(const LayoutObject&);
   void NotifyImageRemoved(const LayoutObject&, const ImageResourceContent*);
   void NotifyPaintFinished();

third_party/blink/renderer/core/timing/largest_contentful_paint.cc

@@ -12,14 +12,14 @@ namespace blink {
 
 LargestContentfulPaint::LargestContentfulPaint(double render_time,
                                                uint64_t size,
-                                               double response_end,
+                                               double load_time,
                                                const AtomicString& id,
                                                const String& url,
                                                Element* element)
     : PerformanceEntry(g_empty_atom, 0, 0),
       size_(size),
       render_time_(render_time),
-      response_end_(response_end),
+      load_time_(load_time),
       id_(id),
       url_(url),
       element_(element) {}
@@ -44,7 +44,8 @@ Element* LargestContentfulPaint::element() const {
 void LargestContentfulPaint::BuildJSONValue(V8ObjectBuilder& builder) const {
   PerformanceEntry::BuildJSONValue(builder);
   builder.Add("size", size_);
-  builder.Add("responseEnd", response_end_);
+  builder.Add("renderTime", render_time_);
+  builder.Add("loadTime", load_time_);
   builder.Add("id", id_);
   builder.Add("url", url_);
   builder.Add("element", element_);

third_party/blink/renderer/core/timing/largest_contentful_paint.h

@@ -19,7 +19,7 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry {
  public:
   LargestContentfulPaint(double render_time,
                          uint64_t size,
-                         double response_end,
+                         double load_time,
                          const AtomicString& id,
                          const String& url,
                          Element*);
@@ -30,7 +30,7 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry {
 
   uint64_t size() const { return size_; }
   DOMHighResTimeStamp renderTime() const { return render_time_; }
-  DOMHighResTimeStamp responseEnd() const { return response_end_; }
+  DOMHighResTimeStamp loadTime() const { return load_time_; }
   const AtomicString& id() const { return id_; }
   const String& url() const { return url_; }
   Element* element() const;
@@ -42,7 +42,7 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry {
 
   uint64_t size_;
   DOMHighResTimeStamp render_time_;
-  DOMHighResTimeStamp response_end_;
+  DOMHighResTimeStamp load_time_;
   AtomicString id_;
   String url_;
   WeakMember<Element> element_;

third_party/blink/renderer/core/timing/largest_contentful_paint.idl

@@ -6,7 +6,7 @@
 [Exposed=Window, RuntimeEnabled=LargestContentfulPaint]
 interface LargestContentfulPaint : PerformanceEntry {
     readonly attribute DOMHighResTimeStamp renderTime;
-    readonly attribute DOMHighResTimeStamp responseEnd;
+    readonly attribute DOMHighResTimeStamp loadTime;
     readonly attribute unsigned long long size;
     readonly attribute DOMString id;
     readonly attribute DOMString url;

third_party/blink/renderer/core/timing/window_performance.cc

@@ -442,13 +442,13 @@ void WindowPerformance::AddLayoutJankFraction(double jank_fraction,
 void WindowPerformance::OnLargestContentfulPaintUpdated(
     base::TimeTicks paint_time,
     uint64_t paint_size,
-    base::TimeTicks response_end,
+    base::TimeTicks load_time,
     const AtomicString& id,
     const String& url,
     Element* element) {
   auto* entry = MakeGarbageCollected<LargestContentfulPaint>(
       MonotonicTimeToDOMHighResTimeStamp(paint_time), paint_size,
-      MonotonicTimeToDOMHighResTimeStamp(response_end), id, url, element);
+      MonotonicTimeToDOMHighResTimeStamp(load_time), id, url, element);
   if (HasObserverFor(PerformanceEntry::kLargestContentfulPaint))
     NotifyObserversOfEntry(*entry);
   AddLargestContentfulPaint(entry);

third_party/blink/renderer/core/timing/window_performance.h

@@ -90,7 +90,7 @@ class CORE_EXPORT WindowPerformance final : public Performance,
 
   void OnLargestContentfulPaintUpdated(base::TimeTicks paint_time,
                                        uint64_t paint_size,
-                                       base::TimeTicks response_end,
+                                       base::TimeTicks load_time,
                                        const AtomicString& id,
                                        const String& url,
                                        Element*);

third_party/blink/web_tests/external/wpt/largest-contentful-paint/cross-origin-image.sub.html

@@ -9,6 +9,7 @@
     if (!window.LargestContentfulPaint) {
       assert_unreached("LargestContentfulPaint is not implemented");
     }
+    const beforeLoad = performance.now();
     const observer = new PerformanceObserver(
       t.step_func_done(function(entryList) {
         assert_equals(entryList.getEntries().length, 1);
@@ -22,8 +23,8 @@
         assert_equals(entry.id, 'image_id');
         const pathname = 'http://{{domains[www]}}:{{ports[http][1]}}/images/blue.png';
         assert_equals(entry.url, pathname);
-        assert_equals(entry.responseEnd,
-            performance.getEntriesByName(pathname, 'resource')[0].responseEnd);
+        assert_greater_than_equal(entry.loadTime, beforeLoad);
+        assert_less_than(entry.loadTime, performance.now());
         assert_equals(entry.element, document.getElementById('image_id'));
       })
     );

third_party/blink/web_tests/external/wpt/largest-contentful-paint/loadTime-after-appendChild.html

+<!DOCTYPE HTML>
+<meta charset=utf-8>
+<title>Largest Contentful Paint: delayed appended image.</title>
+<body>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+  async_test(function (t) {
+    if (!window.LargestContentfulPaint) {
+      assert_unreached("LargestContentfulPaint is not implemented");
+    }
+    let beforeLoad;
+    const observer = new PerformanceObserver(
+      t.step_func_done(entryList => {
+        assert_equals(entryList.getEntries().length, 1);
+        const entry = entryList.getEntries()[0];
+        assert_equals(entry.entryType, 'largest-contentful-paint');
+        assert_equals(entry.startTime, 0);
+        assert_equals(entry.duration, 0);
+        assert_equals(entry.url, window.location.origin + '/images/black-rectangle.png');
+        assert_greater_than(entry.renderTime, entry.loadTime,
+          'The image render time should occur after it is appended to the div.');
+        assert_greater_than(entry.loadTime, beforeLoad,
+          'The image load timestamp should occur after script starts running.');
+        assert_less_than(entry.renderTime, performance.now(),
+          'Image render time should be before the observer callback is executed.')
+      })
+    );
+    observer.observe({type: 'largest-contentful-paint', buffered: true});
+    const img = document.createElement('img');
+    img.src = '/images/black-rectangle.png';
+    t.step_timeout(() => {
+      beforeLoad = performance.now();
+      document.getElementById('image_div').appendChild(img);
+    }, 200)
+  }, 'Image loadTime occurs after appendChild is called.');
+</script>
+<div id='image_div'></div>
+</body>

third_party/blink/web_tests/external/wpt/largest-contentful-paint/observe-image.html

@@ -9,7 +9,7 @@
     if (!window.LargestContentfulPaint) {
       assert_unreached("LargestContentfulPaint is not implemented");
     }
-    let beforeRender = performance.now();
+    const beforeRender = performance.now();
     const observer = new PerformanceObserver(
       t.step_func_done(function(entryList) {
         assert_equals(entryList.getEntries().length, 1);
@@ -28,8 +28,10 @@
         const index = window.location.href.lastIndexOf('/') - 25;
         const pathname = window.location.href.substring(0, index) + '/images/blue.png';
         assert_equals(entry.url, pathname);
-        assert_equals(entry.responseEnd,
-            performance.getEntriesByName(pathname, 'resource')[0].responseEnd);
+        assert_greater_than(entry.loadTime, beforeRender,
+          'The load timestamp should occur after script starts running.');
+        assert_less_than(entry.loadTime, entry.renderTime,
+          'The load timestamp should occur before the render timestamp.')
         assert_equals(entry.element, document.getElementById('image_id'));
       })
     );

third_party/blink/web_tests/external/wpt/largest-contentful-paint/observe-text.html

@@ -10,11 +10,11 @@ p {
 }
 </style>
 <script>
-  let beforeRender;
   async_test(function (t) {
     if (!window.LargestContentfulPaint) {
       assert_unreached("LargestContentfulPaint is not implemented");
     }
+    let beforeRender;
     const observer = new PerformanceObserver(
       t.step_func_done(function(entryList) {
         assert_equals(entryList.getEntries().length, 1);
@@ -28,7 +28,7 @@ p {
         // Width of at least 100 px.
         // TODO: find a good way to bound text width.
         assert_greater_than_equal(entry.size, 1200);
-        assert_equals(entry.responseEnd, 0);
+        assert_equals(entry.loadTime, 0);
         assert_equals(entry.id, 'my_text');
         assert_equals(entry.url, '');
         assert_equals(entry.element, document.getElementById('my_text'));

third_party/blink/web_tests/external/wpt/largest-contentful-paint/repeated-image.html

+<!DOCTYPE HTML>
+<meta charset=utf-8>
+<title>Largest Contentful Paint: repeated image.</title>
+<style>
+  #image_id {
+    width: 10px;
+    height: 10px;
+  }
+</style>
+<body>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+  async_test(function (t) {
+    if (!window.LargestContentfulPaint) {
+      assert_unreached("LargestContentfulPaint is not implemented");
+    }
+    const beforeFirstLoad = performance.now();
+    let firstCallback = true;
+    const path = window.location.origin + '/images/black-rectangle.png';
+    let beforeSecondLoad;
+    const observer = new PerformanceObserver(
+      t.step_func(entryList => {
+        assert_equals(entryList.getEntries().length, 1);
+        const entry = entryList.getEntries()[0];
+        assert_equals(entry.entryType, 'largest-contentful-paint');
+        assert_equals(entry.startTime, 0);
+        assert_equals(entry.duration, 0);
+        assert_equals(entry.url, path);
+        assert_less_than(entry.renderTime, performance.now(),
+          'Image render time should be before the observer callback is executed.')
+        if (firstCallback) {
+          assert_equals(entry.id, 'image_id');
+          assert_greater_than(entry.renderTime, entry.loadTime,
+            'The first image render time should occur after its load time.');
+          assert_greater_than(entry.loadTime, beforeFirstLoad,
+            'The first image load timestamp should occur after script starts running.');
+          // Image is shrunk to be 10 x 10.
+          assert_equals(entry.size, 100);
+          const img = document.createElement('img');
+          img.src = '/images/black-rectangle.png';
+          beforeSecondLoad = performance.now();
+          document.getElementById('image_div').appendChild(img);
+          firstCallback = false;
+          return;
+        }
+        // The second image is added at its natural size: 100 x 50.
+        assert_equals(entry.size, 5000);
+        assert_greater_than(entry.loadTime, beforeSecondLoad,
+          'The second image load time should occur after adding it to the document body.');
+        assert_greater_than(entry.renderTime, entry.loadTime,
+          'The second image render time should occur after its load time.');
+        t.done();
+      })
+    );
+    observer.observe({type: 'largest-contentful-paint', buffered: true});
+  }, 'Repeated image produces different timestamps.');
+</script>
+<img src='/images/black-rectangle.png' id='image_id'/>
+<div id='image_div'></div>
+</body>

third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

@@ -4421,8 +4421,8 @@ interface LargestContentfulPaint : PerformanceEntry
     attribute @@toStringTag
     getter element
     getter id
+    getter loadTime
     getter renderTime
-    getter responseEnd
     getter size
     getter url
     method constructor