[M77][LCP] Update LCP when both text and image are ready
Largest Contentful Paint compares the text and the image candidate
to decide which is the largest candidate. Currently, LCP generates
a result entry between updating the text candidate and the image
candidate, which produces an intermediate result. It causes
the issue in crbug.com/988115, #c4 explains the mechanism of the bug.
To remove the intermediate state, this CL changes the way
LCP-calculator takes the text candidate and image candidate. Instead
of notifying LCP-calculator of the text candidate's update right after
the update, we wait until both text candidate and image candidate have
been updated to notify LCP-calculator.
Also, LCP-calculator adds the logic of checking whether candidate
has changed, from ImagePaintTimingDetector and TextPaintTimingDetector.
Although it's redundant with |UpdateCandidate| in text and image,
these are necessary because going forwards, the "has_changed" logic in
both detectors would have to be removed along with the LIP and LTP
logic.
This CL also considers the chances where either detector is destroyed,
where it would no longer be able to find the largest candidate. In
this case, perf API would use the last reported candidate as the
candidate to compare with the candidate from another detector, in
order to decide the largest between text and image.
Bug: 982307, 988115
(cherry picked from commit 77c7dc37185a13f5e379b0e8338f0ed3bd0f1f01)
Change-Id: I562c7aeab6037678dd4bd7d6eddacdeb5be8bfbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1733456
Commit-Queue: Liquan (Max) Gu <[email protected]>
Reviewed-by: Steve Kobes <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#685300}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1749843
Reviewed-by: Nicolás Peña Moreno <[email protected]>
Cr-Commit-Position: refs/branch-heads/3865@{#333}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
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 a94e6abe..ac7d3b09 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
@@ -107,7 +107,7 @@
ToTraceValue(&frame_view_->GetFrame()));
}
-void ImagePaintTimingDetector::UpdateCandidate() {
+ImageRecord* ImagePaintTimingDetector::UpdateCandidate() {
ImageRecord* largest_image_record =
records_manager_.FindLargestPaintCandidate();
const base::TimeTicks time = largest_image_record
@@ -116,27 +116,26 @@
const uint64_t size =
largest_image_record ? largest_image_record->first_size : 0;
PaintTimingDetector& detector = frame_view_->GetPaintTimingDetector();
+ // Two different candidates are rare to have the same time and size.
+ // So when they are unchanged, the candidate is considered unchanged.
bool changed = detector.NotifyIfChangedLargestImagePaint(time, size);
- if (!changed)
- return;
- if (!time.is_null()) {
- if (auto* lcp_calculator = detector.GetLargestContentfulPaintCalculator())
- lcp_calculator->OnLargestImageUpdated(largest_image_record);
- // If an image has paint time, it must have been loaded.
- DCHECK(largest_image_record->loaded);
- ReportCandidateToTrace(*largest_image_record);
- } else {
- if (auto* lcp_calculator = detector.GetLargestContentfulPaintCalculator())
- lcp_calculator->OnLargestImageUpdated(nullptr);
- ReportNoCandidateToTrace();
+ if (changed) {
+ if (!time.is_null()) {
+ DCHECK(largest_image_record->loaded);
+ ReportCandidateToTrace(*largest_image_record);
+ } else {
+ ReportNoCandidateToTrace();
+ }
}
+ return largest_image_record;
}
void ImagePaintTimingDetector::OnPaintFinished() {
frame_index_++;
if (need_update_timing_at_frame_end_) {
need_update_timing_at_frame_end_ = false;
- UpdateCandidate();
+ frame_view_->GetPaintTimingDetector()
+ .UpdateLargestContentfulPaintCandidate();
}
if (!records_manager_.HasUnregisteredRecordsInQueued(
@@ -187,7 +186,6 @@
DCHECK(ThreadState::Current()->IsMainThread());
records_manager_.AssignPaintTimeToRegisteredQueuedRecords(
timestamp, last_queued_frame_index);
- UpdateCandidate();
num_pending_swap_callbacks_--;
DCHECK_GE(num_pending_swap_callbacks_, 0);
}
diff --git a/third_party/blink/renderer/core/paint/image_paint_timing_detector.h b/third_party/blink/renderer/core/paint/image_paint_timing_detector.h
index b458d31..087f860 100644
--- a/third_party/blink/renderer/core/paint/image_paint_timing_detector.h
+++ b/third_party/blink/renderer/core/paint/image_paint_timing_detector.h
@@ -228,6 +228,9 @@
}
void ReportSwapTime(unsigned last_queued_frame_index, base::TimeTicks);
+ // Return the candidate.
+ ImageRecord* UpdateCandidate();
+
void Trace(blink::Visitor*);
private:
@@ -241,8 +244,6 @@
void ReportNoCandidateToTrace();
void Deactivate();
- void UpdateCandidate();
-
// Used to find the last candidate.
unsigned count_candidates_ = 0;
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 99c2aab..05b8b742 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
@@ -140,9 +140,7 @@
}
void UpdateCandidate() {
- return GetPaintTimingDetector()
- .GetImagePaintTimingDetector()
- ->UpdateCandidate();
+ GetPaintTimingDetector().GetImagePaintTimingDetector()->UpdateCandidate();
}
base::TimeTicks LargestPaintStoredResult() {
@@ -206,6 +204,7 @@
MockPaintTimingCallbackManager* image_callback_manager) {
image_callback_manager->InvokeSwapTimeCallback(
test_task_runner_->NowTicks());
+ UpdateCandidate();
}
void SetImageAndPaint(AtomicString id, int width, int height) {
diff --git a/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.cc b/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.cc
index 9ce51de3..969a354 100644
--- a/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.cc
+++ b/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.cc
@@ -8,6 +8,32 @@
namespace blink {
+namespace {
+bool HasLargestTextChanged(const std::unique_ptr<TextRecord>& a,
+ const base::WeakPtr<TextRecord> b) {
+ if (!a && !b)
+ return false;
+ if (!a && b)
+ return true;
+ if (a && !b)
+ return true;
+ return a->node_id != b->node_id || a->first_size != b->first_size ||
+ a->paint_time != b->paint_time;
+}
+
+bool HasLargestImageChanged(const std::unique_ptr<ImageRecord>& a,
+ const ImageRecord* b) {
+ if (!a && !b)
+ return false;
+ if (!a && b)
+ return true;
+ if (a && !b)
+ return true;
+ return a->node_id != b->node_id || a->first_size != b->first_size ||
+ a->paint_time != b->paint_time || a->load_time != b->load_time;
+}
+} // namespace
+
LargestContentfulPaintCalculator::LargestContentfulPaintCalculator(
WindowPerformance* window_performance)
: window_performance_(window_performance) {}
@@ -23,16 +49,6 @@
largest_image_->cached_image = largest_image->cached_image;
largest_image_->load_time = largest_image->load_time;
}
-
- if (LargestImageSize() > LargestTextSize()) {
- // The new largest image is the largest content, so report it as the LCP.
- OnLargestContentfulPaintUpdated(LargestContentType::kImage);
- } else if (largest_text_ && last_type_ == LargestContentType::kImage) {
- // The text is at least as large as the new image. Because the last reported
- // content type was image, this means that the largest image is now smaller
- // and the largest text now needs to be reported as the LCP.
- OnLargestContentfulPaintUpdated(LargestContentType::kText);
- }
}
void LargestContentfulPaintCalculator::OnLargestTextUpdated(
@@ -43,24 +59,46 @@
largest_text->node_id, largest_text->first_size, FloatRect());
largest_text_->paint_time = largest_text->paint_time;
}
+}
+void LargestContentfulPaintCalculator::UpdateLargestContentPaintIfNeeded(
+ base::Optional<base::WeakPtr<TextRecord>> largest_text,
+ base::Optional<const ImageRecord*> largest_image) {
+ bool image_has_changed = false;
+ bool text_has_changed = false;
+ if (largest_image.has_value()) {
+ image_has_changed = HasLargestImageChanged(largest_image_, *largest_image);
+ OnLargestImageUpdated(*largest_image);
+ }
+ if (largest_text.has_value()) {
+ text_has_changed = HasLargestTextChanged(largest_text_, *largest_text);
+ OnLargestTextUpdated(*largest_text);
+ }
+ // If |largest_image| does not have value, the detector may have been
+ // destroyed. In this case, keep using its last candidate for comparison with
+ // the text candidate. The same for |largest_text|.
+ if ((!largest_image.has_value() || !image_has_changed) &&
+ (!largest_text.has_value() || !text_has_changed))
+ return;
+
+ if (!largest_text_ && !largest_image_)
+ return;
if (LargestTextSize() > LargestImageSize()) {
- // The new largest text is the largest content, so report it as the LCP.
- OnLargestContentfulPaintUpdated(LargestContentType::kText);
- } else if (largest_image_ && last_type_ == LargestContentType::kText) {
- // The image is at least as large as the new text. Because the last reported
- // content type was text, this means that the largest text is now smaller
- // and the largest image now needs to be reported as the LCP.
- OnLargestContentfulPaintUpdated(LargestContentType::kImage);
+ if (largest_text_->paint_time > base::TimeTicks())
+ UpdateLargestContentfulPaint(LargestContentType::kText);
+ } else {
+ if (largest_image_->paint_time > base::TimeTicks())
+ UpdateLargestContentfulPaint(LargestContentType::kImage);
}
}
-void LargestContentfulPaintCalculator::OnLargestContentfulPaintUpdated(
+void LargestContentfulPaintCalculator::UpdateLargestContentfulPaint(
LargestContentType type) {
DCHECK(window_performance_);
DCHECK(type != LargestContentType::kUnknown);
last_type_ = type;
if (type == LargestContentType::kImage) {
+ DCHECK(largest_image_);
const ImageResourceContent* cached_image = largest_image_->cached_image;
Node* image_node = DOMNodeIds::NodeForId(largest_image_->node_id);
@@ -77,13 +115,12 @@
const KURL& url = cached_image->Url();
auto* document = window_performance_->GetExecutionContext();
+ bool expose_paint_time_to_api = true;
if (!url.ProtocolIsData() &&
(!document || !Performance::PassesTimingAllowCheck(
cached_image->GetResponse(),
*document->GetSecurityOrigin(), document))) {
- // Reset the paint time of this image. It cannot be exposed to the
- // webexposed API.
- largest_image_->paint_time = base::TimeTicks();
+ expose_paint_time_to_api = false;
}
const String& image_url =
url.ProtocolIsData()
@@ -95,9 +132,12 @@
const AtomicString& image_id =
image_element ? image_element->GetIdAttribute() : AtomicString();
window_performance_->OnLargestContentfulPaintUpdated(
- largest_image_->paint_time, largest_image_->first_size,
- largest_image_->load_time, image_id, image_url, image_element);
+ expose_paint_time_to_api ? largest_image_->paint_time
+ : base::TimeTicks(),
+ largest_image_->first_size, largest_image_->load_time, image_id,
+ image_url, image_element);
} else {
+ DCHECK(largest_text_);
Node* text_node = DOMNodeIds::NodeForId(largest_text_->node_id);
// |text_node| could be null and |largest_text_| should be ignored in this
// case.
diff --git a/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.h b/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.h
index 3025610..5bfe5dc75 100644
--- a/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.h
+++ b/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator.h
@@ -19,9 +19,9 @@
public:
explicit LargestContentfulPaintCalculator(WindowPerformance*);
- void OnLargestImageUpdated(const ImageRecord* largest_image);
-
- void OnLargestTextUpdated(base::WeakPtr<TextRecord> largest_text);
+ void UpdateLargestContentPaintIfNeeded(
+ base::Optional<base::WeakPtr<TextRecord>> largest_text,
+ base::Optional<const ImageRecord*> largest_image);
void Trace(blink::Visitor* visitor);
@@ -33,7 +33,9 @@
kImage,
kText,
};
- void OnLargestContentfulPaintUpdated(LargestContentType type);
+ void OnLargestImageUpdated(const ImageRecord* largest_image);
+ void OnLargestTextUpdated(base::WeakPtr<TextRecord> largest_text);
+ void UpdateLargestContentfulPaint(LargestContentType type);
uint64_t LargestTextSize() {
return largest_text_ ? largest_text_->first_size : 0u;
diff --git a/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator_test.cc b/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator_test.cc
index f43b595..0173fe5 100644
--- a/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator_test.cc
+++ b/third_party/blink/renderer/core/paint/largest_contentful_paint_calculator_test.cc
@@ -18,6 +18,8 @@
class LargestContentfulPaintCalculatorTest : public RenderingTest {
public:
+ using LargestContentType =
+ LargestContentfulPaintCalculator::LargestContentType;
void SetUp() override {
// Advance the clock so we do not assign null TimeTicks.
simulated_clock_.Advance(base::TimeDelta::FromMilliseconds(100));
@@ -64,14 +66,8 @@
return original_image_resource;
}
- bool IsLastReportedImage() {
- return GetLargestContentfulPaintCalculator()->last_type_ ==
- LargestContentfulPaintCalculator::LargestContentType::kImage;
- }
-
- bool IsLastReportedText() {
- return GetLargestContentfulPaintCalculator()->last_type_ ==
- LargestContentfulPaintCalculator::LargestContentType::kText;
+ LargestContentType LastReportedType() {
+ return GetLargestContentfulPaintCalculator()->last_type_;
}
uint64_t LargestImageSize() {
@@ -82,14 +78,41 @@
return GetLargestContentfulPaintCalculator()->LargestTextSize();
}
+ void UpdateLargestContentfulPaintCandidate() {
+ GetFrame()
+ .View()
+ ->GetPaintTimingDetector()
+ .UpdateLargestContentfulPaintCandidate();
+ }
+
+ void SimulateContentSwapPromise() {
+ mock_text_callback_manager_->InvokeSwapTimeCallback(
+ simulated_clock_.NowTicks());
+ mock_image_callback_manager_->InvokeSwapTimeCallback(
+ simulated_clock_.NowTicks());
+ // Outside the tests, this is invoked by
+ // |PaintTimingCallbackManagerImpl::ReportPaintTime|.
+ UpdateLargestContentfulPaintCandidate();
+ }
+
+ // Outside the tests, the text callback and the image callback are run
+ // together, as in |SimulateContentSwapPromise|.
void SimulateImageSwapPromise() {
mock_image_callback_manager_->InvokeSwapTimeCallback(
simulated_clock_.NowTicks());
+ // Outside the tests, this is invoked by
+ // |PaintTimingCallbackManagerImpl::ReportPaintTime|.
+ UpdateLargestContentfulPaintCandidate();
}
+ // Outside the tests, the text callback and the image callback are run
+ // together, as in |SimulateContentSwapPromise|.
void SimulateTextSwapPromise() {
mock_text_callback_manager_->InvokeSwapTimeCallback(
simulated_clock_.NowTicks());
+ // Outside the tests, this is invoked by
+ // |PaintTimingCallbackManagerImpl::ReportPaintTime|.
+ UpdateLargestContentfulPaintCandidate();
}
private:
@@ -114,7 +137,7 @@
UpdateAllLifecyclePhasesForTest();
SimulateImageSwapPromise();
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
EXPECT_EQ(LargestImageSize(), 15000u);
EXPECT_EQ(LargestTextSize(), 0u);
}
@@ -126,7 +149,7 @@
)HTML");
UpdateAllLifecyclePhasesForTest();
SimulateTextSwapPromise();
- EXPECT_TRUE(IsLastReportedText());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kText);
}
TEST_F(LargestContentfulPaintCalculatorTest, ImageLargerText) {
@@ -138,10 +161,10 @@
SetImage("target", 3, 3);
UpdateAllLifecyclePhasesForTest();
SimulateImageSwapPromise();
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
SimulateTextSwapPromise();
- EXPECT_TRUE(IsLastReportedText());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kText);
EXPECT_EQ(LargestImageSize(), 9u);
EXPECT_GT(LargestTextSize(), 9u);
}
@@ -155,11 +178,11 @@
SetImage("target", 100, 200);
UpdateAllLifecyclePhasesForTest();
SimulateImageSwapPromise();
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
SimulateTextSwapPromise();
// Text should not be reported, since it is smaller than the image.
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
EXPECT_EQ(LargestImageSize(), 20000u);
EXPECT_GT(LargestTextSize(), 0u);
}
@@ -172,11 +195,9 @@
)HTML");
SetImage("target", 100, 200);
UpdateAllLifecyclePhasesForTest();
- SimulateTextSwapPromise();
- EXPECT_TRUE(IsLastReportedText());
- SimulateImageSwapPromise();
+ SimulateContentSwapPromise();
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
EXPECT_EQ(LargestImageSize(), 20000u);
EXPECT_GT(LargestTextSize(), 0u);
}
@@ -189,12 +210,10 @@
)HTML");
SetImage("target", 3, 3);
UpdateAllLifecyclePhasesForTest();
- SimulateTextSwapPromise();
- EXPECT_TRUE(IsLastReportedText());
- SimulateImageSwapPromise();
+ SimulateContentSwapPromise();
// Image should not be reported, since it is smaller than the text.
- EXPECT_TRUE(IsLastReportedText());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kText);
EXPECT_EQ(LargestImageSize(), 9u);
EXPECT_GT(LargestTextSize(), 9u);
}
@@ -212,7 +231,7 @@
SimulateImageSwapPromise();
SimulateTextSwapPromise();
// Image is larger than the text.
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
EXPECT_EQ(LargestImageSize(), 20000u);
EXPECT_GT(LargestTextSize(), 9u);
@@ -220,7 +239,7 @@
UpdateAllLifecyclePhasesForTest();
// The LCP should now be the text because it is larger than the remaining
// image.
- EXPECT_TRUE(IsLastReportedText());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kText);
EXPECT_EQ(LargestImageSize(), 9u);
EXPECT_GT(LargestTextSize(), 9u);
}
@@ -241,7 +260,7 @@
SimulateImageSwapPromise();
SimulateTextSwapPromise();
// Test is larger than the image.
- EXPECT_TRUE(IsLastReportedText());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kText);
EXPECT_EQ(LargestImageSize(), 50u);
EXPECT_GT(LargestTextSize(), 50u);
@@ -249,7 +268,7 @@
UpdateAllLifecyclePhasesForTest();
// The LCP should now be the image because it is larger than the remaining
// text.
- EXPECT_TRUE(IsLastReportedImage());
+ EXPECT_EQ(LastReportedType(), LargestContentType::kImage);
EXPECT_EQ(LargestImageSize(), 50u);
EXPECT_LT(LargestTextSize(), 50u);
}
diff --git a/third_party/blink/renderer/core/paint/paint_timing_detector.cc b/third_party/blink/renderer/core/paint/paint_timing_detector.cc
index 9385ee0..27c5b112 100644
--- a/third_party/blink/renderer/core/paint/paint_timing_detector.cc
+++ b/third_party/blink/renderer/core/paint/paint_timing_detector.cc
@@ -168,6 +168,7 @@
}
if (image_paint_timing_detector_)
image_paint_timing_detector_->StopRecordEntries();
+ largest_contentful_paint_calculator_ = nullptr;
}
void PaintTimingDetector::NotifyInputEvent(WebInputEvent::Type type) {
@@ -297,6 +298,33 @@
return float_rect;
}
+void PaintTimingDetector::UpdateLargestContentfulPaintCandidate() {
+ auto* lcp_calculator = GetLargestContentfulPaintCalculator();
+ if (!lcp_calculator)
+ return;
+
+ // Optional, WeakPtr, Record have different roles:
+ // * !Optional means |UpdateCandidate() is not reachable, e.g., user input
+ // has been given to stop LCP. In this case, we still use the last recorded
+ // result.
+ // * !Weak means there is no candidate, e.g., no content show up on the page.
+ // * Record.paint_time == 0 means there is an image but the image is still
+ // loading. The perf API should wait until the paint-time is available.
+ base::Optional<base::WeakPtr<TextRecord>> largest_text_record;
+ base::Optional<const ImageRecord*> largest_image_record;
+ if (auto* text_timing_detector = GetTextPaintTimingDetector()) {
+ if (text_timing_detector->IsRecordingLargestTextPaint()) {
+ largest_text_record.emplace(text_timing_detector->UpdateCandidate());
+ }
+ }
+ if (auto* image_timing_detector = GetImagePaintTimingDetector()) {
+ largest_image_record.emplace(image_timing_detector->UpdateCandidate());
+ }
+
+ lcp_calculator->UpdateLargestContentPaintIfNeeded(largest_text_record,
+ largest_image_record);
+}
+
ScopedPaintTimingDetectorBlockPaintHook*
ScopedPaintTimingDetectorBlockPaintHook::top_ = nullptr;
@@ -371,6 +399,7 @@
std::move(frame_callbacks->front()).Run(paint_time);
frame_callbacks->pop();
}
+ frame_view_->GetPaintTimingDetector().UpdateLargestContentfulPaintCandidate();
}
void PaintTimingCallbackManagerImpl::Trace(Visitor* visitor) {
diff --git a/third_party/blink/renderer/core/paint/paint_timing_detector.h b/third_party/blink/renderer/core/paint/paint_timing_detector.h
index 269d629..61da7ba 100644
--- a/third_party/blink/renderer/core/paint/paint_timing_detector.h
+++ b/third_party/blink/renderer/core/paint/paint_timing_detector.h
@@ -168,6 +168,9 @@
uint64_t LargestImagePaintSize() const { return largest_image_paint_size_; }
base::TimeTicks LargestTextPaint() const { return largest_text_paint_time_; }
uint64_t LargestTextPaintSize() const { return largest_text_paint_size_; }
+
+ void UpdateLargestContentfulPaintCandidate();
+
void Trace(Visitor* visitor);
private:
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 390ea30..49c1f4b 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
@@ -76,7 +76,7 @@
ToTraceValue(&frame_view_->GetFrame()));
}
-void LargestTextPaintManager::UpdateCandidate() {
+base::WeakPtr<TextRecord> LargestTextPaintManager::UpdateCandidate() {
base::WeakPtr<TextRecord> largest_text_record = FindLargestPaintCandidate();
const base::TimeTicks time =
largest_text_record ? largest_text_record->paint_time : base::TimeTicks();
@@ -85,27 +85,20 @@
DCHECK(paint_timing_detector_);
bool changed =
paint_timing_detector_->NotifyIfChangedLargestTextPaint(time, size);
- if (!changed)
- return;
-
- if (!time.is_null()) {
- if (auto* lcp_calculator =
- paint_timing_detector_->GetLargestContentfulPaintCalculator())
- lcp_calculator->OnLargestTextUpdated(largest_text_record);
- ReportCandidateToTrace(*largest_text_record);
- } else {
- if (auto* lcp_calculator =
- paint_timing_detector_->GetLargestContentfulPaintCalculator())
- lcp_calculator->OnLargestTextUpdated(nullptr);
- ReportNoCandidateToTrace();
+ if (changed) {
+ if (!time.is_null())
+ ReportCandidateToTrace(*largest_text_record);
+ else
+ ReportNoCandidateToTrace();
}
+ return largest_text_record;
}
void TextPaintTimingDetector::OnPaintFinished() {
if (need_update_timing_at_frame_end_) {
need_update_timing_at_frame_end_ = false;
- if (records_manager_.GetLargestTextPaintManager())
- records_manager_.GetLargestTextPaintManager()->UpdateCandidate();
+ frame_view_->GetPaintTimingDetector()
+ .UpdateLargestContentfulPaintCandidate();
}
if (records_manager_.NeedMeausuringPaintTime()) {
if (!awaiting_swap_promise_) {
@@ -150,8 +143,8 @@
}
}
records_manager_.AssignPaintTimeToQueuedRecords(timestamp);
- if (records_manager_.GetLargestTextPaintManager())
- records_manager_.GetLargestTextPaintManager()->UpdateCandidate();
+ if (IsRecordingLargestTextPaint())
+ UpdateCandidate();
awaiting_swap_promise_ = false;
}
diff --git a/third_party/blink/renderer/core/paint/text_paint_timing_detector.h b/third_party/blink/renderer/core/paint/text_paint_timing_detector.h
index 149c826..39c878d 100644
--- a/third_party/blink/renderer/core/paint/text_paint_timing_detector.h
+++ b/third_party/blink/renderer/core/paint/text_paint_timing_detector.h
@@ -69,7 +69,7 @@
void ReportCandidateToTrace(const TextRecord&);
void ReportNoCandidateToTrace();
- void UpdateCandidate();
+ base::WeakPtr<TextRecord> UpdateCandidate();
void PopulateTraceValue(TracedValue&, const TextRecord& first_text_paint);
inline void SetCachedResultInvalidated(bool value) {
is_result_invalidated_ = value;
@@ -145,8 +145,9 @@
text_element_timing_ = text_element_timing;
}
- inline base::Optional<LargestTextPaintManager>& GetLargestTextPaintManager() {
- return ltp_manager_;
+ inline base::WeakPtr<TextRecord> UpdateCandidate() {
+ DCHECK(ltp_manager_);
+ return ltp_manager_->UpdateCandidate();
}
inline bool IsRecordingLargestTextPaint() const {
@@ -212,6 +213,12 @@
void ResetCallbackManager(PaintTimingCallbackManager* manager) {
callback_manager_ = manager;
}
+ inline bool IsRecordingLargestTextPaint() const {
+ return records_manager_.IsRecordingLargestTextPaint();
+ }
+ inline base::WeakPtr<TextRecord> UpdateCandidate() {
+ return records_manager_.UpdateCandidate();
+ }
void ReportSwapTime(base::TimeTicks timestamp);
void Trace(blink::Visitor*);
diff --git a/third_party/blink/renderer/core/paint/text_paint_timing_detector_test.cc b/third_party/blink/renderer/core/paint/text_paint_timing_detector_test.cc
index b3171ed..23da9db 100644
--- a/third_party/blink/renderer/core/paint/text_paint_timing_detector_test.cc
+++ b/third_party/blink/renderer/core/paint/text_paint_timing_detector_test.cc
@@ -77,8 +77,7 @@
}
base::Optional<LargestTextPaintManager>& GetLargestTextPaintManager() {
- return GetTextPaintTimingDetector()
- ->records_manager_.GetLargestTextPaintManager();
+ return GetTextPaintTimingDetector()->records_manager_.ltp_manager_;
}
wtf_size_t CountVisibleTexts() {
@@ -89,9 +88,7 @@
wtf_size_t CountRankingSetSize() {
DCHECK(GetTextPaintTimingDetector());
- return GetTextPaintTimingDetector()
- ->records_manager_.GetLargestTextPaintManager()
- ->size_ordered_set_.size();
+ return GetLargestTextPaintManager()->size_ordered_set_.size();
}
wtf_size_t CountInvisibleTexts() {
@@ -126,6 +123,9 @@
void InvokeSwapTimeCallback(
MockPaintTimingCallbackManager* callback_manager) {
callback_manager->InvokeSwapTimeCallback(test_task_runner_->NowTicks());
+ // Outside the tests, this is invoked by
+ // |PaintTimingCallbackManagerImpl::ReportPaintTime|.
+ GetLargestTextPaintManager()->UpdateCandidate();
}
base::TimeTicks LargestPaintStoredResult() {
@@ -192,19 +192,14 @@
}
base::WeakPtr<TextRecord> TextRecordOfLargestTextPaint() {
- return GetFrameView()
- .GetPaintTimingDetector()
- .GetTextPaintTimingDetector()
- ->records_manager_.GetLargestTextPaintManager()
- ->FindLargestPaintCandidate();
+ return GetLargestTextPaintManager()->FindLargestPaintCandidate();
}
base::WeakPtr<TextRecord> ChildFrameTextRecordOfLargestTextPaint() {
return GetChildFrameView()
.GetPaintTimingDetector()
.GetTextPaintTimingDetector()
- ->records_manager_.GetLargestTextPaintManager()
- ->FindLargestPaintCandidate();
+ ->records_manager_.ltp_manager_->FindLargestPaintCandidate();
}
void SetFontSize(Element* font_element, uint16_t font_size) {
