[LCP] Image: remove the record cap

Largest Image Paint used to have a cap on the map & set, containers for
the recorded nodes/objects/images. The cap was placed in order to
prevent the containers from growing uncontrollably. But actually, if
it grows in proportion to the size of the DOM tree, its size should
be deemed under controlled. Based on this idea, we should remove the
cap of containers in LIP.

This CL also fixes the issue that the |invisible_node_ids_| set didn't
remove node_id. Otherwise, its size would be grow uncontrollably.

Related CL:
crrev.com/c/1685325: the text counterpart

Bug: 966476,967837
Change-Id: I29462450311b337bf635bc3cfd68379286422e6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1692199
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676154}

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

@@ -153,6 +153,19 @@ void ImagePaintTimingDetector::OnPaintFinished() {
   RegisterNotifySwapTime();
 }
 
+void ImagePaintTimingDetector::LayoutObjectWillBeDestroyed(
+    const LayoutObject& object) {
+  if (!is_recording_)
+    return;
+
+  DOMNodeId node_id = DOMNodeIds::ExistingIdForNode(object.GetNode());
+  if (node_id == kInvalidDOMNodeId)
+    return;
+  // The visible record removal has been handled by
+  // |NotifyBackgroundImageRemoved|.
+  records_manager_.RemoveInvisibleRecordIfNeeded(node_id);
+}
+
 void ImagePaintTimingDetector::NotifyBackgroundImageRemoved(
     DOMNodeId node_id,
     const ImageResourceContent* cached_image) {
@@ -270,15 +283,6 @@ void ImagePaintTimingDetector::RecordBackgroundImage(
       need_update_timing_at_frame_end_ = true;
     }
   }
-
-  if (records_manager_.RecordedTooManyNodes())
-    HandleTooManyNodes();
-}
-
-void ImagePaintTimingDetector::HandleTooManyNodes() {
-  TRACE_EVENT_INSTANT0("loading", "ImagePaintTimingDetector::OverNodeLimit",
-                       TRACE_EVENT_SCOPE_THREAD);
-  StopRecordEntries();
 }
 
 ImageRecordsManager::ImageRecordsManager()
@@ -311,7 +315,6 @@ std::unique_ptr<ImageRecord> ImageRecordsManager::CreateImageRecord(
     const DOMNodeId& node_id,
     const ImageResourceContent* cached_image,
     const uint64_t& visual_size) {
-  DCHECK(!RecordedTooManyNodes());
   DCHECK_GT(visual_size, 0u);
   std::unique_ptr<ImageRecord> record =
       std::make_unique<ImageRecord>(node_id, cached_image, visual_size);

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

@@ -68,16 +68,15 @@ class CORE_EXPORT ImageRecordsManager {
       std::set<base::WeakPtr<ImageRecord>, NodesQueueComparator>;
 
  public:
-  // Set a big enough limit for the number of nodes to ensure memory usage is
-  // capped. Exceeding such limit will make the detactor stops recording
-  // entries.
-  static constexpr size_t kImageNodeNumberLimit = 5000;
-
   ImageRecordsManager();
   ImageRecord* FindLargestPaintCandidate() const;
 
   bool AreAllVisibleNodesDetached() const;
 
+  inline void RemoveInvisibleRecordIfNeeded(
+      const DOMNodeId& invisible_node_id) {
+    invisible_node_ids_.erase(invisible_node_id);
+  }
   inline void RemoveVisibleRecord(
       const BackgroundImageId& background_image_id) {
     base::WeakPtr<ImageRecord> record =
@@ -90,7 +89,6 @@ class CORE_EXPORT ImageRecordsManager {
   }
 
   inline void RecordInvisibleNode(const DOMNodeId& node_id) {
-    DCHECK(!RecordedTooManyNodes());
     invisible_node_ids_.insert(node_id);
   }
   void RecordVisibleNode(const BackgroundImageId& background_image_id,
@@ -104,11 +102,6 @@ class CORE_EXPORT ImageRecordsManager {
     return invisible_node_ids_.Contains(node_id);
   }
 
-  inline bool RecordedTooManyNodes() const {
-    return visible_background_image_map_.size() + invisible_node_ids_.size() >
-           kImageNodeNumberLimit;
-  }
-
   inline bool WasVisibleNodeLoaded(
       const BackgroundImageId& background_image_id) const {
     DCHECK(visible_background_image_map_.Contains(background_image_id));
@@ -212,7 +205,7 @@ class CORE_EXPORT ImagePaintTimingDetector final
       const ImageResourceContent& cached_image,
       const PropertyTreeState& current_paint_chunk_properties);
   void OnPaintFinished();
-  void LayoutObjectWillBeDestroyed(DOMNodeId);
+  void LayoutObjectWillBeDestroyed(const LayoutObject&);
   void NotifyBackgroundImageRemoved(DOMNodeId, const ImageResourceContent*);
   // After the method being called, the detector stops to record new entries and
   // node removal. But it still observe the loading status. In other words, if
@@ -241,7 +234,6 @@ class CORE_EXPORT ImagePaintTimingDetector final
   void ReportCandidateToTrace(ImageRecord&);
   void ReportNoCandidateToTrace();
   void Deactivate();
-  void HandleTooManyNodes();
 
   void UpdateCandidate();
 

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

@@ -102,8 +102,17 @@ class ImagePaintTimingDetectorTest
         ->records_manager_.visible_background_image_map_.size();
   }
 
+  size_t CountInvisibleRecords() {
+    return GetPaintTimingDetector()
+        .GetImagePaintTimingDetector()
+        ->records_manager_.invisible_node_ids_.size();
+  }
+
   size_t ContainerTotalSize() {
     return GetPaintTimingDetector()
+               .GetImagePaintTimingDetector()
+               ->records_manager_.invisible_node_ids_.size() +
+           GetPaintTimingDetector()
                .GetImagePaintTimingDetector()
                ->records_manager_.visible_background_image_map_.size() +
            GetPaintTimingDetector()
@@ -596,6 +605,35 @@ TEST_F(ImagePaintTimingDetectorTest,
   EXPECT_EQ(ContainerTotalSize(), 0u);
 }
 
+TEST_F(ImagePaintTimingDetectorTest,
+       RemoveRecordFromAllContainersAfterInvisibleImageRemoved) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #target {
+        position: relative;
+        left: 100px;
+      }
+      #parent {
+        background-color: yellow;
+        height: 50px;
+        width: 50px;
+        overflow: scroll;
+      }
+    </style>
+    <div id='parent'>
+      <img id='target'></img>
+    </div>
+  )HTML");
+  SetImageAndPaint("target", 5, 5);
+  UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
+  EXPECT_EQ(ContainerTotalSize(), 1u);
+  EXPECT_EQ(CountInvisibleRecords(), 1u);
+
+  GetDocument().body()->RemoveChild(GetDocument().getElementById("parent"));
+  EXPECT_EQ(ContainerTotalSize(), 0u);
+  EXPECT_EQ(CountInvisibleRecords(), 0u);
+}
+
 TEST_F(ImagePaintTimingDetectorTest,
        RemoveRecordFromAllContainersAfterBackgroundImageRemoval) {
   SetBodyInnerHTML(R"HTML(

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

@@ -120,6 +120,9 @@ void PaintTimingDetector::LayoutObjectWillBeDestroyed(
     const LayoutObject& object) {
   if (text_paint_timing_detector_)
     text_paint_timing_detector_->LayoutObjectWillBeDestroyed(object);
+
+  if (image_paint_timing_detector_)
+    image_paint_timing_detector_->LayoutObjectWillBeDestroyed(object);
 }
 
 void PaintTimingDetector::NotifyBackgroundImageRemoved(