[FCP++] Background image: track individually

Background image is different from normal image in that one node
could have multiple background images, while a normal image is one
node by itself.

Currently we are using node id to track all of the background
images attached to a node. All of the background images attached
to one node are regarded as one image (tracked by one ImageRecord)
in FCP++.

We should track each background images separately instead to have
higher resolution to the background image behaviors.

This CL will address this issue. We will use the DOM node id and the
style image pointer as a pair to be the id.

Bug: 936149

Change-Id: I6302872dbd2a2b3e082eeeaa7720354eced50275
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1554426Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653274}

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

@@ -3858,6 +3858,12 @@ void LayoutObject::NotifyImageFullyRemoved(ImageResourceContent* image) {
                                                                      image);
     }
   }
+  if (RuntimeEnabledFeatures::FirstContentfulPaintPlusPlusEnabled()) {
+    if (LocalFrameView* frame_view = GetFrameView()) {
+      frame_view->GetPaintTimingDetector().NotifyBackgroundImageRemoved(*this,
+                                                                        image);
+    }
+  }
 }
 
 PositionWithAffinity LayoutObject::CreatePositionWithAffinity(

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

@@ -549,7 +549,7 @@ inline bool PaintFastBottomLayer(Node* node,
   if (RuntimeEnabledFeatures::FirstContentfulPaintPlusPlusEnabled()) {
     if (info.image && info.image->IsImageResource()) {
       PaintTimingDetector::NotifyBackgroundImagePaint(
-          node, image, info.image,
+          node, image, info.image->CachedImage(),
           paint_info.context.GetPaintController()
               .CurrentPaintChunkProperties());
     }
@@ -682,7 +682,7 @@ void PaintFillLayerBackground(GraphicsContext& context,
     if (RuntimeEnabledFeatures::FirstContentfulPaintPlusPlusEnabled()) {
       if (info.image && info.image->IsImageResource()) {
         PaintTimingDetector::NotifyBackgroundImagePaint(
-            node, image, info.image,
+            node, image, info.image->CachedImage(),
             context.GetPaintController().CurrentPaintChunkProperties());
       }
     }

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

@@ -22,10 +22,14 @@ class LocalFrameView;
 class PropertyTreeState;
 class TracedValue;
 class Image;
+class ImageResourceContent;
 
 class ImageRecord : public base::SupportsWeakPtr<ImageRecord> {
  public:
+  unsigned record_id;
   DOMNodeId node_id = kInvalidDOMNodeId;
+  // Mind that |first_size| has to be assigned before pusing to
+  // |size_ordered_set_| since it's the sorting key.
   uint64_t first_size = 0;
   unsigned frame_index = 0;
   // The time of the first paint after fully loaded.
@@ -36,6 +40,8 @@ class ImageRecord : public base::SupportsWeakPtr<ImageRecord> {
 #endif
 };
 
+typedef std::pair<DOMNodeId, const ImageResourceContent*> BackgroundImageId;
+
 // |ImageRecordsManager| is the manager of all of the images that Largest Image
 // Paint cares about. Note that an image does not necessarily correspond to a
 // node; it can also be one of the background images attached to a node.
@@ -60,12 +66,19 @@ class CORE_EXPORT ImageRecordsManager {
   void RecordInvisibleNode(const DOMNodeId&);
   void RecordVisibleNode(const DOMNodeId&,
                          const uint64_t& visual_size,
-                         const LayoutObject&);
+                         const String& url);
+  void RecordVisibleNode(const BackgroundImageId& background_image_id,
+                         const uint64_t& visual_size,
+                         const String& url);
   size_t CountVisibleNodes() const { return visible_node_map_.size(); }
   size_t CountInvisibleNodes() const { return invisible_node_ids_.size(); }
   bool IsRecordedVisibleNode(const DOMNodeId& node_id) const {
     return visible_node_map_.Contains(node_id);
   }
+  bool IsRecordedVisibleNode(
+      const BackgroundImageId& background_image_id) const {
+    return visible_background_image_map_.Contains(background_image_id);
+  }
   bool IsRecordedInvisibleNode(const DOMNodeId& node_id) const {
     return invisible_node_ids_.Contains(node_id);
   }
@@ -73,7 +86,11 @@ class CORE_EXPORT ImageRecordsManager {
   bool RecordedTooManyNodes() const;
 
   bool WasVisibleNodeLoaded(const DOMNodeId&) const;
+  bool WasVisibleNodeLoaded(const BackgroundImageId& background_image_id) const;
   void OnImageLoaded(const DOMNodeId&, unsigned current_frame_index);
+  void OnImageLoaded(const BackgroundImageId&, unsigned current_frame_index);
+  void OnImageLoadedInternal(base::WeakPtr<ImageRecord>&,
+                             unsigned current_frame_index);
 
   bool NeedMeausuringPaintTime() const {
     return !images_queued_for_paint_time_.empty();
@@ -91,12 +108,23 @@ class CORE_EXPORT ImageRecordsManager {
  private:
   // Find the image record of an visible image.
   base::WeakPtr<ImageRecord> FindVisibleRecord(const DOMNodeId&) const;
+  base::WeakPtr<ImageRecord> FindVisibleRecord(
+      const BackgroundImageId& background_image_id) const;
+  std::unique_ptr<ImageRecord> CreateImageRecord(
+      const DOMNodeId&,
+      const ImageResourceContent* cached_image,
+      const uint64_t& visual_size,
+      const String& url);
   void QueueToMeasurePaintTime(base::WeakPtr<ImageRecord>&,
                                unsigned current_frame_index);
   void SetLoaded(base::WeakPtr<ImageRecord>&);
+
+  unsigned max_record_id_ = 0;
   // We will never destroy the pointers within |visible_node_map_|. Once created
   // they will exist for the whole life cycle of |visible_node_map_|.
   HashMap<DOMNodeId, std::unique_ptr<ImageRecord>> visible_node_map_;
+  HashMap<BackgroundImageId, std::unique_ptr<ImageRecord>>
+      visible_background_image_map_;
   HashSet<DOMNodeId> invisible_node_ids_;
   // Use |DOMNodeId| instead of |ImageRecord|* for the efficiency of inserting
   // and erasing.
@@ -139,12 +167,18 @@ class CORE_EXPORT ImagePaintTimingDetector final
   ImageRecord* FindLargestPaintCandidate();
   void RecordImage(const LayoutObject&,
                    const IntSize& intrinsic_size,
-                   bool is_loaded,
+                   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);
   static bool IsBackgroundImageContentful(const LayoutObject&, const Image&);
   static bool HasBackgroundImage(const LayoutObject& object);
   void OnPaintFinished();
   void NotifyNodeRemoved(DOMNodeId);
+  void NotifyBackgroundImageRemoved(DOMNodeId, const ImageResourceContent*);
   base::TimeTicks LargestImagePaint() const {
     return !largest_image_paint_ ? base::TimeTicks()
                                  : largest_image_paint_->paint_time;
@@ -190,6 +224,8 @@ class CORE_EXPORT ImagePaintTimingDetector final
   // no effect on recording the loading status.
   bool is_recording_ = true;
 
+  bool need_update_timing_at_frame_end_ = false;
+
   ImageRecord* largest_image_paint_ = nullptr;
   ImageRecordsManager records_manager_;
   Member<LocalFrameView> frame_view_;

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

@@ -84,13 +84,19 @@ class ImagePaintTimingDetectorTest
         .records_manager_.FindLargestPaintCandidate();
   }
 
-  unsigned CountRecords() {
+  size_t CountVisibleImageRecords() {
     return GetPaintTimingDetector()
         .GetImagePaintTimingDetector()
         .records_manager_.visible_node_map_.size();
   }
 
-  unsigned CountChildFrameRecords() {
+  size_t CountVisibleBackgroundImageRecords() {
+    return GetPaintTimingDetector()
+        .GetImagePaintTimingDetector()
+        .records_manager_.visible_background_image_map_.size();
+  }
+
+  size_t CountChildFrameRecords() {
     return GetChildPaintTimingDetector()
         .GetImagePaintTimingDetector()
         .records_manager_.visible_node_map_.size();
@@ -206,12 +212,12 @@ TEST_F(ImagePaintTimingDetectorTest,
     <img id="target"></img>
   )HTML");
   UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleImageRecords(), 0u);
   SetImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
   ImageRecord* record = FindLargestPaintCandidate();
   EXPECT_TRUE(record);
-  EXPECT_EQ(CountRecords(), 1u);
+  EXPECT_EQ(CountVisibleImageRecords(), 1u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, LargestImagePaint_Largest) {
@@ -249,7 +255,8 @@ TEST_F(ImagePaintTimingDetectorTest,
   EXPECT_FALSE(record);
 }
 
-TEST_F(ImagePaintTimingDetectorTest, LargestImagePaint_IgnoreTheRemoved) {
+TEST_F(ImagePaintTimingDetectorTest,
+       LargestImagePaint_UpdateOnRemovingTheLastImage) {
   SetBodyInnerHTML(R"HTML(
     <div id="parent">
       <img id="target"></img>
@@ -270,6 +277,38 @@ TEST_F(ImagePaintTimingDetectorTest, LargestImagePaint_IgnoreTheRemoved) {
   EXPECT_EQ(LargestPaintStoredResult(), base::TimeTicks());
 }
 
+TEST_F(ImagePaintTimingDetectorTest, LargestImagePaint_UpdateOnRemoving) {
+  SetBodyInnerHTML(R"HTML(
+    <div id="parent">
+      <img id="target1"></img>
+      <img id="target2"></img>
+    </div>
+  )HTML");
+  SetImageAndPaint("target1", 5, 5);
+  UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
+  ImageRecord* record1 = FindLargestPaintCandidate();
+  EXPECT_TRUE(record1);
+  EXPECT_NE(LargestPaintStoredResult(), base::TimeTicks());
+  base::TimeTicks first_largest_image_paint = LargestPaintStoredResult();
+
+  SetImageAndPaint("target2", 10, 10);
+  UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
+  ImageRecord* record2 = FindLargestPaintCandidate();
+  EXPECT_TRUE(record2);
+  EXPECT_NE(LargestPaintStoredResult(), base::TimeTicks());
+  base::TimeTicks second_largest_image_paint = LargestPaintStoredResult();
+
+  EXPECT_NE(record1, record2);
+  EXPECT_NE(first_largest_image_paint, second_largest_image_paint);
+
+  GetDocument().getElementById("parent")->RemoveChild(
+      GetDocument().getElementById("target2"));
+  UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
+  ImageRecord* record1_2 = FindLargestPaintCandidate();
+  EXPECT_EQ(record1, record1_2);
+  EXPECT_EQ(first_largest_image_paint, LargestPaintStoredResult());
+}
+
 TEST_F(ImagePaintTimingDetectorTest,
        LargestImagePaint_NodeRemovedBetweenRegistrationAndInvocation) {
   SetBodyInnerHTML(R"HTML(
@@ -459,7 +498,7 @@ TEST_F(ImagePaintTimingDetectorTest, BackgroundImage) {
   )HTML");
   ImageRecord* record = FindLargestPaintCandidate();
   EXPECT_TRUE(record);
-  EXPECT_EQ(CountRecords(), 1u);
+  EXPECT_EQ(CountVisibleBackgroundImageRecords(), 1u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, BackgroundImage_IgnoreBody) {
@@ -470,7 +509,7 @@ TEST_F(ImagePaintTimingDetectorTest, BackgroundImage_IgnoreBody) {
       }
     </style>
   )HTML");
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleBackgroundImageRecords(), 0u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, BackgroundImage_IgnoreHtml) {
@@ -483,7 +522,7 @@ TEST_F(ImagePaintTimingDetectorTest, BackgroundImage_IgnoreHtml) {
     </style>
     </html>
   )HTML");
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleBackgroundImageRecords(), 0u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, BackgroundImage_IgnoreGradient) {
@@ -497,7 +536,25 @@ TEST_F(ImagePaintTimingDetectorTest, BackgroundImage_IgnoreGradient) {
       place-holder
     </div>
   )HTML");
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleBackgroundImageRecords(), 0u);
+}
+
+// We put two background images in the same object, and test whether FCP++ can
+// find two different images.
+TEST_F(ImagePaintTimingDetectorTest, BackgroundImageTrackedDifferently) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #d {
+        width: 50px;
+        height: 50px;
+        background-image:
+          url("data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=="),
+          url("data:image/gif;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABHNCSVQICAgIfAhkiAAAAAlwSFlzAAAN1wAADdcBQiibeAAAAb5JREFUOMulkr1KA0EQgGdvTwwnYmER0gQsrFKmSy+pLESw9Qm0F/ICNnba+h6iEOuAEWslKJKTOyJJvIT72d1xZuOFC0giOLA77O7Mt/PnNptN+I+49Xr9GhH3f3mb0v1ht9vtLAUYYw5ItkgDL3KyD8PhcLvdbl/WarXT3DjLMnAcR/f7/YfxeKwtgC5RKQVhGILWeg4hQ6hUKjWyucmhLFEUuWR3QYBWAZABQ9i5CCmXy16pVALP80BKaaG+70MQBLvzFMjRKKXh8j6FSYKF7ITdEWLa4/ktokN74wiqjSMpnVcbQZqmEJHz+ckeCPFjWKwULpyspAqhdXVXdcnZcPjsIgn+2BsVA8jVYuWlgJ3yBj0icgq2uoK+lg4t+ZvLomSKamSQ4AI5BcMADtMhyNoSgNIISUaFNtwlazcDcBc4gjjVwCWid2usCWroYEhnaqbzFJLUzAHIXRDChXCcQP8zhkSZ5eNLgHAUzwDcRu4CoIRn/wsGUQIIy4Vr9TH6SYFCNzw4nALn5627K4vIttOUOwfa5YnrDYzt/9OLv9I5l8kk5hZ3XLO20b7tbR7zHLy/BX8G0IeBEM7ZN1NGIaFUaKLgAAAAAElFTkSuQmCC");
+      }
+    </style>
+    <div id="d"></div>
+  )HTML");
+  EXPECT_EQ(CountVisibleBackgroundImageRecords(), 2u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, DeactivateAfterUserInput) {
@@ -509,7 +566,7 @@ TEST_F(ImagePaintTimingDetectorTest, DeactivateAfterUserInput) {
   SimulateScroll();
   SetImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleImageRecords(), 0u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, NullTimeNoCrash) {
@@ -533,7 +590,7 @@ TEST_F(ImagePaintTimingDetectorTest, Iframe) {
   SetChildFrameImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhasesForTest();
   // Ensure main frame doesn't capture this image.
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleImageRecords(), 0u);
   EXPECT_EQ(CountChildFrameRecords(), 1u);
   InvokeCallback();
   ImageRecord* image = FindChildFrameLargestPaintCandidate();
@@ -558,7 +615,7 @@ TEST_F(ImagePaintTimingDetectorTest, Iframe_ClippedByMainFrameViewport) {
   ReplaceCallBackQueue(GetChildPaintTimingDetector());
   SetChildFrameImageAndPaint("target", 5, 5);
   UpdateAllLifecyclePhasesForTest();
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleImageRecords(), 0u);
 }
 
 TEST_F(ImagePaintTimingDetectorTest, Iframe_HalfClippedByMainFrameViewport) {
@@ -575,7 +632,7 @@ TEST_F(ImagePaintTimingDetectorTest, Iframe_HalfClippedByMainFrameViewport) {
   ReplaceCallBackQueue(GetChildPaintTimingDetector());
   SetChildFrameImageAndPaint("target", 10, 10);
   UpdateAllLifecyclePhasesForTest();
-  EXPECT_EQ(CountRecords(), 0u);
+  EXPECT_EQ(CountVisibleImageRecords(), 0u);
   EXPECT_EQ(CountChildFrameRecords(), 1u);
   InvokeCallback();
   ImageRecord* image = FindChildFrameLargestPaintCandidate();

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

@@ -38,9 +38,10 @@ void PaintTimingDetector::NotifyPaintFinished() {
 void PaintTimingDetector::NotifyBackgroundImagePaint(
     const Node* node,
     const Image* image,
-    const StyleImage* cached_image,
+    const ImageResourceContent* cached_image,
     const PropertyTreeState& current_paint_chunk_properties) {
   DCHECK(image);
+  DCHECK(cached_image);
   if (!node)
     return;
   LayoutObject* object = node->GetLayoutObject();
@@ -48,21 +49,14 @@ void PaintTimingDetector::NotifyBackgroundImagePaint(
     return;
   if (!ImagePaintTimingDetector::IsBackgroundImageContentful(*object, *image))
     return;
-  // TODO(crbug/936149): This check is needed because the |image| and the
-  // background images in node could have inconsistent state. This can be
-  // resolved by tracking each background image separately. We will no longer
-  // need to find background images from a node's layers.
-  if (!ImagePaintTimingDetector::HasBackgroundImage(*object))
-    return;
   LocalFrameView* frame_view = object->GetFrameView();
   if (!frame_view)
     return;
   if (!cached_image)
     return;
   PaintTimingDetector& detector = frame_view->GetPaintTimingDetector();
-  detector.GetImagePaintTimingDetector().RecordImage(
-      *object, image->Size(), cached_image->IsLoaded(),
-      current_paint_chunk_properties);
+  detector.GetImagePaintTimingDetector().RecordBackgroundImage(
+      *object, image->Size(), cached_image, current_paint_chunk_properties);
 }
 
 // static
@@ -78,8 +72,7 @@ void PaintTimingDetector::NotifyImagePaint(
     return;
   PaintTimingDetector& detector = frame_view->GetPaintTimingDetector();
   detector.GetImagePaintTimingDetector().RecordImage(
-      object, intrinsic_size, cached_image->IsLoaded(),
-      current_paint_chunk_properties);
+      object, intrinsic_size, cached_image, current_paint_chunk_properties);
 }
 
 // static
@@ -102,6 +95,16 @@ void PaintTimingDetector::NotifyNodeRemoved(const LayoutObject& object) {
   image_paint_timing_detector_->NotifyNodeRemoved(node_id);
 }
 
+void PaintTimingDetector::NotifyBackgroundImageRemoved(
+    const LayoutObject& object,
+    const ImageResourceContent* cached_image) {
+  DOMNodeId node_id = DOMNodeIds::ExistingIdForNode(object.GetNode());
+  if (node_id == kInvalidDOMNodeId)
+    return;
+  image_paint_timing_detector_->NotifyBackgroundImageRemoved(node_id,
+                                                             cached_image);
+}
+
 void PaintTimingDetector::NotifyInputEvent(WebInputEvent::Type type) {
   if (type == WebInputEvent::kMouseMove || type == WebInputEvent::kMouseEnter ||
       type == WebInputEvent::kMouseLeave ||

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

@@ -18,7 +18,6 @@ class ImageResourceContent;
 class LayoutObject;
 class LocalFrameView;
 class PropertyTreeState;
-class StyleImage;
 class TextPaintTimingDetector;
 
 // PaintTimingDetector contains some of paint metric detectors,
@@ -33,13 +32,10 @@ class CORE_EXPORT PaintTimingDetector
  public:
   PaintTimingDetector(LocalFrameView*);
 
-  // TODO(crbug/936124): the detector no longer need to look for background
-  // images in each layer.
-  // Notify the paint of background image.
   static void NotifyBackgroundImagePaint(
       const Node*,
       const Image*,
-      const StyleImage* cached_image,
+      const ImageResourceContent* cached_image,
       const PropertyTreeState& current_paint_chunk_properties);
   static void NotifyImagePaint(
       const LayoutObject&,
@@ -47,13 +43,14 @@ class CORE_EXPORT PaintTimingDetector
       const ImageResourceContent* cached_image,
       const PropertyTreeState& current_paint_chunk_properties);
 
-  static void NotifyTextPaint(const LayoutObject& object,
-                              const PropertyTreeState&);
+  static void NotifyTextPaint(const LayoutObject&, const PropertyTreeState&);
   void NotifyNodeRemoved(const LayoutObject&);
+  void NotifyBackgroundImageRemoved(const LayoutObject&,
+                                    const ImageResourceContent*);
   void NotifyPaintFinished();
   void NotifyInputEvent(WebInputEvent::Type);
   bool NeedToNotifyInputOrScroll();
-  void NotifyScroll(ScrollType scroll_type);
+  void NotifyScroll(ScrollType);
   void DidChangePerformanceTiming();
 
   // |visual_rect| should be an object's bounding rect in the space of