Prerender: Remove PerceivedPLT histograms

chrome/browser/prerender/prerender_browsertest.cc

 // Copyright (c) 2012 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 <stddef.h>
 #include <deque>
 #include <set>
 #include <string>
 #include <unordered_map>
 #include <utility>
@@ skipping 1024 matching lines @@
 
 // Checks that a page is correctly prerendered in the case of a
 // <link rel=prerender> tag and then loaded into a tab in response to a
 // navigation.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPage) {
   test_utils::FirstContentfulPaintManagerWaiter* fcp_waiter =
       test_utils::FirstContentfulPaintManagerWaiter::Create(
           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(
       GetActiveWebContents()->GetRenderProcessHost());
   NavigateToDestURL();
   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(
       "Prerender.websame_PerceivedTTFCPRecorded.Visible", 1);
 
   ASSERT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
 // Checks that the correct page load metrics observers are produced without a
 // prerender.
@@ skipping 45 matching lines @@
       "PageLoad.PaintTiming.NavigationToFirstContentfulPaint", 0);
 }
 
 // Checks that cross-domain prerenders emit the correct histograms.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageCrossDomain) {
   test_utils::FirstContentfulPaintManagerWaiter* fcp_waiter =
       test_utils::FirstContentfulPaintManagerWaiter::Create(
           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);
 }
 
 // Checks that pending prerenders launch and receive proper event treatment.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPagePending) {
   std::unique_ptr<TestPrerender> prerender = PrerenderTestURL(
       "/prerender/prerender_page_pending.html", FINAL_STATUS_USED, 1);
 
   // Navigate to the prerender.
@@ skipping 355 matching lines @@
 
 // Checks that redirects with location.replace do not cancel a prerender and
 // and swap when navigating to the second.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderLocationReplaceNavigateToSecond) {
   PrerenderTestURL("/prerender/prerender_location_replace.html",
                    FINAL_STATUS_USED, 2);
   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.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderClientRedirectNavigateToSecondViaClick) {
   GURL prerender_url = embedded_test_server()->GetURL(
       CreateClientRedirect("/prerender/prerender_page.html"));
   GURL destination_url =
       embedded_test_server()->GetURL("/prerender/prerender_page.html");
   PrerenderTestURL(prerender_url, FINAL_STATUS_USED, 2);
@@ skipping 1445 matching lines @@
   DisableJavascriptCalls();
 
   // 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/prerender_deferred_image.html", FINAL_STATUS_USED, 0);
   WaitForASCIITitle(prerender->contents()->prerender_contents(), kReadyTitle);
   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(),
                                          GetActiveWebContents());
   ui_test_utils::NavigateToURLWithDisposition(
       current_browser(), dest_url(), WindowOpenDisposition::CURRENT_TAB,
       ui_test_utils::BROWSER_TEST_NONE);
   swap_observer.Wait();
 
   // The prerender never observes the final load.
   EXPECT_EQ(0, prerender->number_of_loads());
 
   // 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
 // page is visible, even after another redirect.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
                        PrerenderDeferredImageAfterRedirect) {
   DisableJavascriptCalls();
 
   // The prerender will not completely load until after the swap, so wait for a
   // title change before calling DidPrerenderPass.
@@ skipping 149 matching lines @@
       BrowserThread::IO, FROM_HERE,
       base::Bind(&CreateCountingInterceptorOnIO,
                  kPingURL, empty_file, ping_counter.AsWeakPtr()));
 
   PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
   OpenDestURLViaClickPing(kPingURL);
 
   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();
   PrerenderTestURL("/prerender/prerender_window_close.html",
                    FINAL_STATUS_CLOSED, 0);
 }
 
 // Tests interaction between prerender and POST (i.e. POST request should still
 // be made and POST data should not be dropped when the POST target is the same
 // as a prerender link).
@@ skipping 575 matching lines @@
       browser()->tab_strip_model()->GetActiveWebContents();
   bool display_test_result = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(web_contents,
                                                    "DidDisplayReallyPass()",
                                                    &display_test_result));
   ASSERT_TRUE(display_test_result);
 }
 #endif  // !defined(DISABLE_NACL)
 
 }  // namespace prerender