[FCP++] Simplify reactivation logic in TextPaint, ImagePaint

The design of FCP++ have several containers that store the node ids of objects.
In case the size of them will grow unlimitedly, we set a cap for these
containers.

The original implementation uses a counter to count the number of nodes. As a
better approach, we can use the sum of the sizes of the containers for the
same purpose.

The original implementation uses "recorded_node_count_ > kTextNodeNumberLimit"
as a condition. The condition was still checked each time after the condition
has been met. As A better approach, we can have "is_recording_" flag to
indicate whether the container still accepts new entries.

Bug: 869924
Change-Id: Ifc03e5f782e16bab65f6f024d9f73974051ca421
Reviewed-on: https://chromium-review.googlesource.com/c/1349477
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611849}

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

@@ -326,44 +326,39 @@ void ImagePaintTimingDetector::RecordImage(const LayoutObject& object,
   if (size_zero_ids_.Contains(node_id))
     return;
 
-  if (!id_record_map_.Contains(node_id)) {
-    recorded_node_count_++;
-    if (recorded_node_count_ < kImageNodeNumberLimit) {
-      LayoutRect invalidated_rect = object.FirstFragment().VisualRect();
-      // Before the image resource is loaded, <img> has size 0, so we do not
-      // record the first size until the invalidated rect's size becomes
-      // non-empty.
-      if (invalidated_rect.IsEmpty())
-        return;
-      uint64_t rect_size =
-          frame_view_->GetPaintTimingDetector().CalculateVisualSize(
-              invalidated_rect, painting_layer);
-      if (rect_size == 0) {
-        // When rect_size == 0, it either means the image is size 0 or the image
-        // is out of viewport. Either way, we don't track this image anymore, to
-        // reduce computation.
-        size_zero_ids_.insert(node_id);
-        return;
-      }
-      std::unique_ptr<ImageRecord> record = std::make_unique<ImageRecord>();
-      record->node_id = node_id;
-      record->image_url = GetImageUrl(object);
-      // Mind that first_size has to be assigned at the push of
-      // largest_image_heap_ since it's the sorting key.
-      record->first_size = rect_size;
-      largest_image_heap_.push(record->AsWeakPtr());
-      id_record_map_.insert(node_id, std::move(record));
-    } else {
-      // for assessing whether kImageNodeNumberLimit is large enough for all
-      // normal cases
-      TRACE_EVENT_INSTANT1("loading", "ImagePaintTimingDetector::OverNodeLimit",
-                           TRACE_EVENT_SCOPE_THREAD, "recorded_node_count",
-                           recorded_node_count_);
+  if (!id_record_map_.Contains(node_id) && is_recording_) {
+    LayoutRect invalidated_rect = object.FirstFragment().VisualRect();
+    // Before the image resource is loaded, <img> has size 0, so we do not
+    // record the first size until the invalidated rect's size becomes
+    // non-empty.
+    if (invalidated_rect.IsEmpty())
+      return;
+    uint64_t rect_size =
+        frame_view_->GetPaintTimingDetector().CalculateVisualSize(
+            invalidated_rect, painting_layer);
+    if (rect_size == 0) {
+      // When rect_size == 0, it either means the image is size 0 or the image
+      // is out of viewport. Either way, we don't track this image anymore, to
+      // reduce computation.
+      size_zero_ids_.insert(node_id);
+      return;
     }
+    // Non-trivial image is found.
+    std::unique_ptr<ImageRecord> record = std::make_unique<ImageRecord>();
+    record->node_id = node_id;
+    record->image_url = GetImageUrl(object);
+    // Mind that first_size has to be assigned at the push of
+    // largest_image_heap_ since it's the sorting key.
+    record->first_size = rect_size;
+    largest_image_heap_.push(record->AsWeakPtr());
+    id_record_map_.insert(node_id, std::move(record));
+    if (id_record_map_.size() + size_zero_ids_.size() > kImageNodeNumberLimit)
+      Deactivate();
   }
 
   if (id_record_map_.Contains(node_id) && !id_record_map_.at(node_id)->loaded &&
       IsLoaded(object)) {
+    // The image is just loaded.
     records_pending_timing_.push(node_id);
     ImageRecord* record = id_record_map_.at(node_id);
     record->frame_index = frame_index_;
@@ -379,6 +374,14 @@ void ImagePaintTimingDetector::RecordImage(const LayoutObject& object,
   }
 }
 
+void ImagePaintTimingDetector::Deactivate() {
+  TRACE_EVENT_INSTANT2("loading", "ImagePaintTimingDetector::OverNodeLimit",
+                       TRACE_EVENT_SCOPE_THREAD, "recorded_node_count",
+                       id_record_map_.size(), "size_zero_node_count",
+                       size_zero_ids_.size());
+  is_recording_ = false;
+}
+
 ImageRecord* ImagePaintTimingDetector::FindLargestPaintCandidate() {
   while (!largest_image_heap_.empty() && !largest_image_heap_.top()) {
     // Discard the elements that have been removed from |id_record_map_|.

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

@@ -84,6 +84,7 @@ class CORE_EXPORT ImagePaintTimingDetector final
   void RegisterNotifySwapTime();
   void OnLargestImagePaintDetected(const ImageRecord&);
   void OnLastImagePaintDetected(const ImageRecord&);
+  void Deactivate();
 
   void Analyze();
 
@@ -102,7 +103,6 @@ class CORE_EXPORT ImagePaintTimingDetector final
                       bool (*)(const base::WeakPtr<ImageRecord>&,
                                const base::WeakPtr<ImageRecord>&)>
       latest_image_heap_;
-  unsigned recorded_node_count_ = 0;
 
   // Node-ids of records pending swap time are stored in this queue until they
   // get a swap time.
@@ -119,6 +119,7 @@ class CORE_EXPORT ImagePaintTimingDetector final
   unsigned frame_index_ = 1;
 
   unsigned last_frame_index_queued_for_timing_ = 0;
+  bool is_recording_ = true;
 
   base::TimeTicks largest_image_paint_;
   base::TimeTicks last_image_paint_;

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

@@ -172,11 +172,15 @@ void TextPaintTimingDetector::ReportSwapTime(
 
 void TextPaintTimingDetector::RecordText(const LayoutObject& object,
                                          const PaintLayer& painting_layer) {
+  if (!is_recording_)
+    return;
   Node* node = object.GetNode();
   if (!node)
     return;
   DOMNodeId node_id = DOMNodeIds::IdForNode(node);
 
+  // This metric defines the size of a text by its first size. So it
+  // early-returns if the text has been recorded.
   if (size_zero_node_ids_.find(node_id) != size_zero_node_ids_.end())
     return;
   if (recorded_text_node_ids_.find(node_id) != recorded_text_node_ids_.end())
@@ -184,16 +188,6 @@ void TextPaintTimingDetector::RecordText(const LayoutObject& object,
   // When node_id is not found in recorded_text_node_ids_, this invalidation is
   // the text's first invalidation.
 
-  // We deactivate the algorithm if the number of nodes exceeds limitation.
-  recorded_node_count_++;
-  if (recorded_node_count_ > kTextNodeNumberLimit) {
-    // for assessing whether kTextNodeNumberLimit is large enough for all
-    // normal cases
-    TRACE_EVENT_INSTANT1("loading", "TextPaintTimingDetector::OverNodeLimit",
-                         TRACE_EVENT_SCOPE_THREAD, "recorded_node_count",
-                         recorded_node_count_);
-    return;
-  }
   uint64_t rect_size = 0;
   LayoutRect invalidated_rect = object.FirstFragment().VisualRect();
   if (!invalidated_rect.IsEmpty()) {
@@ -201,16 +195,31 @@ void TextPaintTimingDetector::RecordText(const LayoutObject& object,
         invalidated_rect, painting_layer);
   }
 
-  // When rect_size == 0, it either means invalidated_rect.IsEmpty() or
-  // the text is size 0 or the text is out of viewport. Either way, we don't
-  // record their time, to reduce computation.
+  // When rect_size == 0, it either means the text size is 0 or the text is out
+  // of viewport. In either case, we don't record their time for efficiency.
   if (rect_size == 0) {
     size_zero_node_ids_.insert(node_id);
   } else {
+    // Non-trivial text is found.
     TextRecord record = {node_id, rect_size, base::TimeTicks(),
                          ToLayoutText(&object)->GetText()};
     texts_to_record_swap_time_.push_back(record);
   }
+
+  if (recorded_text_node_ids_.size() + size_zero_node_ids_.size() +
+          texts_to_record_swap_time_.size() >=
+      kTextNodeNumberLimit) {
+    Deactivate();
+  }
+}
+
+void TextPaintTimingDetector::Deactivate() {
+  TRACE_EVENT_INSTANT2("loading", "TextPaintTimingDetector::OverNodeLimit",
+                       TRACE_EVENT_SCOPE_THREAD, "recorded_node_count",
+                       recorded_text_node_ids_.size(), "size_zero_node_count",
+                       size_zero_node_ids_.size());
+  timer_.Stop();
+  is_recording_ = false;
 }
 
 TextRecord* TextPaintTimingDetector::FindLargestPaintCandidate() {

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

@@ -77,6 +77,7 @@ class CORE_EXPORT TextPaintTimingDetector final
   void RegisterNotifySwapTime(ReportTimeCallback callback);
   void OnLargestTextDetected(const TextRecord&);
   void OnLastTextDetected(const TextRecord&);
+  void Deactivate();
 
   HashSet<DOMNodeId> recorded_text_node_ids_;
   HashSet<DOMNodeId> size_zero_node_ids_;
@@ -94,9 +95,9 @@ class CORE_EXPORT TextPaintTimingDetector final
 
   // Make sure that at most one swap promise is ongoing.
   bool awaiting_swap_promise_ = false;
-  unsigned recorded_node_count_ = 0;
   unsigned largest_text_candidate_index_max_ = 0;
   unsigned last_text_candidate_index_max_ = 0;
+  bool is_recording_ = true;
 
   base::TimeTicks largest_text_paint_;
   base::TimeTicks last_text_paint_;