[FCP++] Prevent null from being trace time
We introduced a bug in https://chromium-review.googlesource.com/c/chromium/src/+/1380193.
When image paint detector finds the largest paint candidate with null time, it passes
in to the trace, which will cause a crash.
To fix it, we skip the candidate if the candidate's time is null.
Bug: 920627
Change-Id: Ie39ac9fbc1d09fc88e09851497a80610969bd527
Reviewed-on: https://chromium-review.googlesource.com/c/1407088
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622112}
diff --git a/third_party/blink/renderer/core/paint/image_paint_timing_detector.cc b/third_party/blink/renderer/core/paint/image_paint_timing_detector.cc
index 9591fc3..abaec5e 100644
--- a/third_party/blink/renderer/core/paint/image_paint_timing_detector.cc
+++ b/third_party/blink/renderer/core/paint/image_paint_timing_detector.cc
@@ -137,6 +137,7 @@
void ImagePaintTimingDetector::OnLargestImagePaintDetected(
ImageRecord* largest_image_record) {
DCHECK(largest_image_record);
+ DCHECK(!largest_image_record->first_paint_time_after_loaded.is_null());
largest_image_paint_ = largest_image_record;
std::unique_ptr<TracedValue> value = TracedValue::Create();
PopulateTraceValue(*value, *largest_image_record,
@@ -150,6 +151,7 @@
void ImagePaintTimingDetector::OnLastImagePaintDetected(
ImageRecord* last_image_record) {
DCHECK(last_image_record);
+ DCHECK(!last_image_record->first_paint_time_after_loaded.is_null());
last_image_paint_ = last_image_record;
std::unique_ptr<TracedValue> value = TracedValue::Create();
PopulateTraceValue(*value, *last_image_record,
@@ -170,12 +172,16 @@
// result unless it's a new candidate.
ImageRecord* largest_image_record = FindLargestPaintCandidate();
bool new_candidate_detected = false;
- if (largest_image_record && largest_image_record != largest_image_paint_) {
+ if (largest_image_record &&
+ !largest_image_record->first_paint_time_after_loaded.is_null() &&
+ largest_image_record != largest_image_paint_) {
new_candidate_detected = true;
OnLargestImagePaintDetected(largest_image_record);
}
ImageRecord* last_image_record = FindLastPaintCandidate();
- if (last_image_record && last_image_record != last_image_paint_) {
+ if (last_image_record &&
+ !last_image_record->first_paint_time_after_loaded.is_null() &&
+ last_image_record != last_image_paint_) {
new_candidate_detected = true;
OnLastImagePaintDetected(last_image_record);
}
diff --git a/third_party/blink/renderer/core/paint/image_paint_timing_detector_test.cc b/third_party/blink/renderer/core/paint/image_paint_timing_detector_test.cc
index 11a94af..bb93f8f 100644
--- a/third_party/blink/renderer/core/paint/image_paint_timing_detector_test.cc
+++ b/third_party/blink/renderer/core/paint/image_paint_timing_detector_test.cc
@@ -70,6 +70,10 @@
.id_record_map_.size();
}
+ void Analyze() {
+ return GetPaintTimingDetector().GetImagePaintTimingDetector().Analyze();
+ }
+
TimeTicks LargestPaintStoredResult() {
ImageRecord* record = GetPaintTimingDetector()
.GetImagePaintTimingDetector()
@@ -689,4 +693,13 @@
EXPECT_EQ(CountRecords(), 0u);
}
+TEST_F(ImagePaintTimingDetectorTest, NullTimeNoCrash) {
+ SetBodyInnerHTML(R"HTML(
+ <img id="target"></img>
+ )HTML");
+ SetImageAndPaint("target", 5, 5);
+ UpdateAllLifecyclePhasesForTest();
+ Analyze();
+}
+
} // namespace blink
diff --git a/third_party/blink/renderer/core/paint/text_paint_timing_detector.cc b/third_party/blink/renderer/core/paint/text_paint_timing_detector.cc
index 6ee2cf6..c5baafdb 100644
--- a/third_party/blink/renderer/core/paint/text_paint_timing_detector.cc
+++ b/third_party/blink/renderer/core/paint/text_paint_timing_detector.cc
@@ -59,7 +59,6 @@
const TextRecord& largest_text_record) {
largest_text_paint_ = largest_text_record.first_paint_time;
largest_text_paint_size_ = largest_text_record.first_size;
-
std::unique_ptr<TracedValue> value = TracedValue::Create();
PopulateTraceValue(*value, largest_text_record,
largest_text_candidate_index_max_++);
@@ -90,12 +89,16 @@
void TextPaintTimingDetector::Analyze() {
TextRecord* largest_text_first_paint = FindLargestPaintCandidate();
bool new_candidate_detected = false;
+ DCHECK(!largest_text_first_paint ||
+ !largest_text_first_paint->first_paint_time.is_null());
if (largest_text_first_paint &&
largest_text_first_paint->first_paint_time != largest_text_paint_) {
OnLargestTextDetected(*largest_text_first_paint);
new_candidate_detected = true;
}
TextRecord* last_text_first_paint = FindLastPaintCandidate();
+ DCHECK(!last_text_first_paint ||
+ !last_text_first_paint->first_paint_time.is_null());
if (last_text_first_paint &&
last_text_first_paint->first_paint_time != last_text_paint_) {
OnLastTextDetected(*last_text_first_paint);