From f764eced978627c0312c1ddbb8cc20c2c8166f48 Mon Sep 17 00:00:00 2001 From: Mikel Astiz Date: Wed, 27 Oct 2021 09:13:47 +0000 Subject: [PATCH] Revert "[LCP] Add animated image support" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b7d510c06e0436cfb4bd7260175cd460b949225c. Reason for revert: speculative revert for new flakiness in PageLoadMetricsBrowserTestWithAnimatedLCPFlag, see first failing build https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests%20%28dbg%29%281%29/99853 Original change's description: > [LCP] Add animated image support > > This CL adds support for better handling of animated images in LCP: > * A new attribute is exposing the first animated frame's paint time > (behind a flag). > * `startTime` is not changed. > * The PageLoadMetrics reported for LCP are set to that first frame paint > time for animated images (behind another flag). > * Entries are not emitted until the image is loaded. > > Relevant spec issue: > https://github.com/WICG/largest-contentful-paint/issues/83 > > Change-Id: I6bb01eacb4f200f9c032ffcfcd9a1a41126a7773 > Bug: 1260953 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3226157 > Commit-Queue: Yoav Weiss > Reviewed-by: Nicolás Peña Moreno > Cr-Commit-Position: refs/heads/main@{#935133} Bug: 1260953 Change-Id: I00878689de95de38645195da81c6baf577d868a6 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3247071 Auto-Submit: Mikel Astiz Bot-Commit: Rubber Stamper Commit-Queue: Mikel Astiz Owners-Override: Mikel Astiz Cr-Commit-Position: refs/heads/main@{#935350} --- .../page_load_metrics_browsertest.cc | 152 ------------------ content/test/data/animated.png | Bin 460 -> 0 bytes third_party/blink/common/features.cc | 5 - third_party/blink/public/common/features.h | 2 - .../loader/resource/image_resource_content.cc | 7 +- .../loader/resource/image_resource_content.h | 1 - .../core/paint/image_paint_timing_detector.cc | 87 +++------- .../core/paint/image_paint_timing_detector.h | 10 +- .../largest_contentful_paint_calculator.cc | 11 +- .../largest_contentful_paint_calculator.h | 4 +- .../core/paint/paint_timing_detector.cc | 35 ++-- .../core/timing/largest_contentful_paint.cc | 19 +-- .../core/timing/largest_contentful_paint.h | 5 - .../core/timing/largest_contentful_paint.idl | 1 - .../core/timing/window_performance.cc | 6 +- .../renderer/core/timing/window_performance.h | 14 +- .../platform/runtime_enabled_features.json5 | 4 - .../external/wpt/images/anim-tao.png | Bin 460 -> 0 bytes .../external/wpt/images/anim-tao.png.headers | 2 - .../external/wpt/images/webp-animated.webp | Bin 340 -> 0 bytes .../observe-animated-image-gif.tentative.html | 27 ---- ...observe-animated-image-webp.tentative.html | 27 ---- .../observe-animated-image.tentative.html | 29 ---- ...cross-origin-animated-image.tentative.html | 30 ---- ...s-origin-tao-animated-image.tentative.html | 30 ---- .../observe-non-animated-image.tentative.html | 27 ---- .../largest-contentful-paint-helpers.js | 32 ---- .../global-interface-listing-expected.txt | 1 - 28 files changed, 57 insertions(+), 511 deletions(-) delete mode 100644 content/test/data/animated.png delete mode 100644 third_party/blink/web_tests/external/wpt/images/anim-tao.png delete mode 100644 third_party/blink/web_tests/external/wpt/images/anim-tao.png.headers delete mode 100644 third_party/blink/web_tests/external/wpt/images/webp-animated.webp delete mode 100644 third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image-gif.tentative.html delete mode 100644 third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image-webp.tentative.html delete mode 100644 third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image.tentative.html delete mode 100644 third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-animated-image.tentative.html delete mode 100644 third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-tao-animated-image.tentative.html delete mode 100644 third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-non-animated-image.tentative.html diff --git a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc index 3a154d28f6b4b9..be75edbc89119a 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc @@ -12,7 +12,6 @@ #include "base/bind.h" #include "base/check_op.h" -#include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" @@ -77,8 +76,6 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" #include "content/public/common/content_features.h" -#include "content/public/common/content_paths.h" -#include "content/public/common/content_switches.h" #include "content/public/common/referrer.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" @@ -379,134 +376,6 @@ class PageLoadMetricsBrowserTest : public InProcessBrowserTest { std::unique_ptr test_ukm_recorder_; }; -class PageLoadMetricsBrowserTestAnimatedLCP - : public PageLoadMetricsBrowserTest { - protected: - void test_animated_image_lcp(bool smaller, bool animated) { - // Waiter to ensure main content is loaded. - auto waiter = CreatePageLoadMetricsTestWaiter(); - waiter->AddPageExpectation(TimingField::kLoadEvent); - waiter->AddPageExpectation(TimingField::kFirstContentfulPaint); - waiter->AddPageExpectation(TimingField::kLargestContentfulPaint); - - const char kHtmlHttpResponseHeader[] = - "HTTP/1.1 200 OK\r\n" - "Content-Type: text/html; charset=utf-8\r\n" - "\r\n"; - const char kImgHttpResponseHeader[] = - "HTTP/1.1 200 OK\r\n" - "Content-Type: image/png\r\n" - "\r\n"; - auto main_html_response = - std::make_unique( - embedded_test_server(), "/mock_page.html", - false /*relative_url_is_prefix*/); - auto img_response = - std::make_unique( - embedded_test_server(), - animated ? "/images/animated-delayed.png" : "/images/delayed.jpg", - false /*relative_url_is_prefix*/); - - ASSERT_TRUE(embedded_test_server()->Start()); - - // File is under content/test/data/ - const std::string file_name_string = - animated ? "animated.png" : "single_face.jpg"; - std::string file_contents; - // The first_frame_size number for the animated case (262), represents the - // first frame of the animated PNG + an extra chunk enabling the decoder to - // understand the first frame is done and decode it. - // For the non-animated case (5000), it's an arbitrary number that - // represents a part of the JPEG's frame. - const unsigned first_frame_size = animated ? 262 : 5000; - - // Read the animated image into two frames. - { - base::ScopedAllowBlockingForTesting allow_io; - base::FilePath test_dir; - ASSERT_TRUE(base::PathService::Get(content::DIR_TEST_DATA, &test_dir)); - base::FilePath file_name = test_dir.AppendASCII(file_name_string); - ASSERT_TRUE(base::ReadFileToString(file_name, &file_contents)); - } - // Split the contents into 2 frames - std::string first_frame = file_contents.substr(0, first_frame_size); - std::string second_frame = file_contents.substr(first_frame_size); - - browser()->OpenURL(content::OpenURLParams( - embedded_test_server()->GetURL("/mock_page.html"), content::Referrer(), - WindowOpenDisposition::CURRENT_TAB, ui::PAGE_TRANSITION_TYPED, false)); - - main_html_response->WaitForRequest(); - main_html_response->Send(kHtmlHttpResponseHeader); - main_html_response->Send( - animated ? "" - : ""); - main_html_response->Done(); - - img_response->WaitForRequest(); - img_response->Send(kImgHttpResponseHeader); - img_response->Send(first_frame); - - // Trigger a double rAF and take a timestamp afterwards. - content::EvalJsResult result = - EvalJs(browser()->tab_strip_model()->GetActiveWebContents(), - "(async () => {" - "const double_raf = () => {" - "return new Promise(r => {" - "requestAnimationFrame(()=>requestAnimationFrame(r))})};" - "await double_raf();})()"); - EXPECT_EQ("", result.error); - content::EvalJsResult result2 = - EvalJs(browser()->tab_strip_model()->GetActiveWebContents(), - "performance.now()"); - EXPECT_EQ("", result2.error); - double timestamp = result2.ExtractDouble(); - - img_response->Send(second_frame); - img_response->Done(); - - waiter->Wait(); - - // LCP is collected only at the end of the page lifecycle. Navigate to - // flush. - NavigateToUntrackedUrl(); - - histogram_tester_->ExpectTotalCount( - internal::kHistogramLargestContentfulPaint, 1); - auto value = - histogram_tester_ - ->GetAllSamples(internal::kHistogramLargestContentfulPaint)[0] - .min; - - if (smaller) { - ASSERT_LT(value, timestamp); - } else { - ASSERT_GT(value, timestamp); - } - } -}; - -class PageLoadMetricsBrowserTestWithAnimatedLCPFlag - : public PageLoadMetricsBrowserTestAnimatedLCP { - public: - PageLoadMetricsBrowserTestWithAnimatedLCPFlag() { - scoped_feature_list_.Reset(); - scoped_feature_list_.InitWithFeatures( - {blink::features::kLCPAnimatedImagesReporting}, {}); - } -}; - -class PageLoadMetricsBrowserTestWithRuntimeAnimatedLCPFlag - : public PageLoadMetricsBrowserTestAnimatedLCP { - public: - void SetUpCommandLine(base::CommandLine* command_line) override { - command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures, - "LCPAnimatedImagesWebExposed"); - } -}; - IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoNavigation) { ASSERT_TRUE(embedded_test_server()->Start()); EXPECT_TRUE(NoPageLoadMetricsRecorded()) @@ -3120,27 +2989,6 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PageLCPStopsUponInput) { ASSERT_EQ(all_frames_value, main_frame_value); } -// Tests that an animated image's reported LCP values are smaller than its load -// times, when the feature flag for animated image reporting is enabled. -IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTestWithAnimatedLCPFlag, - PageLCPAnimatedImage) { - test_animated_image_lcp(/*smaller=*/true, /*animated=*/true); -} - -// Tests that an animated image's reported LCP values are larger than its load -// times, when only the feature flag for animated image web exposure is enabled. -IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTestWithRuntimeAnimatedLCPFlag, - PageLCPAnimatedImageOnlyRuntimeFlag) { - test_animated_image_lcp(/*smaller=*/false, /*animated=*/true); -} - -// Tests that a non-animated image's reported LCP values are larger than its -// load times, when the feature flag for animated image reporting is enabled. -IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTestWithAnimatedLCPFlag, - PageLCPNonAnimatedImage) { - test_animated_image_lcp(/*smaller=*/false, /*animated=*/false); -} - IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, FirstInputDelayFromClick) { ASSERT_TRUE(embedded_test_server()->Start()); diff --git a/content/test/data/animated.png b/content/test/data/animated.png deleted file mode 100644 index 925e2efc9a97ade490f04d57271adc10b2fe69b6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 460 zcmeAS@N?(olHy`uVBq!ia0vp^DL`z*!3HE(nbz$CQXGlNAwEEw35Xd!_f9SVQc`IU zF^~{g1Bd|zjLa_>8Ci1NIDx$Bo-U3d6?5L6GvoytbHL!w?6zvIV;iqHq?@gO(Ktbl zoS|)rq)@U)kD_7Q5eZr|_$v>-*5=KZ^ai?#!PC{xWt~$( F698Hck68c! diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc index e4b40d2e63659e..4e11dd49ce6666 100644 --- a/third_party/blink/common/features.cc +++ b/third_party/blink/common/features.cc @@ -1106,10 +1106,5 @@ const base::Feature kDeprecationWillLogToConsole{ const base::Feature kDeprecationWillLogToDevToolsIssue{ "DeprecationWillLogToDevToolsIssue", base::FEATURE_DISABLED_BY_DEFAULT}; -// Enables reporting and web-exposure (respectively) of the time the first frame -// of an animated image was painted. -const base::Feature kLCPAnimatedImagesReporting{ - "LCPAnimatedImagesReporting", base::FEATURE_DISABLED_BY_DEFAULT}; - } // namespace features } // namespace blink diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h index 9c41b060eed879..986b34c947556e 100644 --- a/third_party/blink/public/common/features.h +++ b/third_party/blink/public/common/features.h @@ -522,8 +522,6 @@ BLINK_COMMON_EXPORT extern const base::Feature kDeprecationWillLogToConsole; BLINK_COMMON_EXPORT extern const base::Feature kDeprecationWillLogToDevToolsIssue; -BLINK_COMMON_EXPORT extern const base::Feature kLCPAnimatedImagesReporting; - } // namespace features } // namespace blink diff --git a/third_party/blink/renderer/core/loader/resource/image_resource_content.cc b/third_party/blink/renderer/core/loader/resource/image_resource_content.cc index 6662e6857d7eb8..bc333d664bb957 100644 --- a/third_party/blink/renderer/core/loader/resource/image_resource_content.cc +++ b/third_party/blink/renderer/core/loader/resource/image_resource_content.cc @@ -626,12 +626,7 @@ ResourceStatus ImageResourceContent::GetContentStatus() const { return content_status_; } -bool ImageResourceContent::IsAnimatedImageWithPaintedFirstFrame() const { - return (image_ && !image_->IsNull() && image_->MaybeAnimated() && - image_->CurrentFrameIsComplete()); -} - -// TODO(hiroshige): Consider removing the following methods, or stopping +// TODO(hiroshige): Consider removing the following methods, or stoping // redirecting to ImageResource. const KURL& ImageResourceContent::Url() const { return info_->Url(); diff --git a/third_party/blink/renderer/core/loader/resource/image_resource_content.h b/third_party/blink/renderer/core/loader/resource/image_resource_content.h index 57f333c400ae33..572c7369976f31 100644 --- a/third_party/blink/renderer/core/loader/resource/image_resource_content.h +++ b/third_party/blink/renderer/core/loader/resource/image_resource_content.h @@ -109,7 +109,6 @@ class CORE_EXPORT ImageResourceContent final bool IsLoading() const; bool ErrorOccurred() const; bool LoadFailedOrCanceled() const; - bool IsAnimatedImageWithPaintedFirstFrame() const; // Redirecting methods to Resource. const KURL& Url() const; 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 24a895df72a74b..2d1a2418339f04 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 @@ -53,11 +53,6 @@ uint64_t DownScaleIfIntrinsicSizeIsSmaller( return visual_size; } -bool ShouldReportAnimatedImages() { - return (RuntimeEnabledFeatures::LCPAnimatedImagesWebExposedEnabled() || - base::FeatureList::IsEnabled(features::kLCPAnimatedImagesReporting)); -} - } // namespace static bool LargeImageFirst(const base::WeakPtr& a, @@ -106,7 +101,6 @@ void ImagePaintTimingDetector::ReportCandidateToTrace( DCHECK(!largest_image_record.paint_time.is_null()); auto value = std::make_unique(); PopulateTraceValue(*value, largest_image_record); - // TODO(yoav): Report first animated frame times as well. TRACE_EVENT_MARK_WITH_TIMESTAMP2("loading", "LargestImagePaint::Candidate", largest_image_record.paint_time, "data", std::move(value), "frame", @@ -129,29 +123,20 @@ void ImagePaintTimingDetector::ReportNoCandidateToTrace() { ImageRecord* ImagePaintTimingDetector::UpdateCandidate() { ImageRecord* largest_image_record = records_manager_.FindLargestPaintCandidate(); - base::TimeTicks time = largest_image_record ? largest_image_record->paint_time - : base::TimeTicks(); - // This doesn't use ShouldReportAnimatedImages(), as it should only update the - // record when the base::Feature is enabled, regardless of the runtime-enabled - // flag. - if (base::FeatureList::IsEnabled(features::kLCPAnimatedImagesReporting) && - largest_image_record && - !largest_image_record->first_animated_frame_time.is_null()) { - time = largest_image_record->first_animated_frame_time; - } + const base::TimeTicks time = largest_image_record + ? largest_image_record->paint_time + : base::TimeTicks(); const uint64_t size = largest_image_record ? largest_image_record->first_size : 0; PaintTimingDetector& detector = frame_view_->GetPaintTimingDetector(); - // Calling NotifyIfChangedLargestImagePaint only has an impact on - // PageLoadMetrics, and not on the web exposed metrics. - // // 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, records_manager_.LargestRemovedImagePaintTime(), records_manager_.LargestRemovedImageSize()); if (changed) { - if (!time.is_null() && largest_image_record->loaded) { + if (!time.is_null()) { + DCHECK(largest_image_record->loaded); ReportCandidateToTrace(*largest_image_record); } else { ReportNoCandidateToTrace(); @@ -181,7 +166,7 @@ void ImagePaintTimingDetector::NotifyImageRemoved( const LayoutObject& object, const ImageResourceContent* cached_image) { RecordId record_id = std::make_pair(&object, cached_image); - records_manager_.RemoveImageTimeRecords(record_id); + records_manager_.RemoveImageFinishedRecord(record_id); records_manager_.RemoveInvisibleRecordIfNeeded(record_id); if (!records_manager_.IsRecordedVisibleImage(record_id)) return; @@ -230,13 +215,7 @@ void ImageRecordsManager::AssignPaintTimeToRegisteredQueuedRecords( } if (record->frame_index > last_queued_frame_index) break; - if (record->loaded) { - record->paint_time = timestamp; - } - if (record->queue_animated_paint) { - record->first_animated_frame_time = timestamp; - record->queue_animated_paint = false; - } + record->paint_time = timestamp; images_queued_for_paint_time_.pop_front(); } } @@ -249,7 +228,6 @@ void ImagePaintTimingDetector::RecordImage( const StyleFetchedImage* style_image, const IntRect& image_border) { Node* node = object.GetNode(); - if (!node) return; @@ -291,28 +269,25 @@ void ImagePaintTimingDetector::RecordImage( return; } - if (is_recorded_visible_image) { - if (ShouldReportAnimatedImages() && - cached_image.IsAnimatedImageWithPaintedFirstFrame()) { - need_update_timing_at_frame_end_ |= - records_manager_.OnFirstAnimatedFramePainted(record_id, frame_index_); - } - if (!records_manager_.IsVisibleImageLoaded(record_id) && - cached_image.IsLoaded()) { - records_manager_.OnImageLoaded(record_id, frame_index_, style_image); - need_update_timing_at_frame_end_ = true; - if (absl::optional& visualizer = - frame_view_->GetPaintTimingDetector().Visualizer()) { - FloatRect mapped_visual_rect = - frame_view_->GetPaintTimingDetector().CalculateVisualRect( - image_border, current_paint_chunk_properties); - visualizer->DumpImageDebuggingRect(object, mapped_visual_rect, - cached_image); - } + if (is_recorded_visible_image && + !records_manager_.IsVisibleImageLoaded(record_id) && + cached_image.IsLoaded()) { + records_manager_.OnImageLoaded(record_id, frame_index_, style_image); + need_update_timing_at_frame_end_ = true; + if (absl::optional& visualizer = + frame_view_->GetPaintTimingDetector().Visualizer()) { + FloatRect mapped_visual_rect = + frame_view_->GetPaintTimingDetector().CalculateVisualRect( + image_border, current_paint_chunk_properties); + visualizer->DumpImageDebuggingRect(object, mapped_visual_rect, + cached_image); } return; } + if (is_recorded_visible_image) + return; + FloatRect mapped_visual_rect = frame_view_->GetPaintTimingDetector().CalculateVisualRect( image_border, current_paint_chunk_properties); @@ -324,11 +299,6 @@ void ImagePaintTimingDetector::RecordImage( } else { records_manager_.RecordVisible(record_id, rect_size, image_border, mapped_visual_rect); - if (ShouldReportAnimatedImages() && - cached_image.IsAnimatedImageWithPaintedFirstFrame()) { - need_update_timing_at_frame_end_ |= - records_manager_.OnFirstAnimatedFramePainted(record_id, frame_index_); - } if (cached_image.IsLoaded()) { records_manager_.OnImageLoaded(record_id, frame_index_, style_image); need_update_timing_at_frame_end_ = true; @@ -396,19 +366,6 @@ void ImagePaintTimingDetector::ReportLargestIgnoredImage() { ImageRecordsManager::ImageRecordsManager(LocalFrameView* frame_view) : size_ordered_set_(&LargeImageFirst), frame_view_(frame_view) {} -bool ImageRecordsManager::OnFirstAnimatedFramePainted( - const RecordId& record_id, - unsigned current_frame_index) { - base::WeakPtr record = FindVisibleRecord(record_id); - DCHECK(record); - if (record->first_animated_frame_time.is_null()) { - record->queue_animated_paint = true; - QueueToMeasurePaintTime(record, current_frame_index); - return true; - } - return false; -} - void ImageRecordsManager::OnImageLoaded(const RecordId& record_id, unsigned current_frame_index, const StyleFetchedImage* style_image) { 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 6da959eafc64bd..c9bedf793782a5 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 @@ -56,10 +56,7 @@ class ImageRecord : public base::SupportsWeakPtr { // The time of the first paint after fully loaded. 0 means not painted yet. base::TimeTicks paint_time = base::TimeTicks(); base::TimeTicks load_time = base::TimeTicks(); - base::TimeTicks first_animated_frame_time = base::TimeTicks(); bool loaded = false; - // An animated frame is queued for paint timing. - bool queue_animated_paint = false; // LCP rect information, only populated when tracing is enabled. std::unique_ptr lcp_rect_info_; }; @@ -91,7 +88,7 @@ class CORE_EXPORT ImageRecordsManager { invisible_images_.erase(record_id); } - inline void RemoveImageTimeRecords(const RecordId& record_id) { + inline void RemoveImageFinishedRecord(const RecordId& record_id) { image_finished_times_.erase(record_id); } @@ -136,17 +133,14 @@ class CORE_EXPORT ImageRecordsManager { // not currently the case. If we plumb some information from // ImageResourceContent we may be able to ensure that this call does not // require the Contains() check, which would save time. - if (!image_finished_times_.Contains(record_id)) { + if (!image_finished_times_.Contains(record_id)) image_finished_times_.insert(record_id, base::TimeTicks::Now()); - } } inline bool IsVisibleImageLoaded(const RecordId& record_id) const { DCHECK(visible_images_.Contains(record_id)); return visible_images_.at(record_id)->loaded; } - bool OnFirstAnimatedFramePainted(const RecordId&, - unsigned current_frame_index); void OnImageLoaded(const RecordId&, unsigned current_frame_index, const StyleFetchedImage*); 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 267ff338a07109..3c97f1992f7a7b 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 @@ -22,7 +22,7 @@ LargestContentfulPaintCalculator::LargestContentfulPaintCalculator( WindowPerformance* window_performance) : window_performance_(window_performance) {} -void LargestContentfulPaintCalculator::UpdateLargestContentfulPaintIfNeeded( +void LargestContentfulPaintCalculator::UpdateLargestContentPaintIfNeeded( const TextRecord* largest_text, const ImageRecord* largest_image) { uint64_t text_size = largest_text ? largest_text->first_size : 0u; @@ -72,12 +72,9 @@ void LargestContentfulPaintCalculator::UpdateLargestContentfulImage( image_element ? image_element->GetIdAttribute() : AtomicString(); window_performance_->OnLargestContentfulPaintUpdated( expose_paint_time_to_api ? largest_image->paint_time : base::TimeTicks(), - largest_image->first_size, largest_image->load_time, - expose_paint_time_to_api ? largest_image->first_animated_frame_time - : base::TimeTicks(), - image_id, image_url, image_element); + largest_image->first_size, largest_image->load_time, image_id, image_url, + image_element); - // TODO: update trace value with animated frame data if (LocalDOMWindow* window = window_performance_->DomWindow()) { TRACE_EVENT_MARK_WITH_TIMESTAMP2(kTraceCategories, kLCPCandidate, largest_image->paint_time, "data", @@ -103,7 +100,7 @@ void LargestContentfulPaintCalculator::UpdateLargestContentfulText( text_element ? text_element->GetIdAttribute() : AtomicString(); window_performance_->OnLargestContentfulPaintUpdated( largest_text.paint_time, largest_text.first_size, base::TimeTicks(), - base::TimeTicks(), text_id, g_empty_string, text_element); + text_id, g_empty_string, text_element); if (LocalDOMWindow* window = window_performance_->DomWindow()) { TRACE_EVENT_MARK_WITH_TIMESTAMP2(kTraceCategories, kLCPCandidate, 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 fd2991736901af..e0a32e261629ab 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 @@ -23,8 +23,8 @@ class CORE_EXPORT LargestContentfulPaintCalculator final LargestContentfulPaintCalculator& operator=( const LargestContentfulPaintCalculator&) = delete; - void UpdateLargestContentfulPaintIfNeeded(const TextRecord* largest_text, - const ImageRecord* largest_image); + void UpdateLargestContentPaintIfNeeded(const TextRecord* largest_text, + const ImageRecord* largest_image); void Trace(Visitor* visitor) const; 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 4686e5b26aef96..27ded06284bd6e 100644 --- a/third_party/blink/renderer/core/paint/paint_timing_detector.cc +++ b/third_party/blink/renderer/core/paint/paint_timing_detector.cc @@ -33,7 +33,6 @@ #include "third_party/blink/renderer/platform/graphics/paint/property_tree_state.h" #include "third_party/blink/renderer/platform/graphics/paint/scoped_paint_chunk_properties.h" #include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h" -#include "third_party/blink/renderer/platform/runtime_enabled_features.h" #include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h" #include "third_party/blink/renderer/platform/wtf/functional.h" @@ -123,23 +122,14 @@ void PaintTimingDetector::NotifyBackgroundImagePaint( LocalFrameView* frame_view = object->GetFrameView(); if (!frame_view) return; - - ImagePaintTimingDetector* detector = - frame_view->GetPaintTimingDetector().GetImagePaintTimingDetector(); - if (!detector) + PaintTimingDetector& detector = frame_view->GetPaintTimingDetector(); + if (!detector.GetImagePaintTimingDetector()) return; - if (!IsBackgroundImageContentful(*object, image)) return; - - ImageResourceContent* cached_image = style_image.CachedImage(); - DCHECK(cached_image); - // TODO(yoav): |image| and |cached_image.GetImage()| are not the same here in - // the case of SVGs. Figure out why and if we can remove this footgun. - - detector->RecordImage(*object, image.Size(), *cached_image, - current_paint_chunk_properties, &style_image, - image_border); + detector.GetImagePaintTimingDetector()->RecordImage( + *object, image.Size(), *style_image.CachedImage(), + current_paint_chunk_properties, &style_image, image_border); } // static @@ -154,13 +144,12 @@ void PaintTimingDetector::NotifyImagePaint( LocalFrameView* frame_view = object.GetFrameView(); if (!frame_view) return; - ImagePaintTimingDetector* detector = - frame_view->GetPaintTimingDetector().GetImagePaintTimingDetector(); - if (!detector) + PaintTimingDetector& detector = frame_view->GetPaintTimingDetector(); + if (!detector.GetImagePaintTimingDetector()) return; - - detector->RecordImage(object, intrinsic_size, cached_image, - current_paint_chunk_properties, nullptr, image_border); + detector.GetImagePaintTimingDetector()->RecordImage( + object, intrinsic_size, cached_image, current_paint_chunk_properties, + nullptr, image_border); } void PaintTimingDetector::NotifyImageFinished( @@ -397,8 +386,8 @@ void PaintTimingDetector::UpdateLargestContentfulPaintCandidate() { largest_image_record = image_timing_detector->UpdateCandidate(); } - lcp_calculator->UpdateLargestContentfulPaintIfNeeded(largest_text_record, - largest_image_record); + lcp_calculator->UpdateLargestContentPaintIfNeeded(largest_text_record, + largest_image_record); } void PaintTimingDetector::ReportIgnoredContent() { diff --git a/third_party/blink/renderer/core/timing/largest_contentful_paint.cc b/third_party/blink/renderer/core/timing/largest_contentful_paint.cc index e3d3dfd2eef635..fca205e81c52e5 100644 --- a/third_party/blink/renderer/core/timing/largest_contentful_paint.cc +++ b/third_party/blink/renderer/core/timing/largest_contentful_paint.cc @@ -11,20 +11,17 @@ namespace blink { -LargestContentfulPaint::LargestContentfulPaint( - double start_time, - base::TimeDelta render_time, - uint64_t size, - base::TimeDelta load_time, - base::TimeDelta first_animated_frame_time, - const AtomicString& id, - const String& url, - Element* element) +LargestContentfulPaint::LargestContentfulPaint(double start_time, + base::TimeDelta render_time, + uint64_t size, + base::TimeDelta load_time, + const AtomicString& id, + const String& url, + Element* element) : PerformanceEntry(g_empty_atom, start_time, start_time), size_(size), render_time_(render_time), load_time_(load_time), - first_animated_frame_time_(first_animated_frame_time), id_(id), url_(url), element_(element) {} @@ -56,8 +53,6 @@ void LargestContentfulPaint::BuildJSONValue(V8ObjectBuilder& builder) const { builder.Add("size", size_); builder.Add("renderTime", render_time_.InMillisecondsF()); builder.Add("loadTime", load_time_.InMillisecondsF()); - builder.Add("firstAnimatedFrameTime", - first_animated_frame_time_.InMillisecondsF()); builder.Add("id", id_); builder.Add("url", url_); } diff --git a/third_party/blink/renderer/core/timing/largest_contentful_paint.h b/third_party/blink/renderer/core/timing/largest_contentful_paint.h index 915e1b029f5e1b..97b66eb1e53ce6 100644 --- a/third_party/blink/renderer/core/timing/largest_contentful_paint.h +++ b/third_party/blink/renderer/core/timing/largest_contentful_paint.h @@ -22,7 +22,6 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry { base::TimeDelta render_time, uint64_t size, base::TimeDelta load_time, - base::TimeDelta first_animated_frame_time, const AtomicString& id, const String& url, Element*); @@ -36,9 +35,6 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry { return render_time_.InMillisecondsF(); } DOMHighResTimeStamp loadTime() const { return load_time_.InMillisecondsF(); } - DOMHighResTimeStamp firstAnimatedFrameTime() const { - return first_animated_frame_time_.InMillisecondsF(); - } const AtomicString& id() const { return id_; } const String& url() const { return url_; } Element* element() const; @@ -51,7 +47,6 @@ class CORE_EXPORT LargestContentfulPaint final : public PerformanceEntry { uint64_t size_; base::TimeDelta render_time_; base::TimeDelta load_time_; - base::TimeDelta first_animated_frame_time_; AtomicString id_; String url_; WeakMember element_; diff --git a/third_party/blink/renderer/core/timing/largest_contentful_paint.idl b/third_party/blink/renderer/core/timing/largest_contentful_paint.idl index 41e7faf9b029c0..0d3ae7b0cb1e75 100644 --- a/third_party/blink/renderer/core/timing/largest_contentful_paint.idl +++ b/third_party/blink/renderer/core/timing/largest_contentful_paint.idl @@ -7,7 +7,6 @@ interface LargestContentfulPaint : PerformanceEntry { readonly attribute DOMHighResTimeStamp renderTime; readonly attribute DOMHighResTimeStamp loadTime; - [RuntimeEnabled=LCPAnimatedImagesWebExposed] readonly attribute DOMHighResTimeStamp firstAnimatedFrameTime; readonly attribute unsigned long long size; readonly attribute DOMString id; readonly attribute DOMString url; diff --git a/third_party/blink/renderer/core/timing/window_performance.cc b/third_party/blink/renderer/core/timing/window_performance.cc index dd15fdf3e65727..1a227a2b4c24d2 100644 --- a/third_party/blink/renderer/core/timing/window_performance.cc +++ b/third_party/blink/renderer/core/timing/window_performance.cc @@ -638,20 +638,16 @@ void WindowPerformance::OnLargestContentfulPaintUpdated( base::TimeTicks paint_time, uint64_t paint_size, base::TimeTicks load_time, - base::TimeTicks first_animated_frame_time, const AtomicString& id, const String& url, Element* element) { base::TimeDelta render_timestamp = MonotonicTimeToTimeDelta(paint_time); base::TimeDelta load_timestamp = MonotonicTimeToTimeDelta(load_time); - base::TimeDelta first_animated_frame_timestamp = - MonotonicTimeToTimeDelta(first_animated_frame_time); - // TODO(yoav): Should we modify start to represent the animated frame? base::TimeDelta start_timestamp = render_timestamp.is_zero() ? load_timestamp : render_timestamp; auto* entry = MakeGarbageCollected( start_timestamp.InMillisecondsF(), render_timestamp, paint_size, - load_timestamp, first_animated_frame_timestamp, id, url, element); + load_timestamp, id, url, element); if (HasObserverFor(PerformanceEntry::kLargestContentfulPaint)) NotifyObserversOfEntry(*entry); AddLargestContentfulPaint(entry); diff --git a/third_party/blink/renderer/core/timing/window_performance.h b/third_party/blink/renderer/core/timing/window_performance.h index e3a4432b774910..4362b11cf9b696 100644 --- a/third_party/blink/renderer/core/timing/window_performance.h +++ b/third_party/blink/renderer/core/timing/window_performance.h @@ -146,14 +146,12 @@ class CORE_EXPORT WindowPerformance final : public Performance, // PageVisibilityObserver void PageVisibilityChanged() override; - void OnLargestContentfulPaintUpdated( - base::TimeTicks paint_time, - uint64_t paint_size, - base::TimeTicks load_time, - base::TimeTicks first_animated_frame_time, - const AtomicString& id, - const String& url, - Element*); + void OnLargestContentfulPaintUpdated(base::TimeTicks paint_time, + uint64_t paint_size, + base::TimeTicks load_time, + const AtomicString& id, + const String& url, + Element*); void Trace(Visitor*) const override; diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5 index 22779bc4ba6dd7..20f872f1f3a2fc 100644 --- a/third_party/blink/renderer/platform/runtime_enabled_features.json5 +++ b/third_party/blink/renderer/platform/runtime_enabled_features.json5 @@ -1336,10 +1336,6 @@ name: "LazyInitializeMediaControls", // This is enabled by features::kLazyInitializeMediaControls. }, - { - name: "LCPAnimatedImagesWebExposed", - status: "test", - }, { name: "LegacyWindowsDWriteFontFallback", // Enabled by features::kLegacyWindowsDWriteFontFallback; diff --git a/third_party/blink/web_tests/external/wpt/images/anim-tao.png b/third_party/blink/web_tests/external/wpt/images/anim-tao.png deleted file mode 100644 index 925e2efc9a97ade490f04d57271adc10b2fe69b6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 460 zcmeAS@N?(olHy`uVBq!ia0vp^DL`z*!3HE(nbz$CQXGlNAwEEw35Xd!_f9SVQc`IU zF^~{g1Bd|zjLa_>8Ci1NIDx$Bo-U3d6?5L6GvoytbHL!w?6zvIV;iqHq?@gO(Ktbl zoS|)rq)@U)kD_7Q5eZr|_$v>-*5=KZ^ai?#!PC{xWt~$( F698Hck68c! diff --git a/third_party/blink/web_tests/external/wpt/images/anim-tao.png.headers b/third_party/blink/web_tests/external/wpt/images/anim-tao.png.headers deleted file mode 100644 index 0230e176e44567..00000000000000 --- a/third_party/blink/web_tests/external/wpt/images/anim-tao.png.headers +++ /dev/null @@ -1,2 +0,0 @@ -Timing-Allow-Origin: * - diff --git a/third_party/blink/web_tests/external/wpt/images/webp-animated.webp b/third_party/blink/web_tests/external/wpt/images/webp-animated.webp deleted file mode 100644 index 35a8dfcf34d58060ec70f0a2a04e0d58e357e084..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 340 zcmWIYbaV4zWMBw)bqWXzu!!JdU|B|P>F);l9590Z{MS*!B0;1vt zGXn!qpN|tzM4yX+ok3hd%;(wvmN~~P`X2vb?_+b_xHcr*I-p6!<-_fEjddq~hN*SD z*x#_=1f%tZIXZErQ(4dTt19a~HWL-+;f2`%bPy{81JGsx28J&{4%`Md1{Rq&&Br)8)v=KKh9&++1c^WpP^yW)$4OZRe!M0 zE0}Gar~tAd2Iy9xp+E$*fuDf^<_8y$AJ`5sgKbEX!)8O4^vRv~{y8UIh - - - - Largest Contentful Paint: observe image. - - - - - - - - diff --git a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image-webp.tentative.html b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image-webp.tentative.html deleted file mode 100644 index de59d5c5f78c68..00000000000000 --- a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image-webp.tentative.html +++ /dev/null @@ -1,27 +0,0 @@ - - - - - Largest Contentful Paint: observe image. - - - - - - - - diff --git a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image.tentative.html b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image.tentative.html deleted file mode 100644 index cf7d262b0f842a..00000000000000 --- a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-animated-image.tentative.html +++ /dev/null @@ -1,29 +0,0 @@ - - - - - Largest Contentful Paint: observe image. - - - - - - - - diff --git a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-animated-image.tentative.html b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-animated-image.tentative.html deleted file mode 100644 index 993883c607b8f7..00000000000000 --- a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-animated-image.tentative.html +++ /dev/null @@ -1,30 +0,0 @@ - - - - - Largest Contentful Paint: observe image. - - - - - - - - - diff --git a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-tao-animated-image.tentative.html b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-tao-animated-image.tentative.html deleted file mode 100644 index 137dde66383f77..00000000000000 --- a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-cross-origin-tao-animated-image.tentative.html +++ /dev/null @@ -1,30 +0,0 @@ - - - - - Largest Contentful Paint: observe image. - - - - - - - - - diff --git a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-non-animated-image.tentative.html b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-non-animated-image.tentative.html deleted file mode 100644 index 6bbc0958b1deb2..00000000000000 --- a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/animated/observe-non-animated-image.tentative.html +++ /dev/null @@ -1,27 +0,0 @@ - - - - - Largest Contentful Paint: observe image. - - - - - - - - diff --git a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/resources/largest-contentful-paint-helpers.js b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/resources/largest-contentful-paint-helpers.js index 5012faf3b1be33..e12ece0a7561cb 100644 --- a/third_party/blink/web_tests/external/wpt/largest-contentful-paint/resources/largest-contentful-paint-helpers.js +++ b/third_party/blink/web_tests/external/wpt/largest-contentful-paint/resources/largest-contentful-paint-helpers.js @@ -1,6 +1,3 @@ -const image_delay = 1000; -const delay_pipe_value = image_delay / 1000; - // Receives an image LargestContentfulPaint |entry| and checks |entry|'s attribute values. // The |timeLowerBound| parameter is a lower bound on the loadTime value of the entry. // The |options| parameter may contain some string values specifying the following: @@ -36,33 +33,4 @@ function checkImage(entry, expectedUrl, expectedID, expectedSize, timeLowerBound } else { assert_equals(entry.size, expectedSize); } - if (options.includes('animated')) { - assert_greater_than(entry.loadTime, entry.firstAnimatedFrameTime, - 'firstAnimatedFrameTime should be smaller than loadTime'); - assert_greater_than(entry.renderTime, entry.firstAnimatedFrameTime, - 'firstAnimatedFrameTime should be smaller than renderTime'); - assert_less_than(entry.firstAnimatedFrameTime, image_delay, - 'firstAnimatedFrameTime should be smaller than the delay applied to the second frame'); - assert_greater_than(entry.firstAnimatedFrameTime, 0, - 'firstAnimatedFrameTime should be larger than 0'); - } - if (options.includes('animated-zero')) { - assert_equals(entry.firstAnimatedFrameTime, 0, 'firstAnimatedFrameTime should be 0'); - } } - -const load_and_observe = url => { - return new Promise(resolve => { - (new PerformanceObserver(entryList => { - for (let entry of entryList.getEntries()) { - if (entry.url == url) { - resolve(entryList.getEntries()[0]); - } - } - })).observe({type: 'largest-contentful-paint', buffered: true}); - const img = new Image(); - img.id = 'image_id'; - img.src = url; - document.body.appendChild(img); - }); -}; diff --git a/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt b/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt index 51c58da6c2e71c..87a9dce9b39e0e 100644 --- a/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt +++ b/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt @@ -4944,7 +4944,6 @@ interface KeyframeEffect : AnimationEffect interface LargestContentfulPaint : PerformanceEntry attribute @@toStringTag getter element - getter firstAnimatedFrameTime getter id getter loadTime getter renderTime