Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(83)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc

Issue 2847513002: Break page load metrics test dependency on IPC. (Closed)
Patch Set: add expectation in wait method Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
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 8d69c71e8ddf51c0cebb4c566fda09100271e219..2e54df31d741d14370389f4d6555a932de1fbe4d 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
+++ b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
@@ -38,9 +38,9 @@
namespace {
-// Waits until a PageLoadMetricsMsg_TimingUpdated message IPC is received
-// matching a PageLoadTiming. See WaitForMatchingIPC for details.
-class TimingUpdatedObserver : public content::BrowserMessageFilter {
+// Waits until specified timing and metadata expectations are satisfied.
+class PageLoadMetricsWaiter
+ : public page_load_metrics::MetricsWebContentsObserver::TestingObserver {
public:
// A bitvector to express which timing fields to match on.
enum ExpectedTimingFields {
@@ -49,57 +49,40 @@ class TimingUpdatedObserver : public content::BrowserMessageFilter {
STYLE_UPDATE_BEFORE_FCP = 1 << 2
};
- explicit TimingUpdatedObserver(content::RenderWidgetHost* render_widget_host)
- : content::BrowserMessageFilter(PageLoadMetricsMsgStart) {
- render_widget_host->GetProcess()->AddFilter(this);
+ explicit PageLoadMetricsWaiter(content::WebContents* web_contents)
+ : TestingObserver(web_contents) {}
- // Roundtrip to the IO thread, to ensure that the filter is properly
- // installed.
- content::BrowserThread::PostTaskAndReply(
- content::BrowserThread::IO, FROM_HERE, base::BindOnce(&base::DoNothing),
- base::BindOnce(&TimingUpdatedObserver::Quit, this));
- run_loop_.reset(new base::RunLoop());
- run_loop_->Run();
- run_loop_.reset(nullptr);
- }
+ ~PageLoadMetricsWaiter() override { DCHECK_EQ(nullptr, run_loop_.get()); }
- // Add the given timing fields to the set of fields to match on.
- void AddMatchingFields(ExpectedTimingFields fields) {
+ // Add the given expectation to match on.
+ void AddExpectation(ExpectedTimingFields fields) {
matching_fields_ |= fields;
}
// Instructs observer to also watch for |count|
// WebLoadingBehaviorDocumentWriteBlockReload events.
- void MatchDocumentWriteBlockReload(int count) {
+ void ExpectDocumentWriteBlockReload(int count) {
match_document_write_block_reload_ = count;
}
// Waits for a TimingUpdated IPC that matches the fields set by
- // |AddMatchingFields|. All matching fields must be set in a TimingUpdated
+ // |AddExpectation|. All matching fields must be set in a TimingUpdated
// IPC for it to end this wait.
- void WaitForMatchingIPC() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- if (matched_timing_update_)
+ void Wait() {
+ if (expectations_satisfied_)
return;
run_loop_.reset(new base::RunLoop());
run_loop_->Run();
run_loop_.reset(nullptr);
- }
- private:
- bool OnMessageReceived(const IPC::Message& message) override {
- DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-
- IPC_BEGIN_MESSAGE_MAP(TimingUpdatedObserver, message)
- IPC_MESSAGE_HANDLER(PageLoadMetricsMsg_TimingUpdated, OnTimingUpdated)
- IPC_END_MESSAGE_MAP()
-
- return false;
+ EXPECT_TRUE(expectations_satisfied_);
}
- bool OnTimingUpdated(const page_load_metrics::PageLoadTiming& timing,
- const page_load_metrics::PageLoadMetadata& metadata) {
+ private:
+ void OnTimingUpdated(
+ const page_load_metrics::PageLoadTiming& timing,
+ const page_load_metrics::PageLoadMetadata& metadata) override {
if (match_document_write_block_reload_ > 0 &&
metadata.behavior_flags &
blink::WebLoadingBehaviorFlag::
@@ -108,7 +91,7 @@ class TimingUpdatedObserver : public content::BrowserMessageFilter {
}
if (match_document_write_block_reload_ > 0) {
- return true;
+ return;
}
if ((!(matching_fields_ & FIRST_PAINT) ||
@@ -117,41 +100,15 @@ class TimingUpdatedObserver : public content::BrowserMessageFilter {
timing.paint_timing.first_contentful_paint) &&
(!(matching_fields_ & STYLE_UPDATE_BEFORE_FCP) ||
timing.style_sheet_timing.update_style_duration_before_fcp)) {
- // Ensure that any other handlers of this message, for example the real
- // PageLoadMetric observers, get a chance to handle this message before
- // this waiter unblocks.
- content::BrowserThread::PostTask(
- content::BrowserThread::IO, FROM_HERE,
- base::BindOnce(&TimingUpdatedObserver::BounceTimingUpdate, this,
- timing, metadata));
+ expectations_satisfied_ = true;
+ if (run_loop_)
+ run_loop_->Quit();
}
- return true;
- }
-
- void BounceTimingUpdate(const page_load_metrics::PageLoadTiming& timing,
- const page_load_metrics::PageLoadMetadata& metadata) {
- content::BrowserThread::PostTask(
- content::BrowserThread::UI, FROM_HERE,
- base::BindOnce(&TimingUpdatedObserver::SetTimingUpdatedAndQuit, this));
- }
-
- void Quit() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- if (run_loop_)
- run_loop_->Quit();
- }
-
- void SetTimingUpdatedAndQuit() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- matched_timing_update_ = true;
- Quit();
}
- ~TimingUpdatedObserver() override {}
-
std::unique_ptr<base::RunLoop> run_loop_;
int matching_fields_ = 0; // A bitvector composed from ExpectedTimingFields.
- bool matched_timing_update_ = false;
+ bool expectations_satisfied_ = false;
int match_document_write_block_reload_ = 0;
};
@@ -178,12 +135,10 @@ class PageLoadMetricsBrowserTest : public InProcessBrowserTest {
return total_pageload_histograms - total_internal_histograms == 0;
}
- scoped_refptr<TimingUpdatedObserver> CreateTimingUpdatedObserver() {
+ std::unique_ptr<PageLoadMetricsWaiter> CreatePageLoadMetricsWaiter() {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
- scoped_refptr<TimingUpdatedObserver> observer(new TimingUpdatedObserver(
- web_contents->GetRenderViewHost()->GetWidget()));
- return observer;
+ return base::MakeUnique<PageLoadMetricsWaiter>(web_contents);
}
base::HistogramTester histogram_tester_;
@@ -339,15 +294,14 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, IgnoreDownloads) {
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PreloadDocumentWrite) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL(
"/page_load_metrics/document_write_external_script.html"));
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
histogram_tester_.ExpectTotalCount(
internal::kHistogramDocWriteParseStartToFirstContentfulPaint, 1);
@@ -368,14 +322,13 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoPreloadDocumentWrite) {
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoDocumentWrite) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
ui_test_utils::NavigateToURL(browser(),
embedded_test_server()->GetURL("/title1.html"));
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
histogram_tester_.ExpectTotalCount(
internal::kHistogramDocWriteParseStartToFirstContentfulPaint, 0);
@@ -387,15 +340,14 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoDocumentWrite) {
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteBlock) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL(
"/page_load_metrics/document_write_script_block.html"));
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
histogram_tester_.ExpectTotalCount(
internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
@@ -405,13 +357,12 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteBlock) {
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteReload) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
- scoped_refptr<TimingUpdatedObserver> reload_observer =
- CreateTimingUpdatedObserver();
- reload_observer->MatchDocumentWriteBlockReload(2);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> reload_waiter =
+ CreatePageLoadMetricsWaiter();
+ reload_waiter->ExpectDocumentWriteBlockReload(2);
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL(
@@ -429,8 +380,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteReload) {
histogram_tester_.ExpectTotalCount(
internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
- fcp_observer->WaitForMatchingIPC();
- reload_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
+ reload_waiter->Wait();
histogram_tester_.ExpectTotalCount(
internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
@@ -488,9 +439,9 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoDocumentWriteScript) {
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, MAYBE_BadXhtml) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> timing_observer =
- CreateTimingUpdatedObserver();
- timing_observer->AddMatchingFields(TimingUpdatedObserver::FIRST_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+ CreatePageLoadMetricsWaiter();
+ timing_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
// When an XHTML page contains invalid XML, it causes a paint of the error
// message without a layout. Page load metrics currently treats this as an
@@ -501,7 +452,7 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, MAYBE_BadXhtml) {
browser(),
embedded_test_server()->GetURL("/page_load_metrics/badxml.xhtml"));
- timing_observer->WaitForMatchingIPC();
+ timing_waiter->Wait();
histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 0);
histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 0);
@@ -688,15 +639,14 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
FirstMeaningfulPaintNotRecorded) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL(
"/page_load_metrics/page_with_active_connections.html"));
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
// Navigate away before a FMP is reported.
NavigateToUntrackedUrl();
@@ -714,15 +664,14 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
NoStatePrefetchObserverCacheable) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
ui_test_utils::NavigateToURL(browser(),
embedded_test_server()->GetURL("/title1.html"));
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
histogram_tester_.ExpectTotalCount(
"Prerender.none_PrefetchTTFCP.Reference.NoStore.Visible", 0);
@@ -734,15 +683,14 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
NoStatePrefetchObserverNoStore) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::FIRST_CONTENTFUL_PAINT);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
ui_test_utils::NavigateToURL(browser(),
embedded_test_server()->GetURL("/nostore.html"));
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
histogram_tester_.ExpectTotalCount(
"Prerender.none_PrefetchTTFCP.Reference.NoStore.Visible", 1);
@@ -753,10 +701,9 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, CSSTiming) {
ASSERT_TRUE(embedded_test_server()->Start());
- scoped_refptr<TimingUpdatedObserver> fcp_observer =
- CreateTimingUpdatedObserver();
- fcp_observer->AddMatchingFields(
- TimingUpdatedObserver::STYLE_UPDATE_BEFORE_FCP);
+ std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
+ CreatePageLoadMetricsWaiter();
+ fcp_waiter->AddExpectation(PageLoadMetricsWaiter::STYLE_UPDATE_BEFORE_FCP);
// Careful: Blink code clamps timestamps to 5us, so any CSS parsing we do here
// must take >> 5us, otherwise we'll log 0 for the value and it will remain
@@ -765,7 +712,7 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, CSSTiming) {
browser(),
embedded_test_server()->GetURL("/page_load_metrics/page_with_css.html"));
NavigateToUntrackedUrl();
- fcp_observer->WaitForMatchingIPC();
+ fcp_waiter->Wait();
histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
1);
« no previous file with comments | « chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698