Adding DRP specfic UMA for FirstContentfulPaint

content/browser/frame_host/navigator_impl_unittest.cc

Index: content/browser/frame_host/navigator_impl_unittest.cc
diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc
index 2da67a0c839aa0b5d1a518849f86e223ddb44b6b..7bcbb287588bdd0a3ccf026f19c31990cea58eb1 100644
--- a/content/browser/frame_host/navigator_impl_unittest.cc
+++ b/content/browser/frame_host/navigator_impl_unittest.cc
@@ -1,31 +1,33 @@
 // Copyright 2014 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.
 
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_manager.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/streams/stream.h"
 #include "content/common/frame_messages.h"
 #include "content/common/navigation_params.h"
 #include "content/common/site_isolation_policy.h"
+#include "content/public/browser/navigation_data.h"
 #include "content/public/browser/stream_handle.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_utils.h"
 #include "content/test/browser_side_navigation_test_utils.h"
 #include "content/test/test_navigation_url_loader.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_web_contents.h"
 #include "net/base/load_flags.h"
@@ -110,20 +112,29 @@ class NavigatorTestWithBrowserSideNavigation
   }
 
   scoped_refptr<SiteInstance> ConvertToSiteInstance(
       RenderFrameHostManager* rfhm,
       const SiteInstanceDescriptor& descriptor,
       SiteInstance* candidate_instance) {
     return rfhm->ConvertToSiteInstance(descriptor, candidate_instance);
   }
 };
 
+class TestNavigationData : public NavigationData {

[nasko, 2016/05/02 22:02:49]

Defining the same class over multiple unit tests files calls for having it
extracted out in its own file.

[RyanSturm, 2016/05/02 23:02:56]

Removing this from the file.

+ public:
+  TestNavigationData() {}
+  ~TestNavigationData() override {}
+  std::unique_ptr<NavigationData> Clone() const override {
+    return base::WrapUnique(new TestNavigationData());
+  }
+};
+
 // PlzNavigate: Test a complete browser-initiated navigation starting with a
 // non-live renderer.
 TEST_F(NavigatorTestWithBrowserSideNavigation,
        SimpleBrowserInitiatedNavigationFromNonLiveRenderer) {
   const GURL kUrl("http://chromium.org/");
 
   EXPECT_FALSE(main_test_rfh()->IsRenderFrameLive());
 
   // Start a browser-initiated navigation.
   int32_t site_instance_id = main_test_rfh()->GetSiteInstance()->GetId();
@@ -136,22 +147,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
 
   // As there's no live renderer the navigation should not wait for a
   // beforeUnload ACK from the renderer and start right away.
   EXPECT_EQ(NavigationRequest::STARTED, request->state());
   ASSERT_TRUE(GetLoaderForNavigationRequest(request));
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
 
   // Have the current RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));

[nasko, 2016/05/02 22:02:49]

So this change requires always passing NavigationData? That doesn't seem right.
What if an embedder doesn't supply this? Ideally the code will be structured
such that it can take the new data when needed but operate fine without it.
Changing all tests here seems unneeded churn and more overhead for new tests
being written.

[RyanSturm, 2016/05/02 23:02:56]

I can pass in an null NavigationData; it's a good point that a test one really
isn't needed here. the last one in the file does this, but I will just do it for
all of them.

   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   EXPECT_TRUE(main_test_rfh()->is_loading());
   EXPECT_FALSE(node->navigation_request());
 
   // Commit the navigation.
   main_test_rfh()->SendNavigate(0, entry_id, true, kUrl);
   EXPECT_TRUE(main_test_rfh()->is_active());
   EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl),
             main_test_rfh()->GetSiteInstance()->GetSiteURL());
   EXPECT_EQ(kUrl, contents()->GetLastCommittedURL());
@@ -189,22 +200,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   // The navigation is immediately started as there's no need to wait for
   // beforeUnload to be executed.
   EXPECT_EQ(NavigationRequest::STARTED, request->state());
   EXPECT_FALSE(request->begin_params().has_user_gesture);
   EXPECT_EQ(kUrl2, request->common_params().url);
   EXPECT_FALSE(request->browser_initiated());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Have the current RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   EXPECT_TRUE(main_test_rfh()->is_loading());
   EXPECT_FALSE(node->navigation_request());
 
   // Commit the navigation.
   main_test_rfh()->SendNavigate(1, 0, true, kUrl2);
   EXPECT_TRUE(main_test_rfh()->is_active());
   EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl2),
             main_test_rfh()->GetSiteInstance()->GetSiteURL());
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
@@ -238,22 +249,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_EQ(kUrl2, request->common_params().url);
   EXPECT_FALSE(request->browser_initiated());
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   }
 
   // Have the current RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(
         DidRenderFrameHostRequestCommit(GetSpeculativeRenderFrameHost(node)));
   } else {
     EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   }
   EXPECT_TRUE(main_test_rfh()->is_loading());
   EXPECT_FALSE(node->navigation_request());
 
   // Commit the navigation.
@@ -391,22 +402,23 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, NoContent) {
   ASSERT_TRUE(main_request);
 
   // Navigations to a different site do create a speculative RenderFrameHost.
   EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
 
   // Commit an HTTP 204 response.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   const char kNoContentHeaders[] = "HTTP/1.1 204 No Content\0\0";
   response->head.headers = new net::HttpResponseHeaders(
       std::string(kNoContentHeaders, arraysize(kNoContentHeaders)));
-  GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
-      response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnResponseStarted(response, MakeEmptyStream(),
+                              base::WrapUnique(new TestNavigationData()));
 
   // There should be no pending nor speculative RenderFrameHost; the navigation
   // was aborted.
   EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   EXPECT_FALSE(node->navigation_request());
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Now, repeat the test with 205 Reset Content.
 
@@ -417,22 +429,23 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, NoContent) {
 
   main_request = node->navigation_request();
   ASSERT_TRUE(main_request);
   EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
 
   // Commit an HTTP 205 response.
   response = new ResourceResponse;
   const char kResetContentHeaders[] = "HTTP/1.1 205 Reset Content\0\0";
   response->head.headers = new net::HttpResponseHeaders(
       std::string(kResetContentHeaders, arraysize(kResetContentHeaders)));
-  GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
-      response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnResponseStarted(response, MakeEmptyStream(),
+                              base::WrapUnique(new TestNavigationData()));
 
   // There should be no pending nor speculative RenderFrameHost; the navigation
   // was aborted.
   EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   EXPECT_FALSE(node->navigation_request());
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 }
 
 // PlzNavigate: Test that a new RenderFrameHost is created when doing a cross
@@ -451,22 +464,23 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, CrossSiteNavigation) {
   NavigationRequest* main_request = node->navigation_request();
   ASSERT_TRUE(main_request);
   TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
   ASSERT_TRUE(speculative_rfh);
 
   // Receive the beforeUnload ACK.
   main_test_rfh()->SendBeforeUnloadACK(true);
   EXPECT_EQ(speculative_rfh, GetSpeculativeRenderFrameHost(node));
 
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
-      response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnResponseStarted(response, MakeEmptyStream(),
+                              base::WrapUnique(new TestNavigationData()));
   EXPECT_EQ(speculative_rfh, GetSpeculativeRenderFrameHost(node));
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));
   EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));
 
   speculative_rfh->SendNavigate(0, entry_id, true, kUrl2);
 
   RenderFrameHostImpl* final_rfh = main_test_rfh();
   EXPECT_EQ(speculative_rfh, final_rfh);
   EXPECT_NE(initial_rfh, final_rfh);
   EXPECT_TRUE(final_rfh->IsRenderFrameLive());
@@ -494,22 +508,23 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, RedirectCrossSite) {
 
   // It then redirects to another site.
   GetLoaderForNavigationRequest(main_request)->SimulateServerRedirect(kUrl2);
 
   // The redirect should have been followed.
   EXPECT_EQ(1, GetLoaderForNavigationRequest(main_request)->redirect_count());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
-      response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnResponseStarted(response, MakeEmptyStream(),
+                              base::WrapUnique(new TestNavigationData()));
   TestRenderFrameHost* final_speculative_rfh =
       GetSpeculativeRenderFrameHost(node);
   EXPECT_TRUE(final_speculative_rfh);
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(final_speculative_rfh));
 
   // Commit the navigation.
   final_speculative_rfh->SendNavigate(0, entry_id, true, kUrl2);
   RenderFrameHostImpl* final_rfh = main_test_rfh();
   ASSERT_TRUE(final_rfh);
   EXPECT_NE(rfh, final_rfh);
@@ -566,21 +581,21 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
 
   // Confirm that a new speculative RenderFrameHost was created.
   speculative_rfh = GetSpeculativeRenderFrameHost(node);
   ASSERT_TRUE(speculative_rfh);
   int32_t site_instance_id_2 = speculative_rfh->GetSiteInstance()->GetId();
   EXPECT_NE(site_instance_id_1, site_instance_id_2);
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
-      response, MakeEmptyStream());
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));
   EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));
 
   // Commit the navigation.
   speculative_rfh->SendNavigate(0, entry_id, true, kUrl2);
 
   // Confirm that the commit corresponds to the new request.
   ASSERT_TRUE(main_test_rfh());
   EXPECT_EQ(kUrl2_site, main_test_rfh()->GetSiteInstance()->GetSiteURL());
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
@@ -632,22 +647,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   // Confirm that the speculative RenderFrameHost was destroyed in the non
   // SitePerProcess case.
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   }
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request2)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(
         DidRenderFrameHostRequestCommit(GetSpeculativeRenderFrameHost(node)));
   } else {
     EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   }
 
   // Commit the navigation.
   main_test_rfh()->SendNavigate(1, 0, true, kUrl2);
 
@@ -693,22 +708,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_FALSE(request2->browser_initiated());
   EXPECT_TRUE(request2->begin_params().has_user_gesture);
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   }
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request2)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(
         DidRenderFrameHostRequestCommit(GetSpeculativeRenderFrameHost(node)));
   } else {
     EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   }
 
   // Commit the navigation.
   main_test_rfh()->SendNavigate(1, 0, true, kUrl1);
   EXPECT_EQ(kUrl1, contents()->GetLastCommittedURL());
@@ -745,22 +760,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_TRUE(request2->browser_initiated());
   EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
 
   // Now receive the beforeUnload ACK from the still ongoing navigation.
   main_test_rfh()->SendBeforeUnloadACK(true);
   TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
   ASSERT_TRUE(speculative_rfh);
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request2)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));
   EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));
 
   // Commit the navigation.
   speculative_rfh->SendNavigate(0, entry_id, true, kUrl1);
   EXPECT_EQ(kUrl1, contents()->GetLastCommittedURL());
 }
 
 // PlzNavigate: Test that a renderer-initiated non-user-initiated navigation is
 // canceled if a another similar request is issued in the meantime.
@@ -802,22 +817,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   }
 
   // Confirm that the first loader got destroyed.
   EXPECT_FALSE(loader1);
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  GetLoaderForNavigationRequest(request2)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+  GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
+      response, MakeEmptyStream(), base::WrapUnique(new TestNavigationData()));
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     EXPECT_TRUE(
         DidRenderFrameHostRequestCommit(GetSpeculativeRenderFrameHost(node)));
   } else {
     EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   }
 
   // Commit the navigation.
   main_test_rfh()->SendNavigate(1, 0, true, kUrl2);
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
@@ -880,21 +895,22 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_EQ(speculative_rfh, GetSpeculativeRenderFrameHost(node));
   EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl),
             speculative_rfh->GetSiteInstance()->GetSiteURL());
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
   int32_t site_instance_id = speculative_rfh->GetSiteInstance()->GetId();
 
   // Ask Navigator to commit the navigation by simulating a call to
   // OnResponseStarted.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(node->navigation_request())
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+      ->CallOnResponseStarted(response, MakeEmptyStream(),
+                              base::WrapUnique(new TestNavigationData()));
   EXPECT_EQ(speculative_rfh, GetSpeculativeRenderFrameHost(node));
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));
   EXPECT_EQ(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
 
   // Invoke OnDidCommitProvisionalLoad.
   speculative_rfh->SendNavigate(0, entry_id, true, kUrl);
   EXPECT_EQ(site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
@@ -940,22 +956,24 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   // the redirect.
   // TODO(carlosk): once the speculative RenderFrameHost updates with redirects
   // this next check will be changed to verify that it actually happens.
   EXPECT_EQ(speculative_rfh, GetSpeculativeRenderFrameHost(node));
   EXPECT_EQ(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());
   EXPECT_FALSE(rfh_deleted_observer.deleted());
 
   // Commit the navigation with Navigator by simulating the call to
   // OnResponseStarted.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  std::unique_ptr<NavigationData> empty_navigation_data;
   GetLoaderForNavigationRequest(main_request)
-      ->CallOnResponseStarted(response, MakeEmptyStream());
+      ->CallOnResponseStarted(response, MakeEmptyStream(),
+                              std::move(empty_navigation_data));
   speculative_rfh = GetSpeculativeRenderFrameHost(node);
   ASSERT_TRUE(speculative_rfh);
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));
   EXPECT_EQ(init_site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
   EXPECT_TRUE(rfh_deleted_observer.deleted());
 
   // Once commit happens the speculative RenderFrameHost is updated to match the
   // known final SiteInstance.
   EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrlRedirect),
             speculative_rfh->GetSiteInstance()->GetSiteURL());