Add NavigationCallback.onFirstContentfulPaint variant that matches up to CCT's...

Add NavigationCallback.onFirstContentfulPaint variant that matches up to CCT's version for easier comparison.

Bug: 1152600
Change-Id: I3025464676ecb2cc649e445701cdc5797e4d068d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2559285Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833811}

weblayer/BUILD.gn

@@ -225,6 +225,8 @@ source_set("weblayer_lib_base") {
     "browser/no_state_prefetch/prerender_utils.h",
     "browser/page_load_metrics_initialize.cc",
     "browser/page_load_metrics_initialize.h",
+    "browser/page_load_metrics_observer_impl.cc",
+    "browser/page_load_metrics_observer_impl.h",
     "browser/page_specific_content_settings_delegate.cc",
     "browser/page_specific_content_settings_delegate.h",
     "browser/password_manager_driver_factory.cc",
@@ -269,8 +271,6 @@ source_set("weblayer_lib_base") {
     "browser/translate_client_impl.h",
     "browser/translate_ranker_factory.cc",
     "browser/translate_ranker_factory.h",
-    "browser/ukm_page_load_metrics_observer.cc",
-    "browser/ukm_page_load_metrics_observer.h",
     "browser/url_bar/autocomplete_scheme_classifier_impl.cc",
     "browser/url_bar/autocomplete_scheme_classifier_impl.h",
     "browser/url_bar/page_info_delegate_impl.cc",

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/NavigationTest.java

@@ -15,6 +15,7 @@ import static org.chromium.content_public.browser.test.util.TestThreadUtils.runO
 import android.content.Intent;
 import android.net.Uri;
 import android.os.Bundle;
+import android.os.SystemClock;
 import android.support.test.InstrumentationRegistry;
 import android.webkit.WebResourceResponse;
 
@@ -193,6 +194,25 @@ public class NavigationTest {
             }
         }
 
+        public static class FirstContentfulPaintCallbackHelper extends CallbackHelper {
+            private long mNavigationStartMillis;
+            private long mFirstContentfulPaintMs;
+
+            public void notifyCalled(long navigationStartMillis, long firstContentfulPaintMs) {
+                mNavigationStartMillis = navigationStartMillis;
+                mFirstContentfulPaintMs = firstContentfulPaintMs;
+                notifyCalled();
+            }
+
+            public long getNavigationStartMillis() {
+                return mNavigationStartMillis;
+            }
+
+            public long getFirstContentfulPaintMs() {
+                return mFirstContentfulPaintMs;
+            }
+        }
+
         public NavigationCallbackHelper onStartedCallback = new NavigationCallbackHelper();
         public NavigationCallbackHelper onRedirectedCallback = new NavigationCallbackHelper();
         public NavigationCallbackHelper onReadyToCommitCallback = new NavigationCallbackHelper();
@@ -203,6 +223,8 @@ public class NavigationTest {
         public NavigationCallbackValueRecorder loadProgressChangedCallback =
                 new NavigationCallbackValueRecorder();
         public CallbackHelper onFirstContentfulPaintCallback = new CallbackHelper();
+        public FirstContentfulPaintCallbackHelper onFirstContentfulPaint2Callback =
+                new FirstContentfulPaintCallbackHelper();
         public UriCallbackHelper onOldPageNoLongerRenderedCallback = new UriCallbackHelper();
 
         @Override
@@ -235,6 +257,13 @@ public class NavigationTest {
             onFirstContentfulPaintCallback.notifyCalled();
         }
 
+        @Override
+        public void onFirstContentfulPaint(
+                long navigationStartMillis, long firstContentfulPaintMs) {
+            onFirstContentfulPaint2Callback.notifyCalled(
+                    navigationStartMillis, firstContentfulPaintMs);
+        }
+
         @Override
         public void onOldPageNoLongerRendered(Uri newNavigationUri) {
             onOldPageNoLongerRenderedCallback.notifyCalled(newNavigationUri);
@@ -1209,4 +1238,29 @@ public class NavigationTest {
         mCallback.onFailedCallback.assertCalledWith(
                 curFailedCount, STREAM_URL, LoadError.CONNECTIVITY_ERROR);
     }
+
+    @MinWebLayerVersion(89)
+    @Test
+    @SmallTest
+    public void testOnFirstContentfulPaintTiming() throws Exception {
+        long activityStartTimeMs = SystemClock.uptimeMillis();
+
+        TestWebServer testServer = TestWebServer.start();
+        InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(null);
+        setNavigationCallback(activity);
+        String url = testServer.setResponse("/ok.html", "<html>ok</html>", null);
+
+        int count = mCallback.onFirstContentfulPaint2Callback.getCallCount();
+        mActivityTestRule.navigateAndWait(url);
+        mCallback.onFirstContentfulPaint2Callback.waitForCallback(count);
+
+        long navigationStart = mCallback.onFirstContentfulPaint2Callback.getNavigationStartMillis();
+        long current = SystemClock.uptimeMillis();
+        Assert.assertTrue(navigationStart <= current);
+        Assert.assertTrue(navigationStart >= activityStartTimeMs);
+
+        long firstContentfulPaint =
+                mCallback.onFirstContentfulPaint2Callback.getFirstContentfulPaintMs();
+        Assert.assertTrue(firstContentfulPaint <= (current - navigationStart));
+    }
 }

weblayer/browser/java/org/chromium/weblayer_private/NavigationControllerImpl.java

@@ -5,8 +5,10 @@
 package org.chromium.weblayer_private;
 
 import android.os.RemoteException;
+import android.os.SystemClock;
 import android.webkit.WebResourceResponse;
 
+import org.chromium.base.TimeUtilsJni;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
@@ -28,6 +30,10 @@ public final class NavigationControllerImpl extends INavigationController.Stub {
     private long mNativeNavigationController;
     private INavigationControllerClient mNavigationControllerClient;
 
+    // Conversion between native TimeTicks and SystemClock.uptimeMillis().
+    private long mNativeTickOffsetUs;
+    private boolean mNativeTickOffsetUsComputed;
+
     public NavigationControllerImpl(TabImpl tab, INavigationControllerClient client) {
         mTab = tab;
         mNavigationControllerClient = client;
@@ -220,6 +226,27 @@ public final class NavigationControllerImpl extends INavigationController.Stub {
         mNavigationControllerClient.onFirstContentfulPaint();
     }
 
+    @CalledByNative
+    private void onFirstContentfulPaint2(
+            long navigationStartTick, long firstContentfulPaintDurationMs) throws RemoteException {
+        if (WebLayerFactoryImpl.getClientMajorVersion() < 89) return;
+
+        // See logic in CustomTabsConnection.java that this was based on.
+        if (!mNativeTickOffsetUsComputed) {
+            // Compute offset from time ticks to uptimeMillis.
+            mNativeTickOffsetUsComputed = true;
+            long nativeNowUs = TimeUtilsJni.get().getTimeTicksNowUs();
+            long javaNowUs = SystemClock.uptimeMillis() * 1000;
+            mNativeTickOffsetUs = nativeNowUs - javaNowUs;
+        }
+        // SystemClock.uptimeMillis() is used here as it (as of June 2017) uses the same system call
+        // as all the native side of Chrome, that is clock_gettime(CLOCK_MONOTONIC). Meaning that
+        // the offset relative to navigationStart is to be compared with a
+        // SystemClock.uptimeMillis() value.
+        mNavigationControllerClient.onFirstContentfulPaint2(
+                (navigationStartTick - mNativeTickOffsetUs) / 1000, firstContentfulPaintDurationMs);
+    }
+
     @CalledByNative
     private void onOldPageNoLongerRendered(String uri) throws RemoteException {
         if (WebLayerFactoryImpl.getClientMajorVersion() < 85) return;

weblayer/browser/java/org/chromium/weblayer_private/interfaces/INavigationControllerClient.aidl

@@ -33,4 +33,7 @@ interface INavigationControllerClient {
 
   // Added in M85.
   void onOldPageNoLongerRendered(in String uri) = 9;
+
+  // Added in M89.
+  void onFirstContentfulPaint2(long navigationStartMs, long firstContentfulPaintDurationMs) = 10;
 }

weblayer/browser/navigation_controller_impl.cc

@@ -154,6 +154,23 @@ NavigationImpl* NavigationControllerImpl::GetNavigationImplFromId(
   return nullptr;
 }
 
+void NavigationControllerImpl::OnFirstContentfulPaint(
+    const base::TimeTicks& navigation_start,
+    const base::TimeDelta& first_contentful_paint) {
+#if defined(OS_ANDROID)
+  TRACE_EVENT0("weblayer",
+               "Java_NavigationControllerImpl_onFirstContentfulPaint2");
+  int64_t first_contentful_paint_ms = first_contentful_paint.InMilliseconds();
+  Java_NavigationControllerImpl_onFirstContentfulPaint2(
+      AttachCurrentThread(), java_controller_,
+      (navigation_start - base::TimeTicks()).InMicroseconds(),
+      first_contentful_paint_ms);
+#endif
+
+  for (auto& observer : observers_)
+    observer.OnFirstContentfulPaint(navigation_start, first_contentful_paint);
+}
+
 #if defined(OS_ANDROID)
 void NavigationControllerImpl::SetNavigationControllerImpl(
     JNIEnv* env,

weblayer/browser/navigation_controller_impl.h

@@ -48,6 +48,13 @@ class NavigationControllerImpl : public NavigationController,
   // Returns the NavigationImpl for |navigation_id|, or null if there isn't one.
   NavigationImpl* GetNavigationImplFromId(int64_t navigation_id);
 
+  // Called when the first contentful paint page load metric is available.
+  // |navigation_start| is the navigation start time.
+  // |first_contentful_paint_ms| is the duration to first contentful paint from
+  // navigation start.
+  void OnFirstContentfulPaint(const base::TimeTicks& navigation_start,
+                              const base::TimeDelta& first_contentful_paint);
+
 #if defined(OS_ANDROID)
   void SetNavigationControllerImpl(
       JNIEnv* env,

weblayer/browser/page_load_metrics_initialize.cc

@@ -10,7 +10,7 @@
 #include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 #include "components/page_load_metrics/browser/page_load_tracker.h"
 #include "weblayer/browser/no_state_prefetch/prerender_utils.h"
-#include "weblayer/browser/ukm_page_load_metrics_observer.h"
+#include "weblayer/browser/page_load_metrics_observer_impl.h"
 
 namespace weblayer {
 
@@ -39,10 +39,7 @@ class PageLoadMetricsEmbedder
   // page_load_metrics::PageLoadMetricsEmbedderBase:
   void RegisterEmbedderObservers(
       page_load_metrics::PageLoadTracker* tracker) override {
-    std::unique_ptr<page_load_metrics::PageLoadMetricsObserver> ukm_observer =
-        UkmPageLoadMetricsObserver::CreateIfNeeded();
-    if (ukm_observer)
-      tracker->AddObserver(std::move(ukm_observer));
+    tracker->AddObserver(std::make_unique<PageLoadMetricsObserverImpl>());
 
     if (g_callback_for_testing)
       (*g_callback_for_testing).Run(tracker);

weblayer/browser/ukm_page_load_metrics_observer.cc → weblayer/browser/page_load_metrics_observer_impl.cc

@@ -2,31 +2,31 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "weblayer/browser/ukm_page_load_metrics_observer.h"
+#include "weblayer/browser/page_load_metrics_observer_impl.h"
 
+#include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/no_state_prefetch/browser/prerender_manager.h"
 #include "components/no_state_prefetch/browser/prerender_util.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
+#include "weblayer/browser/navigation_controller_impl.h"
 #include "weblayer/browser/no_state_prefetch/prerender_manager_factory.h"
+#include "weblayer/browser/tab_impl.h"
 
 namespace weblayer {
 
-// static
-std::unique_ptr<page_load_metrics::PageLoadMetricsObserver>
-UkmPageLoadMetricsObserver::CreateIfNeeded() {
-  if (!ukm::UkmRecorder::Get()) {
-    return nullptr;
-  }
-  return std::make_unique<UkmPageLoadMetricsObserver>();
-}
-
-UkmPageLoadMetricsObserver::ObservePolicy UkmPageLoadMetricsObserver::OnCommit(
+PageLoadMetricsObserverImpl::ObservePolicy
+PageLoadMetricsObserverImpl::OnCommit(
     content::NavigationHandle* navigation_handle,
     ukm::SourceId source_id) {
 #if defined(OS_ANDROID)
+  if (!ukm::UkmRecorder::Get())
+    return CONTINUE_OBSERVING;
+
+  // If URL-Keyed-Metrics (UKM) is enabled in the system, this is used to
+  // populate it with top-level page-load metrics.
   prerender::PrerenderManager* const prerender_manager =
       PrerenderManagerFactory::GetForBrowserContext(
           navigation_handle->GetWebContents()->GetBrowserContext());
@@ -38,4 +38,17 @@ UkmPageLoadMetricsObserver::ObservePolicy UkmPageLoadMetricsObserver::OnCommit(
   return CONTINUE_OBSERVING;
 }
 
+void PageLoadMetricsObserverImpl::OnFirstContentfulPaintInPage(
+    const page_load_metrics::mojom::PageLoadTiming& timing) {
+  auto* tab = TabImpl::FromWebContents(GetDelegate().GetWebContents());
+  if (!tab)
+    return;
+
+  auto* nav_controller =
+      static_cast<NavigationControllerImpl*>(tab->GetNavigationController());
+  nav_controller->OnFirstContentfulPaint(
+      GetDelegate().GetNavigationStart(),
+      *timing.paint_timing->first_contentful_paint);
+}
+
 }  // namespace weblayer

weblayer/browser/ukm_page_load_metrics_observer.h → weblayer/browser/page_load_metrics_observer_impl.h

@@ -2,34 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef WEBLAYER_BROWSER_UKM_PAGE_LOAD_METRICS_OBSERVER_H_
-#define WEBLAYER_BROWSER_UKM_PAGE_LOAD_METRICS_OBSERVER_H_
+#ifndef WEBLAYER_BROWSER_PAGE_LOAD_METRICS_OBSERVER_IMPL_H_
+#define WEBLAYER_BROWSER_PAGE_LOAD_METRICS_OBSERVER_IMPL_H_
 
 #include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 
-namespace content {
-class NavigationHandle;
-}
-
 namespace weblayer {
 
-// If URL-Keyed-Metrics (UKM) is enabled in the system, this is used to
-// populate it with top-level page-load metrics.
-class UkmPageLoadMetricsObserver
+class PageLoadMetricsObserverImpl
     : public page_load_metrics::PageLoadMetricsObserver {
  public:
-  // Returns a UkmPageLoadMetricsObserver, or nullptr if it is not needed.
-  static std::unique_ptr<page_load_metrics::PageLoadMetricsObserver>
-  CreateIfNeeded();
-
-  UkmPageLoadMetricsObserver() = default;
-  ~UkmPageLoadMetricsObserver() override = default;
+  PageLoadMetricsObserverImpl() = default;
+  ~PageLoadMetricsObserverImpl() override = default;
 
   // page_load_metrics::PageLoadMetricsObserver implementation:
   ObservePolicy OnCommit(content::NavigationHandle* navigation_handle,
                          ukm::SourceId source_id) override;
+  void OnFirstContentfulPaintInPage(
+      const page_load_metrics::mojom::PageLoadTiming& timing) override;
 };
 
 }  // namespace weblayer
 
-#endif  // WEBLAYER_BROWSER_UKM_PAGE_LOAD_METRICS_OBSERVER_H_
+#endif  // WEBLAYER_BROWSER_PAGE_LOAD_METRICS_OBSERVER_IMPL_H_

weblayer/public/java/org/chromium/weblayer/NavigationCallback.java

@@ -115,6 +115,19 @@ public abstract class NavigationCallback {
      */
     public void onFirstContentfulPaint() {}
 
+    /**
+     * Similar to onFirstContentfulPaint but contains timing information from the renderer process
+     * to better align with the Navigation Timing API.
+     *
+     * @param navigationStartMs the absolute navigation start time in milliseconds since boot,
+     *        not counting time spent in deep sleep. This comes from SystemClock.uptimeMillis().
+     * @param firstContentfulPaintDurationMs the number of milliseconds to first contentful paint
+     *        from navigation start.
+     * @since 89
+     */
+    public void onFirstContentfulPaint(
+            long navigationStartMs, long firstContentfulPaintDurationMs) {}
+
     /**
      * Called after each navigation to indicate that the old page is no longer
      * being rendered. Note this is not ordered with respect to onFirstContentfulPaint.

weblayer/public/java/org/chromium/weblayer/NavigationController.java

@@ -378,6 +378,15 @@ public class NavigationController {
             }
         }
 
+        @Override
+        public void onFirstContentfulPaint2(
+                long navigationStartMs, long firstContentfulPaintDurationMs) {
+            StrictModeWorkaround.apply();
+            for (NavigationCallback callback : mCallbacks) {
+                callback.onFirstContentfulPaint(navigationStartMs, firstContentfulPaintDurationMs);
+            }
+        }
+
         @Override
         public void onOldPageNoLongerRendered(String uri) {
             StrictModeWorkaround.apply();

weblayer/public/navigation_observer.h

@@ -7,6 +7,11 @@
 
 class GURL;
 
+namespace base {
+class TimeDelta;
+class TimeTicks;
+}  // namespace base
+
 namespace weblayer {
 class Navigation;
 
@@ -98,6 +103,15 @@ class NavigationObserver {
   // first paint after a non-empty layout has finished.
   virtual void OnFirstContentfulPaint() {}
 
+  // Similar to OnFirstContentfulPaint but contains timing information from the
+  // renderer process to better align with the Navigation Timing API.
+  // |navigation_start| is the navigation start time.
+  // |first_contentful_paint| is the duration to first contentful paint from
+  // navigation start.
+  virtual void OnFirstContentfulPaint(
+      const base::TimeTicks& navigation_start,
+      const base::TimeDelta& first_contentful_paint) {}
+
   // Called after each navigation to indicate that the old page is no longer
   // being rendered. Note this is not ordered with respect to
   // OnFirstContentfulPaint.