Skip to content

Commit

Permalink
Reland #2 "[LCP] Add animated image support"
Browse files Browse the repository at this point in the history
This is a manual reland of
https://chromium-review.googlesource.com/c/chromium/src/+/3247449

The difference from the previous reland is that the browser tests now
include 2 separate timeouts and a double rAF, to ensure that the
presentation timestamp taken is far enough from both the time the first
frame is sent as well as from the time the second frame is sent.
More importantly, the test now actually is looking at the UKM metric,
rather than at the histogram.

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:
> w3c/largest-contentful-paint#83

Bug: 1260953
Change-Id: I34070bd90a74ed44281da63b547f13d9669f389b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3250690
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936516}
  • Loading branch information
Yoav Weiss authored and Chromium LUCI CQ committed Oct 29, 2021
1 parent 60a6238 commit 0955a5d
Show file tree
Hide file tree
Showing 26 changed files with 533 additions and 57 deletions.
181 changes: 181 additions & 0 deletions chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#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"
Expand Down Expand Up @@ -76,6 +77,8 @@
#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"
Expand Down Expand Up @@ -173,6 +176,18 @@ class PageLoadMetricsBrowserTest : public InProcessBrowserTest {
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL)));
}

int64_t GetUKMPageLoadMetric(std::string metric_name) {
std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> merged_entries =
test_ukm_recorder_->GetMergedEntriesByName(PageLoad::kEntryName);

EXPECT_EQ(1ul, merged_entries.size());
const auto& kv = merged_entries.begin();
const int64_t* recorded =
ukm::TestUkmRecorder::GetEntryMetric(kv->second.get(), metric_name);
EXPECT_TRUE(recorded != nullptr);
return (*recorded);
}

void MakeComponentFullscreen(const std::string& id) {
EXPECT_TRUE(content::ExecuteScript(
browser()->tab_strip_model()->GetActiveWebContents(),
Expand Down Expand Up @@ -376,6 +391,151 @@ class PageLoadMetricsBrowserTest : public InProcessBrowserTest {
std::unique_ptr<ukm::TestAutoSetUkmRecorder> 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<net::test_server::ControllableHttpResponse>(
embedded_test_server(), "/mock_page.html",
false /*relative_url_is_prefix*/);
auto img_response =
std::make_unique<net::test_server::ControllableHttpResponse>(
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 ? "<html><body></body><img "
"src=\"/images/animated-delayed.png\"></script></html>"
: "<html><body></body><img "
"src=\"/images/delayed.jpg\"></script></html>");
main_html_response->Done();

img_response->WaitForRequest();
img_response->Send(kImgHttpResponseHeader);
img_response->Send(first_frame);

// Trigger a double rAF and wait a bit, then take a timestamp that's after
// the presentation time of the first frame.
// Then wait some more to ensure the timestamp is not too close to the point
// where the second frame is sent.
content::EvalJsResult result =
EvalJs(browser()->tab_strip_model()->GetActiveWebContents(), R"(
(async () => {
const double_raf = () => {
return new Promise(r => {
requestAnimationFrame(()=>requestAnimationFrame(r));
})
};
await double_raf();
await new Promise(r => setTimeout(r, 50));
const timestamp = performance.now();
await new Promise(r => setTimeout(r, 50));
return timestamp;
})();)");
EXPECT_EQ("", result.error);
double timestamp = result.ExtractDouble();

img_response->Send(second_frame);
img_response->Done();

// Wait on an LCP entry to make sure we have one to report when navigating
// away.
content::EvalJsResult result2 =
EvalJs(browser()->tab_strip_model()->GetActiveWebContents(), R"(
(async () => {
await new Promise(resolve => {
(new PerformanceObserver(list => {
const entries = list.getEntries();
for (let entry of entries) {
if (entry.url.includes('images')) {resolve()}
}
}))
.observe({type: 'largest-contentful-paint', buffered: true});
})})())");
EXPECT_EQ("", result2.error);
waiter->Wait();

// LCP is collected only at the end of the page lifecycle. Navigate to
// flush.
NavigateToUntrackedUrl();

int64_t value = GetUKMPageLoadMetric(
PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name);

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())
Expand Down Expand Up @@ -2989,6 +3149,27 @@ 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());

Expand Down
Binary file added content/test/data/animated.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,12 @@ ResourceStatus ImageResourceContent::GetContentStatus() const {
return content_status_;
}

// TODO(hiroshige): Consider removing the following methods, or stoping
bool ImageResourceContent::IsAnimatedImageWithPaintedFirstFrame() const {
return (image_ && !image_->IsNull() && image_->MaybeAnimated() &&
image_->CurrentFrameIsComplete());
}

// TODO(hiroshige): Consider removing the following methods, or stopping
// redirecting to ImageResource.
const KURL& ImageResourceContent::Url() const {
return info_->Url();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ 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;
Expand Down
Loading

0 comments on commit 0955a5d

Please sign in to comment.