content: updates NavigationRequest::ShouldUpdateHistory()

This makes it so that ShouldUpdateHistory() returns false for
same document navigations to the same url without a user gesture. A
common use of this history.replaceState(), which we don't really want
logged to history as it's not really what the user thinks of as a
navigation.

BUG=1103495
TEST=HistoryBrowserTest.ReplaceStateSamePageIsNotRecorded

Change-Id: Iafc0c7b93298940eb76cd1ca93ba7ffe3b2d32df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2305236
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790424}

chrome/browser/history/history_browsertest.cc

@@ -787,3 +787,28 @@ IN_PROC_BROWSER_TEST_F(HistoryBrowserTest, OneHistoryTabPerWindow) {
       browser()->tab_strip_model()->GetWebContentsAt(1);
   ASSERT_NE(history_url, second_tab->GetVisibleURL());
 }
+
+// Verifies history.replaceState() to the same url without a user gesture does
+// not log a visit.
+IN_PROC_BROWSER_TEST_F(HistoryBrowserTest, ReplaceStateSamePageIsNotRecorded) {
+  // Use the default embedded_test_server() for this test because replaceState
+  // requires a real, non-file URL.
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url(embedded_test_server()->GetURL("foo.com", "/title3.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Do a replaceState() to create a new navigation entry.
+  ASSERT_TRUE(content::ExecuteScript(web_contents,
+                                     "history.replaceState({foo: 'bar'},'')"));
+  content::WaitForLoadStop(web_contents);
+
+  // Because there was no user gesture and the url did not change, there should
+  // be a single url with a single visit.
+  std::vector<GURL> urls(GetHistoryContents());
+  ASSERT_EQ(1u, urls.size());
+  EXPECT_EQ(url, urls[0]);
+  history::QueryURLResult url_result = QueryURL(url);
+  EXPECT_EQ(1u, url_result.visits.size());
+}

content/browser/frame_host/navigation_request.cc

@@ -4003,6 +4003,16 @@ void NavigationRequest::DidCommitNavigation(
   common_params_->url = params.url;
   did_replace_entry_ = did_replace_entry;
   should_update_history_ = params.should_update_history;
+  // A same document navigation with the same url, and no user-gesture is
+  // typically the result of 'history.replaceState().' As the page is
+  // controlling this, the user doesn't really think of this as a navigation
+  // and it doesn't make sense to log this in history. Logging this in history
+  // would lead to lots of visits to a particular page, which impacts the
+  // visit count.
+  if (should_update_history_ && IsSameDocument() && !HasUserGesture() &&
+      params.url == previous_url) {
+    should_update_history_ = false;
+  }
   previous_url_ = previous_url;
   navigation_type_ = navigation_type;
 

content/public/browser/navigation_handle.h

@@ -246,7 +246,9 @@ class CONTENT_EXPORT NavigationHandle {
   virtual bool DidReplaceEntry() = 0;
 
   // Returns true if the browser history should be updated. Otherwise only
-  // the session history will be updated. E.g., on unreachable urls.
+  // the session history will be updated. E.g., on unreachable urls or other
+  // navigations that the users may not think of as navigations (such as
+  // happens with 'history.replaceState()').
   virtual bool ShouldUpdateHistory() = 0;
 
   // The previous main frame URL that the user was on. This may be empty if