[LCP] Record's secondary ranking key is the insertion order

Currently, LargestTextPaint and LargestImagePaint uses node_id as the
secondary ranking key. This means when two records have the same size,
the node_id will be used to decide which one is the largest. However,
the order of node_id can be affected by factors out of LCP. So it's
potentially indeterministic. To make the result more deterministic, we
can use the insertion order as the secondary ranking key.

This CL reflects the above change.

Bug: 959984

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

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

@@ -65,9 +65,7 @@ static bool LargeImageFirst(const base::WeakPtr<ImageRecord>& a,
     return a->first_size > b->first_size;
   // This make sure that two different nodes with the same |first_size| wouldn't
   // be merged in the set.
-  if (a->node_id != b->node_id)
-    return a->node_id > b->node_id;
-  return a->record_id > b->record_id;
+  return a->insertion_index < b->insertion_index;
 }
 
 ImagePaintTimingDetector::ImagePaintTimingDetector(LocalFrameView* frame_view)
@@ -315,11 +313,8 @@ std::unique_ptr<ImageRecord> ImageRecordsManager::CreateImageRecord(
     const uint64_t& visual_size) {
   DCHECK(!RecordedTooManyNodes());
   DCHECK_GT(visual_size, 0u);
-  std::unique_ptr<ImageRecord> record = std::make_unique<ImageRecord>();
-  record->record_id = max_record_id_++;
-  record->node_id = node_id;
-  record->first_size = visual_size;
-  record->cached_image = cached_image;
+  std::unique_ptr<ImageRecord> record =
+      std::make_unique<ImageRecord>(node_id, cached_image, visual_size);
   return record;
 }
 

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

@@ -28,15 +28,27 @@ class Image;
 // TODO(crbug/960502): we should limit the access of these properties.
 class ImageRecord : public base::SupportsWeakPtr<ImageRecord> {
  public:
-  unsigned record_id;
+  ImageRecord(DOMNodeId new_node_id,
+              const ImageResourceContent* new_cached_image,
+              uint64_t new_first_size)
+      : node_id(new_node_id),
+        cached_image(new_cached_image),
+        first_size(new_first_size) {
+    static unsigned next_insertion_index_ = 1;
+    insertion_index = next_insertion_index_++;
+  }
+
+  ImageRecord() {}
+
   DOMNodeId node_id = kInvalidDOMNodeId;
+  WeakPersistent<const ImageResourceContent> cached_image;
   // 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;
+  unsigned insertion_index;
   // The time of the first paint after fully loaded. 0 means not painted yet.
   base::TimeTicks paint_time = base::TimeTicks();
-  WeakPersistent<const ImageResourceContent> cached_image;
   bool loaded = false;
 };
 
@@ -151,7 +163,6 @@ class CORE_EXPORT ImageRecordsManager {
     record->loaded = true;
   }
 
-  unsigned max_record_id_ = 0;
   HashMap<BackgroundImageId, std::unique_ptr<ImageRecord>>
       visible_background_image_map_;
   HashSet<DOMNodeId> invisible_node_ids_;

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

@@ -235,6 +235,31 @@ TEST_F(ImagePaintTimingDetectorTest, LargestImagePaint_OneImage) {
   EXPECT_TRUE(record->loaded);
 }
 
+TEST_F(ImagePaintTimingDetectorTest, InsertionOrderIsSecondaryRankingKey) {
+  SetBodyInnerHTML(R"HTML(
+  )HTML");
+
+  auto* image1 = MakeGarbageCollected<HTMLImageElement>(GetDocument());
+  image1->setAttribute("id", "image1");
+  GetDocument().body()->AppendChild(image1);
+  SetImageAndPaint("image1", 5, 5);
+
+  auto* image2 = MakeGarbageCollected<HTMLImageElement>(GetDocument());
+  image2->setAttribute("id", "image2");
+  GetDocument().body()->AppendChild(image2);
+  SetImageAndPaint("image2", 5, 5);
+
+  auto* image3 = MakeGarbageCollected<HTMLImageElement>(GetDocument());
+  image3->setAttribute("id", "image3");
+  GetDocument().body()->AppendChild(image3);
+  SetImageAndPaint("image3", 5, 5);
+
+  UpdateAllLifecyclePhasesAndInvokeCallbackIfAny();
+
+  EXPECT_EQ(FindLargestPaintCandidate()->node_id,
+            DOMNodeIds::ExistingIdForNode(image1));
+}
+
 TEST_F(ImagePaintTimingDetectorTest, LargestImagePaint_TraceEvent_Candidate) {
   using trace_analyzer::Query;
   trace_analyzer::Start("loading");

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

@@ -35,7 +35,7 @@ bool LargeTextFirst(const base::WeakPtr<TextRecord>& a,
     return a->first_size > b->first_size;
   // This make sure that two different nodes with the same |first_size| wouldn't
   // be merged in the set.
-  return a->node_id > b->node_id;
+  return a->insertion_index_ < b->insertion_index_;
 }
 
 }  // namespace

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

@@ -32,10 +32,16 @@ class TextRecord : public base::SupportsWeakPtr<TextRecord> {
              const FloatRect& element_timing_rect)
       : node_id(new_node_id),
         first_size(new_first_size),
-        element_timing_rect_(element_timing_rect) {}
+        element_timing_rect_(element_timing_rect) {
+    static unsigned next_insertion_index_ = 1;
+    insertion_index_ = next_insertion_index_++;
+  }
 
   DOMNodeId node_id = kInvalidDOMNodeId;
   uint64_t first_size = 0;
+  // |insertion_index_| is ordered by insertion time, used as a secondary key
+  // for ranking.
+  unsigned insertion_index_ = 0;
   FloatRect element_timing_rect_;
   // The time of the first paint after fully loaded.
   base::TimeTicks paint_time = base::TimeTicks();

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

@@ -197,6 +197,17 @@ TEST_F(TextPaintTimingDetectorTest, LargestTextPaint_OneText) {
             DOMNodeIds::ExistingIdForNode(only_text));
 }
 
+TEST_F(TextPaintTimingDetectorTest, InsertionOrderIsSecondaryRankingKey) {
+  SetBodyInnerHTML(R"HTML(
+  )HTML");
+  Element* first = AppendDivElementToBody("text");
+  AppendDivElementToBody("text");
+  AppendDivElementToBody("text");
+  UpdateAllLifecyclePhasesAndSimulateSwapTime();
+  EXPECT_EQ(TextRecordOfLargestTextPaint()->node_id,
+            DOMNodeIds::ExistingIdForNode(first));
+}
+
 TEST_F(TextPaintTimingDetectorTest, LargestTextPaint_TraceEvent_Candidate) {
   using trace_analyzer::Query;
   trace_analyzer::Start("*");