Don't treat back/forwards as typed transitions in history.

BUG=393441
TEST=See bug comment 23 for repro steps

Review URL: https://codereview.chromium.org/1011943003

Cr-Commit-Position: refs/heads/master@{#322644}

chrome/browser/history/history_backend_unittest.cc

@@ -888,6 +888,17 @@ TEST_F(HistoryBackendTest, KeywordGenerated) {
   backend_->db()->GetVisibleVisitsInRange(query_options, &visits);
   EXPECT_TRUE(visits.empty());
 
+  // Going back to the same entry should not increment the typed count.
+  ui::PageTransition back_transition = ui::PageTransitionFromInt(
+      ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FORWARD_BACK);
+  HistoryAddPageArgs back_request(url, visit_time, NULL, 0, GURL(),
+                                  history::RedirectList(), back_transition,
+                                  history::SOURCE_BROWSED, false);
+  backend_->AddPage(back_request);
+  url_id = backend_->db()->GetRowForURL(url, &row);
+  ASSERT_NE(0, url_id);
+  ASSERT_EQ(1, row.typed_count());
+
   // Expire the visits.
   std::set<GURL> restrict_urls;
   backend_->expire_backend()->ExpireHistoryBetween(restrict_urls,
@@ -1202,6 +1213,81 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) {
   ASSERT_EQ(1U, visits.size());
 }
 
+TEST_F(HistoryBackendTest, AddPageVisitBackForward) {
+  ASSERT_TRUE(backend_.get());
+
+  GURL url("http://www.google.com");
+
+  // Clear all history.
+  backend_->DeleteAllHistory();
+
+  // Visit the url after typing it.
+  backend_->AddPageVisit(url, base::Time::Now(), 0,
+      ui::PAGE_TRANSITION_TYPED,
+      history::SOURCE_BROWSED);
+
+  // Ensure both the typed count and visit count are 1.
+  VisitVector visits;
+  URLRow row;
+  URLID id = backend_->db()->GetRowForURL(url, &row);
+  ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits));
+  EXPECT_EQ(1, row.typed_count());
+  EXPECT_EQ(1, row.visit_count());
+
+  // Visit the url again via back/forward.
+  backend_->AddPageVisit(url, base::Time::Now(), 0,
+      ui::PageTransitionFromInt(
+          ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FORWARD_BACK),
+      history::SOURCE_BROWSED);
+
+  // Ensure the typed count is still 1 but the visit count is 2.
+  id = backend_->db()->GetRowForURL(url, &row);
+  ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits));
+  EXPECT_EQ(1, row.typed_count());
+  EXPECT_EQ(2, row.visit_count());
+}
+
+TEST_F(HistoryBackendTest, AddPageVisitRedirectBackForward) {
+  ASSERT_TRUE(backend_.get());
+
+  GURL url1("http://www.google.com");
+  GURL url2("http://www.chromium.org");
+
+  // Clear all history.
+  backend_->DeleteAllHistory();
+
+  // Visit a typed URL with a redirect.
+  backend_->AddPageVisit(url1, base::Time::Now(), 0,
+      ui::PAGE_TRANSITION_TYPED,
+      history::SOURCE_BROWSED);
+  backend_->AddPageVisit(url2, base::Time::Now(), 0,
+      ui::PageTransitionFromInt(
+          ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_CLIENT_REDIRECT),
+      history::SOURCE_BROWSED);
+
+  // Ensure the redirected URL does not count as typed.
+  VisitVector visits;
+  URLRow row;
+  URLID id = backend_->db()->GetRowForURL(url2, &row);
+  ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits));
+  EXPECT_EQ(0, row.typed_count());
+  EXPECT_EQ(1, row.visit_count());
+
+  // Visit the redirected url again via back/forward.
+  backend_->AddPageVisit(url2, base::Time::Now(), 0,
+      ui::PageTransitionFromInt(
+          ui::PAGE_TRANSITION_TYPED |
+          ui::PAGE_TRANSITION_FORWARD_BACK |
+          ui::PAGE_TRANSITION_CLIENT_REDIRECT),
+      history::SOURCE_BROWSED);
+
+  // Ensure the typed count is still 1 but the visit count is 2.
+  id = backend_->db()->GetRowForURL(url2, &row);
+  ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits));
+  EXPECT_EQ(0, row.typed_count());
+  EXPECT_EQ(2, row.visit_count());
+}
+
 TEST_F(HistoryBackendTest, AddPageVisitSource) {
   ASSERT_TRUE(backend_.get());
 

components/history/core/browser/expire_history_backend.cc

@@ -386,16 +386,14 @@ void ExpireHistoryBackend::ExpireURLsForVisits(const VisitVector& visits,
   for (size_t i = 0; i < visits.size(); i++) {
     ChangedURL& cur = changed_urls[visits[i].url_id];
     // NOTE: This code must stay in sync with HistoryBackend::AddPageVisit().
-    // TODO(pkasting): http://b/1148304 We shouldn't be marking so many URLs as
-    // typed, which would help eliminate the need for this code (we still would
-    // need to handle RELOAD transitions specially, though).
     ui::PageTransition transition =
         ui::PageTransitionStripQualifier(visits[i].transition);
     if (transition != ui::PAGE_TRANSITION_RELOAD)
       cur.visit_count++;
-    if ((transition == ui::PAGE_TRANSITION_TYPED &&
-        !ui::PageTransitionIsRedirect(visits[i].transition)) ||
-        transition == ui::PAGE_TRANSITION_KEYWORD_GENERATED)
+    if (ui::PageTransitionIsNewNavigation(visits[i].transition) &&
+        ((transition == ui::PAGE_TRANSITION_TYPED &&
+          !ui::PageTransitionIsRedirect(visits[i].transition)) ||
+         transition == ui::PAGE_TRANSITION_KEYWORD_GENERATED))
       cur.typed_count++;
   }
 

components/history/core/browser/history_backend.cc

@@ -725,14 +725,13 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit(
 
   // NOTE: This code must stay in sync with
   // ExpireHistoryBackend::ExpireURLsForVisits().
-  // TODO(pkasting): http://b/1148304 We shouldn't be marking so many URLs as
-  // typed, which would eliminate the need for this code.
   int typed_increment = 0;
   ui::PageTransition transition_type =
       ui::PageTransitionStripQualifier(transition);
-  if ((transition_type == ui::PAGE_TRANSITION_TYPED &&
-       !ui::PageTransitionIsRedirect(transition)) ||
-      transition_type == ui::PAGE_TRANSITION_KEYWORD_GENERATED)
+  if (ui::PageTransitionIsNewNavigation(transition) &&
+      ((transition_type == ui::PAGE_TRANSITION_TYPED &&
+        !ui::PageTransitionIsRedirect(transition)) ||
+       transition_type == ui::PAGE_TRANSITION_KEYWORD_GENERATED))
     typed_increment = 1;
 
   // See if this URL is already in the DB.

components/history/core/browser/history_backend.h

@@ -466,6 +466,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, StripUsernamePasswordTest);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteThumbnailsDatabaseTest);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitSource);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitBackForward);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitRedirectBackForward);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitNotLastVisit);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
                            AddPageVisitFiresNotificationWithCorrectDetails);