Reload: deprecate consecutive reload duration time metrics

Deprecate metrics that were introduced to monitor reload duration times
to investigate a chance to detect a signal for users' reload intention.
But, we could not figure out good characteristics to utilize this
information.

This change effectively reverts the following CL, but the reload_type_
in NavigationHandleImpl will be remained because it's now used for
another purpose.

https://codereview.chromium.org/2174293002

Bug: 975314
Change-Id: Idda509df284247c42687a69a2a654607e921b788
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670142
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Auto-Submit: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672066}
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 433274e..818e8e4 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -523,8 +523,7 @@
       is_initial_navigation_(true),
       in_navigate_to_pending_entry_(false),
       pending_reload_(ReloadType::NONE),
-      get_timestamp_callback_(base::Bind(&base::Time::Now)),
-      last_committed_reload_type_(ReloadType::NONE) {
+      get_timestamp_callback_(base::Bind(&base::Time::Now)) {
   DCHECK(browser_context_);
 }
 
@@ -605,26 +604,7 @@
   if (!entry)
     return;
 
-  // Check if previous navigation was a reload to track consecutive reload
-  // operations.
-  if (last_committed_reload_type_ != ReloadType::NONE) {
-    DCHECK(!last_committed_reload_time_.is_null());
-    base::Time now =
-        time_smoother_.GetSmoothedTime(get_timestamp_callback_.Run());
-    DCHECK_GT(now, last_committed_reload_time_);
-    if (!last_committed_reload_time_.is_null() &&
-        now > last_committed_reload_time_) {
-      base::TimeDelta delta = now - last_committed_reload_time_;
-      UMA_HISTOGRAM_MEDIUM_TIMES("Navigation.Reload.ReloadToReloadDuration",
-                                 delta);
-      if (last_committed_reload_type_ == ReloadType::NORMAL) {
-        UMA_HISTOGRAM_MEDIUM_TIMES(
-            "Navigation.Reload.ReloadMainResourceToReloadDuration", delta);
-      }
-    }
-  }
-
-  // Set ReloadType for |entry| in order to check it at commit time.
+  // Set ReloadType for |entry|.
   entry->set_reload_type(reload_type);
 
   if (g_check_for_repost && check_for_repost &&
@@ -1056,28 +1036,13 @@
   // is_same_document must be computed before the entry gets committed.
   details->is_same_document = is_same_document_navigation;
 
-  // Save reload type and timestamp for a reload navigation to detect
-  // consecutive reloads when the next reload is requested.
-  NavigationHandleImpl* navigation_handle =
-      navigation_request->navigation_handle();
-  DCHECK(navigation_handle);
-  if (PendingEntryMatchesHandle(navigation_handle)) {
-    if (pending_entry_->reload_type() != ReloadType::NONE) {
-      last_committed_reload_type_ = pending_entry_->reload_type();
-      last_committed_reload_time_ =
-          time_smoother_.GetSmoothedTime(get_timestamp_callback_.Run());
-    } else if (!pending_entry_->is_renderer_initiated() ||
-               params.gesture == NavigationGestureUser) {
-      last_committed_reload_type_ = ReloadType::NONE;
-      last_committed_reload_time_ = base::Time();
-    }
-  }
-
   // Make sure we do not discard the pending entry for a different ongoing
   // navigation when a same document commit comes in unexpectedly from the
   // renderer.  Limit this to a very narrow set of conditions to avoid risks to
   // other navigation types. See https://crbug.com/900036.
   // TODO(crbug.com/926009): Handle history.pushState() as well.
+  NavigationHandleImpl* navigation_handle =
+      navigation_request->navigation_handle();
   bool keep_pending_entry = is_same_document_navigation &&
                             details->type == NAVIGATION_TYPE_EXISTING_PAGE &&
                             pending_entry_ &&
diff --git a/content/browser/frame_host/navigation_controller_impl.h b/content/browser/frame_host/navigation_controller_impl.h
index 9491744..48796303 100644
--- a/content/browser/frame_host/navigation_controller_impl.h
+++ b/content/browser/frame_host/navigation_controller_impl.h
@@ -582,13 +582,6 @@
   // the wrong order in the history view.
   TimeSmoother time_smoother_;
 
-  // Used for tracking consecutive reload requests.  If the last user-initiated
-  // navigation (either browser-initiated or renderer-initiated with a user
-  // gesture) was a reload, these hold the ReloadType and timestamp.  Otherwise
-  // these are ReloadType::NONE and a null timestamp, respectively.
-  ReloadType last_committed_reload_type_;
-  base::Time last_committed_reload_time_;
-
   // BackForwardCache:
   //
   // Stores frozen RenderFrameHost. Restores them on history navigation.
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index c25849f..d1c1ce1 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -20,7 +20,6 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
-#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread_restrictions.h"
@@ -7319,121 +7318,6 @@
   // test will fail.
 }
 
-namespace {
-
-// Execute JavaScript without the user gesture flag set, and wait for the
-// triggered load finished.
-void ExecuteJavaScriptAndWaitForLoadStop(WebContents* web_contents,
-                                         const std::string script) {
-  // WaitForLoadStop() does not work to wait for loading that is triggered by
-  // JavaScript asynchronously.
-  TestNavigationObserver observer(web_contents);
-
-  // ExecJs() sets a user gesture flag internally for testing, but we
-  // want to run JavaScript without the flag.  Call ExecuteJavaScriptForTests
-  // directly.
-  static_cast<WebContentsImpl*>(web_contents)
-      ->GetMainFrame()
-      ->ExecuteJavaScriptForTests(base::UTF8ToUTF16(script),
-                                  base::NullCallback());
-
-  observer.Wait();
-}
-
-}  // namespace
-
-// Check if consecutive reloads can be correctly captured by metrics.
-IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
-                       ConsecutiveReloadMetrics) {
-  base::HistogramTester histogram;
-
-  const char kReloadToReloadMetricName[] =
-      "Navigation.Reload.ReloadToReloadDuration";
-  const char kReloadMainResourceToReloadMetricName[] =
-      "Navigation.Reload.ReloadMainResourceToReloadDuration";
-
-  // Navigate to a page, and check if metrics are initialized correctly.
-  EXPECT_TRUE(NavigateToURL(
-      shell(), embedded_test_server()->GetURL(
-                   "/navigation_controller/page_with_links.html")));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 0);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 0);
-
-  NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
-      shell()->web_contents()->GetController());
-
-  // Reload triggers a reload of ReloadType::NORMAL.  The first reload should
-  // not be counted.
-  controller.Reload(ReloadType::NORMAL, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 0);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 0);
-
-  // Reload with ReloadType::BYPASSING_CACHE.  Both metrics should count the
-  // consecutive reloads.
-  controller.Reload(ReloadType::BYPASSING_CACHE, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 1);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 1);
-
-  // Triggers another reload with ReloadType::BYPASSING_CACHE.
-  // ReloadMainResourceToReload should not be counted here.
-  controller.Reload(ReloadType::BYPASSING_CACHE, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 2);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 1);
-
-  // A browser-initiated navigation should reset the reload tracking
-  // information.
-  EXPECT_TRUE(
-      NavigateToURL(shell(), embedded_test_server()->GetURL(
-                                 "/navigation_controller/simple_page_1.html")));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 2);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 1);
-
-  // Then, the next reload should be assumed as the first reload.  Metrics
-  // should not be changed for the first reload.
-  controller.Reload(ReloadType::NORMAL, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 2);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 1);
-
-  // Another reload of ReloadType::NORMAL should be counted by both metrics
-  // again.
-  controller.Reload(ReloadType::NORMAL, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 3);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 2);
-
-  // A renderer-initiated navigations with no user gesture don't reset reload
-  // tracking information, and the following reload will be counted by metrics.
-  ExecuteJavaScriptAndWaitForLoadStop(
-      shell()->web_contents(),
-      "history.pushState({}, 'page 1', 'simple_page_1.html')");
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 3);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 2);
-  ExecuteJavaScriptAndWaitForLoadStop(shell()->web_contents(),
-                                      "location.href='simple_page_2.html'");
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 3);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 2);
-
-  controller.Reload(ReloadType::NORMAL, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 4);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 3);
-
-  // Go back to the first page. Reload tracking information should be reset.
-  shell()->web_contents()->GetController().GoBack();
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 4);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 3);
-
-  controller.Reload(ReloadType::NORMAL, false);
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  histogram.ExpectTotalCount(kReloadToReloadMetricName, 4);
-  histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 3);
-}
-
 // Check that the referrer is stored inside FrameNavigationEntry for subframes.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        RefererStoredForSubFrame) {
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 4f0385d..407c67e 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -65220,7 +65220,11 @@
 
 <histogram name="Navigation.Reload.ReloadMainResourceToReloadDuration"
     units="ms" expires_after="M77">
+  <obsolete>
+    Removed 06/2019.
+  </obsolete>
   <owner>toyoshim@chromium.org</owner>
+  <owner>kinuko@chromium.org</owner>
   <summary>
     Reported when a user triggers reload without any other navigations after the
     previous reload in the same page, and the previous reload variant was
@@ -65230,7 +65234,11 @@
 
 <histogram name="Navigation.Reload.ReloadToReloadDuration" units="ms"
     expires_after="M77">
+  <obsolete>
+    Removed 06/2019.
+  </obsolete>
   <owner>toyoshim@chromium.org</owner>
+  <owner>kinuko@chromium.org</owner>
   <summary>
     Reported when a user triggers reload without any other navigations after the
     previous reload in the same page. Duration time between two reloads is