From 03699d8bbd6a02680b989d4f018a247a7aa98110 Mon Sep 17 00:00:00 2001 From: jahagird Date: Fri, 24 Feb 2017 13:55:07 -0800 Subject: [PATCH] Add LoadEventStart to Android PageLoadMetrics This allows Android layer to report additional metrics for loadEventStart. Test: PageLoadMetricsTest, CustomTabActivityTest Review-Url: https://codereview.chromium.org/2710753002 Cr-Commit-Position: refs/heads/master@{#452941} --- AUTHORS | 1 + .../browser/customtabs/CustomTabActivity.java | 9 ++ .../browser/metrics/PageLoadMetrics.java | 21 ++++ chrome/android/java_sources.gni | 1 + .../customtabs/CustomTabActivityTest.java | 22 +++- .../browser/metrics/PageLoadMetricsTest.java | 117 ++++++++++++++++++ .../android_page_load_metrics_observer.cc | 15 +++ .../android_page_load_metrics_observer.h | 3 + 8 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 chrome/android/javatests/src/org/chromium/chrome/browser/metrics/PageLoadMetricsTest.java diff --git a/AUTHORS b/AUTHORS index ece7b0d4ed1a05..5265a6cc132674 100644 --- a/AUTHORS +++ b/AUTHORS @@ -45,6 +45,7 @@ Alfredo Hernandez Ali Vathi Allan Sandfeld Jensen Ambarish Rapte +Amey Jahagirdar Amit Sarkar Amogh Bihani Amos Lim diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java index cdbb845f42b526..5e572ee8d9f62b 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java @@ -143,6 +143,15 @@ public void onFirstContentfulPaint( mConnection.notifyPageLoadMetric(mSession, PageLoadMetrics.FIRST_CONTENTFUL_PAINT, navigationStartTick, firstContentfulPaintMs); } + + @Override + public void onLoadEventStart( + WebContents webContents, long navigationStartTick, long loadEventStartMs) { + if (webContents != mTab.getWebContents()) return; + + mConnection.notifyPageLoadMetric(mSession, PageLoadMetrics.LOAD_EVENT_START, + navigationStartTick, loadEventStartMs); + } } private static class CustomTabCreator extends ChromeTabCreator { diff --git a/chrome/android/java/src/org/chromium/chrome/browser/metrics/PageLoadMetrics.java b/chrome/android/java/src/org/chromium/chrome/browser/metrics/PageLoadMetrics.java index b7bd23022d0545..b2f89fd4f0dd38 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/metrics/PageLoadMetrics.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/metrics/PageLoadMetrics.java @@ -18,6 +18,7 @@ public class PageLoadMetrics { public static final String FIRST_CONTENTFUL_PAINT = "firstContentfulPaint"; public static final String NAVIGATION_START = "navigationStart"; + public static final String LOAD_EVENT_START = "loadEventStart"; /** Observer for page load metrics. */ public interface Observer { @@ -30,6 +31,16 @@ public interface Observer { */ public void onFirstContentfulPaint( WebContents webContents, long navigationStartTick, long firstContentfulPaintMs); + + /** + * Called when the load event start metric is available. + * + * @param webContents the WebContents this metrics is related to. + * @param navigationStartTick Absolute navigation start time, as TimeTicks. + * @param loadEventStartMs Time to load event start from navigation start. + */ + public void onLoadEventStart( + WebContents webContents, long navigationStartTick, long loadEventStartMs); } private static ObserverList sObservers; @@ -59,5 +70,15 @@ static void onFirstContentfulPaint( } } + @CalledByNative + static void onLoadEventStart( + WebContents webContents, long navigationStartTick, long loadEventStartMs) { + ThreadUtils.assertOnUiThread(); + if (sObservers == null) return; + for (Observer observer : sObservers) { + observer.onLoadEventStart(webContents, navigationStartTick, loadEventStartMs); + } + } + private PageLoadMetrics() {} } diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index 482ef776d73ca7..cab70c91cacfce 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni @@ -1338,6 +1338,7 @@ chrome_test_java_sources = [ "javatests/src/org/chromium/chrome/browser/media/ui/NotificationActionsUpdatedTest.java", "javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java", "javatests/src/org/chromium/chrome/browser/media/ui/PauseOnHeadsetUnplugTest.java", + "javatests/src/org/chromium/chrome/browser/metrics/PageLoadMetricsTest.java", "javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowIntegrationTest.java", "javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowUtilsTest.java", "javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java", diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java index 4c239b6b0461e4..fd2d93a5614103 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java @@ -989,13 +989,14 @@ public boolean isSatisfied() { } /** - * Tests that Time To First Contentful Paint is sent. + * Tests that Time To First Contentful Paint and Load Event Start timings are sent. */ @SmallTest @RetryOnFailure public void testPageLoadMetricIsSent() { final AtomicReference firstContentfulPaintMs = new AtomicReference<>(-1L); final AtomicReference activityStartTimeMs = new AtomicReference<>(-1L); + final AtomicReference loadEventStartMs = new AtomicReference<>(-1L); CustomTabsCallback cb = new CustomTabsCallback() { @Override @@ -1009,9 +1010,16 @@ public void extraCallback(String callbackName, Bundle args) { long firstContentfulPaint = args.getLong(PageLoadMetrics.FIRST_CONTENTFUL_PAINT, -1); - assertTrue(firstContentfulPaint > 0); - assertTrue(firstContentfulPaint <= (current - navigationStart)); - firstContentfulPaintMs.set(firstContentfulPaint); + if (firstContentfulPaint > 0) { + assertTrue(firstContentfulPaint <= (current - navigationStart)); + firstContentfulPaintMs.set(firstContentfulPaint); + } + + long loadEventStart = args.getLong(PageLoadMetrics.LOAD_EVENT_START, -1); + if (loadEventStart > 0) { + assertTrue(loadEventStart <= (current - navigationStart)); + loadEventStartMs.set(loadEventStart); + } } }; @@ -1031,6 +1039,12 @@ public boolean isSatisfied() { return firstContentfulPaintMs.get() > 0; } }); + CriteriaHelper.pollInstrumentationThread(new Criteria() { + @Override + public boolean isSatisfied() { + return loadEventStartMs.get() > 0; + } + }); } catch (InterruptedException e) { fail(); } diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/PageLoadMetricsTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/PageLoadMetricsTest.java new file mode 100644 index 00000000000000..c8ff4deca617a7 --- /dev/null +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/PageLoadMetricsTest.java @@ -0,0 +1,117 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.metrics; + +import android.support.test.filters.SmallTest; + +import org.chromium.base.ThreadUtils; +import org.chromium.base.test.util.RetryOnFailure; +import org.chromium.chrome.browser.ChromeActivity; +import org.chromium.chrome.browser.tab.Tab; +import org.chromium.chrome.test.ChromeActivityTestCaseBase; +import org.chromium.content_public.browser.WebContents; +import org.chromium.net.test.EmbeddedTestServer; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +/** + * Tests for {@link PageLoadMetrics} + */ +@RetryOnFailure +public class PageLoadMetricsTest extends ChromeActivityTestCaseBase { + public PageLoadMetricsTest() { + super(ChromeActivity.class); + } + + private static final int PAGE_LOAD_METRICS_TIMEOUT_MS = 3000; + private static final String TEST_PAGE = "/chrome/test/data/android/google.html"; + private static final String TEST_PAGE_TITLE = "The Google"; + + private String mTestPage; + private EmbeddedTestServer mTestServer; + private PageLoadMetricsObserver mMetricsObserver; + + @Override + protected void setUp() throws Exception { + super.setUp(); + mTestServer = EmbeddedTestServer.createAndStartServer(getInstrumentation().getContext()); + mTestPage = mTestServer.getURL(TEST_PAGE); + + mMetricsObserver = new PageLoadMetricsObserver(getActivity().getActivityTab()); + } + + @Override + protected void tearDown() throws Exception { + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + PageLoadMetrics.removeObserver(mMetricsObserver); + } + }); + + mTestServer.stopAndDestroyServer(); + super.tearDown(); + } + + @Override + public void startMainActivity() throws InterruptedException { + startMainActivityOnBlankPage(); + } + + private static class PageLoadMetricsObserver implements PageLoadMetrics.Observer { + private final Tab mTab; + private final CountDownLatch mFirstContentfulPaintLatch = new CountDownLatch(1); + private final CountDownLatch mLoadEventStartLatch = new CountDownLatch(1); + + public PageLoadMetricsObserver(Tab tab) { + mTab = tab; + } + + @Override + public void onFirstContentfulPaint( + WebContents webContents, long navigationStartTick, long firstContentfulPaintMs) { + if (webContents != mTab.getWebContents()) return; + + if (firstContentfulPaintMs > 0) mFirstContentfulPaintLatch.countDown(); + } + + @Override + public void onLoadEventStart( + WebContents webContents, long navigationStartTick, long loadEventStartMs) { + if (webContents != mTab.getWebContents()) return; + + if (loadEventStartMs > 0) mLoadEventStartLatch.countDown(); + } + + public boolean waitForFirstContentfulPaintEvent() throws InterruptedException { + return mFirstContentfulPaintLatch.await( + PAGE_LOAD_METRICS_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } + + public boolean waitForLoadEventStartEvent() throws InterruptedException { + return mLoadEventStartLatch.await(PAGE_LOAD_METRICS_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } + } + + @SmallTest + public void testPageLoadMetricEmitted() throws InterruptedException { + assertFalse("Tab shouldn't be loading anything before we add observer", + getActivity().getActivityTab().isLoading()); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + PageLoadMetrics.addObserver(mMetricsObserver); + } + }); + + loadUrl(mTestPage); + + assertTrue("First Contentful Paint should be reported", + mMetricsObserver.waitForFirstContentfulPaintEvent()); + assertTrue("Load event start event should be reported", + mMetricsObserver.waitForLoadEventStartEvent()); + } +} diff --git a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc index b1ca8e15c9cc1b..07b0399f5aaeee 100644 --- a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc +++ b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.cc @@ -44,3 +44,18 @@ void AndroidPageLoadMetricsObserver::OnFirstContentfulPaint( (navigation_start_ - base::TimeTicks()).InMicroseconds()), static_cast(first_contentful_paint_ms)); } + +void AndroidPageLoadMetricsObserver::OnLoadEventStart( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + int64_t load_event_start_ms = timing.load_event_start->InMilliseconds(); + base::android::ScopedJavaLocalRef java_web_contents = + web_contents_->GetJavaWebContents(); + JNIEnv* env = base::android::AttachCurrentThread(); + Java_PageLoadMetrics_onLoadEventStart( + env, java_web_contents, + static_cast( + (navigation_start_ - base::TimeTicks()).InMicroseconds()), + static_cast(load_event_start_ms)); +} diff --git a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h index ceee273d4ebc0a..823ae85844d210 100644 --- a/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h +++ b/chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h @@ -29,6 +29,9 @@ class AndroidPageLoadMetricsObserver void OnFirstContentfulPaint( const page_load_metrics::PageLoadTiming& timing, const page_load_metrics::PageLoadExtraInfo& extra_info) override; + void OnLoadEventStart( + const page_load_metrics::PageLoadTiming& timing, + const page_load_metrics::PageLoadExtraInfo& info) override; private: content::WebContents* web_contents_;