Prerender: Remove PerceivedPLT histograms

chrome/browser/prerender/prerender_browsertest.cc

Index: chrome/browser/prerender/prerender_browsertest.cc
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc
index 991e8f7fb53b14a7baaf2865dffd66fb69a5d875..c5225f2b6b094a79178fd773dbe457d748d11bc2 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -1042,9 +1042,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPage) {
           GetPrerenderManager());
   PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
   EXPECT_EQ(1, GetPrerenderDomContentLoadedEventCountForLinkNumber(0));
-  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
-  histogram_tester().ExpectTotalCount(
-      "Prerender.websame_PrerenderNotSwappedInPLT", 1);
 
   ChannelDestructionWatcher channel_close_watcher;
   channel_close_watcher.WatchChannel(
@@ -1053,7 +1050,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPage) {
   channel_close_watcher.WaitForChannelClose();
   fcp_waiter->Wait();
 
-  histogram_tester().ExpectTotalCount("Prerender.websame_PerceivedPLT", 1);
   histogram_tester().ExpectTotalCount(
       "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
   histogram_tester().ExpectTotalCount(
@@ -1119,14 +1115,10 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageCrossDomain) {
           GetPrerenderManager());
   PrerenderTestURL(GetCrossDomainTestUrl("prerender/prerender_page.html"),
                    FINAL_STATUS_USED, 1);
-  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
-  histogram_tester().ExpectTotalCount(
-      "Prerender.webcross_PrerenderNotSwappedInPLT", 1);

[mattcary, 2017/03/09 09:02:35]

I guess we're losing verification here of not being swapped in. If we don't
deprecate prerender, we'll have to figure out another way to verify that.

[pasko, 2017/03/23 13:33:24]

We actually perform a swap here in NavigateToDestURL(). The check is verifying
that the onload event fired before the swap happened. I think this also might be
flaky because I there might be no guaranteed ordering of the stoploading and fcp
IPCs.

I don't see why we need to verify that onload fired before swap in this case. It
was meaningful when we had histograms for it. I think in the current world
onload is similarly meaningless for prerender as domcontentloaded and other
loading progress events, hence I'm happy to remove it.

[mattcary, 2017/03/23 13:49:26]

sgtm, thx for digging.

 
   NavigateToDestURL();
   fcp_waiter->Wait();
 
-  histogram_tester().ExpectTotalCount("Prerender.webcross_PerceivedPLT", 1);
   histogram_tester().ExpectTotalCount(
       "Prerender.webcross_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
 }
@@ -1502,66 +1494,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
   NavigateToURL("/prerender/prerender_page.html");
 }
 
-// Checks that we get the right PPLT histograms for client redirect prerenders
-// and navigations when the referring page is Google.
-IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
-                       PrerenderLocationReplaceGWSHistograms) {
-  DisableJavascriptCalls();
-
-  // The loader page should look like Google.
-  static const char kGoogleDotCom[] = "www.google.com";
-  SetLoaderHostOverride(kGoogleDotCom);
-  set_loader_path("/prerender/prerender_loader_with_replace_state.html");

[mattcary, 2017/03/09 09:02:35]

I think you can remove this file from chrome/test/data?

[pasko, 2017/03/23 13:33:25]

Let me think more about this test case (gws prerender leads to a page with
client redirect). I noticed that we do not record PrefetchTTFCP. Do you know
why? I think it might be related to our skew in metrics.

[mattcary, 2017/03/23 13:49:26]

Hmmm. Redirects. We use extras.start_url in the PLMOs, for both prerender and
nostate. But if that isn't correct somehow, than for prerender we use the origin
from the tab helper and record under the Reference histogram (as prefetch_age is
zero), but for NoState/SimpleLoad we would put it under ORIGIN_NONE as well as
the Reference metric. Let me look at UMA and see if this theory is supported.

-
-  GURL dest_url =
-      GetCrossDomainTestUrl("prerender/prerender_deferred_image.html");
-
-  GURL prerender_url = embedded_test_server()->GetURL(
-      "/prerender/prerender_location_replace.html?" +
-      net::EscapeQueryParamValue(dest_url.spec(), false) + "#prerender");
-  GURL::Replacements replacements;
-  replacements.SetHostStr(kGoogleDotCom);
-  prerender_url = prerender_url.ReplaceComponents(replacements);
-
-  // The prerender will not completely load until after the swap, so wait for a
-  // title change before calling DidPrerenderPass.
-  std::unique_ptr<TestPrerender> prerender =
-      PrerenderTestURL(prerender_url, FINAL_STATUS_USED, 1);
-  WaitForASCIITitle(prerender->contents()->prerender_contents(), kReadyTitle);
-  EXPECT_TRUE(DidPrerenderPass(prerender->contents()->prerender_contents()));
-  EXPECT_EQ(1, prerender->number_of_loads());
-
-  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
-  // Although there is a client redirect, it is dropped from histograms because
-  // it is a Google URL. The target page itself does not load until after the
-  // swap.
-  histogram_tester().ExpectTotalCount("Prerender.gws_PrerenderNotSwappedInPLT",
-                                      0);
-
-  GURL navigate_url = embedded_test_server()->GetURL(
-      "/prerender/prerender_location_replace.html?" +
-      net::EscapeQueryParamValue(dest_url.spec(), false) + "#navigate");
-  navigate_url = navigate_url.ReplaceComponents(replacements);
-
-  NavigationOrSwapObserver swap_observer(
-      current_browser()->tab_strip_model(),
-      GetActiveWebContents(), 2);
-  current_browser()->OpenURL(OpenURLParams(navigate_url, Referrer(),
-                                           WindowOpenDisposition::CURRENT_TAB,
-                                           ui::PAGE_TRANSITION_TYPED, false));
-  swap_observer.Wait();
-
-  EXPECT_TRUE(DidDisplayPass(GetActiveWebContents()));
-
-  histogram_tester().ExpectTotalCount("Prerender.gws_PrerenderNotSwappedInPLT",
-                                      0);
-  histogram_tester().ExpectTotalCount("Prerender.gws_PerceivedPLT", 1);
-
-  // The client redirect does /not/ count as a miss because it's a Google URL.
-  histogram_tester().ExpectTotalCount("Prerender.PerceivedPLTFirstAfterMiss",
-                                      0);
-}
-
 // Checks that client-issued redirects work with prerendering.
 // This version navigates to the final destination page, rather than the
 // page which does the redirection via a mouse click.
@@ -3027,9 +2959,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderDeferredImage) {
   EXPECT_EQ(1, GetPrerenderDomContentLoadedEventCountForLinkNumber(0));
   EXPECT_TRUE(DidPrerenderPass(prerender->contents()->prerender_contents()));
   EXPECT_EQ(0, prerender->number_of_loads());
-  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
-  histogram_tester().ExpectTotalCount(
-      "Prerender.websame_PrerenderNotSwappedInPLT", 0);
 
   // Swap.
   NavigationOrSwapObserver swap_observer(current_browser()->tab_strip_model(),
@@ -3044,10 +2973,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderDeferredImage) {
 
   // Now check DidDisplayPass.
   EXPECT_TRUE(DidDisplayPass(GetActiveWebContents()));
-
-  histogram_tester().ExpectTotalCount(
-      "Prerender.websame_PrerenderNotSwappedInPLT", 0);
-  histogram_tester().ExpectTotalCount("Prerender.websame_PerceivedPLT", 1);
 }
 
 // Checks that a deferred redirect to an image is not loaded until the
@@ -3217,12 +3142,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPing) {
   ping_counter.WaitForCount(1);
 }
 
-IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPPLTNormalNavigation) {
-  GURL url = embedded_test_server()->GetURL("/prerender/prerender_page.html");
-  ui_test_utils::NavigateToURL(current_browser(), url);
-  histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
-}
-
 // Checks that a prerender which calls window.close() on itself is aborted.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderWindowClose) {
   DisableLoadEventCheck();