Searching by keyword now generates a visit against the site with a

transition type of TAB_TO_SEARCH. This visit increments the typed
count and ensures if you use TAB_TO_SEARCH you still get autocompleted
to the site.

I'll add some tests for this, but want to make sure we're ok with it
before I do that.

BUG=3633
TEST=will be covered by unit tests.

Review URL: http://codereview.chromium.org/93087

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@14609 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 87 insertions, 18 deletions

diff --git a/chrome/browser/history/expire_history_backend.cc b/chrome/browser/history/expire_history_backend.cc
index 145d9b6..070722bc 100644
--- a/chrome/browser/history/expire_history_backend.cc
+++ b/chrome/browser/history/expire_history_backend.cc
@@ -41,6 +41,7 @@ bool ShouldArchiveVisit(const VisitRow& visit) {
   // navigation and not part of a redirect chain.
   if ((no_qualifier == PageTransition::LINK ||
        no_qualifier == PageTransition::FORM_SUBMIT ||
+       no_qualifier == PageTransition::KEYWORD ||
        no_qualifier == PageTransition::GENERATED) &&
       visit.transition & PageTransition::CHAIN_END)
     return true;
@@ -320,9 +321,11 @@ void ExpireHistoryBackend::ExpireURLsForVisits(
     // 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 eliminate the need for this code.
-    const PageTransition::Type transition = visits[i].transition;
-    if (PageTransition::StripQualifier(transition) == PageTransition::TYPED &&
-        !PageTransition::IsRedirect(transition))
+    PageTransition::Type transition =
+        PageTransition::StripQualifier(visits[i].transition);
+    if ((transition == PageTransition::TYPED &&
+         !PageTransition::IsRedirect(visits[i].transition)) ||
+        transition == PageTransition::KEYWORD_GENERATED)
       cur.typed_count++;
   }
 
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index d045dc9..066a757 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -351,6 +351,10 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
   if (request->time < first_recorded_time_)
     first_recorded_time_ = request->time;
 
+  PageTransition::Type transition =
+      PageTransition::StripQualifier(request->transition);
+  bool is_keyword_generated = (transition == PageTransition::KEYWORD_GENERATED);
+
   if (request->redirects.size() <= 1) {
     // The single entry is both a chain start and end.
     PageTransition::Type t = request->transition |
@@ -360,13 +364,15 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
     last_ids = AddPageVisit(request->url, last_recorded_time_,
                             last_ids.second, t);
 
-    // Update the segment for this visit.
-    UpdateSegments(request->url, from_visit_id, last_ids.second, t,
-                   last_recorded_time_);
+    // Update the segment for this visit. KEYWORD_GENERATED visits should not
+    // result in changing most visited, so we don't update segments (most
+    // visited db).
+    if (!is_keyword_generated) {
+      UpdateSegments(request->url, from_visit_id, last_ids.second, t,
+                     last_recorded_time_);
+    }
   } else {
     // Redirect case. Add the redirect chain.
-    PageTransition::Type transition =
-        PageTransition::StripQualifier(request->transition);
 
     PageTransition::Type redirect_info = PageTransition::CHAIN_START;
 
@@ -445,10 +451,8 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
   // TODO(evanm): Due to http://b/1194536 we lose the referrers of a subframe
   // navigation anyway, so last_visit_id is always zero for them.  But adding
   // them here confuses main frame history, so we skip them for now.
-  PageTransition::Type transition =
-      PageTransition::StripQualifier(request->transition);
   if (transition != PageTransition::AUTO_SUBFRAME &&
-      transition != PageTransition::MANUAL_SUBFRAME) {
+      transition != PageTransition::MANUAL_SUBFRAME && !is_keyword_generated) {
     tracker_.AddVisit(request->id_scope, request->page_id, request->url,
                       last_ids.second);
   }
@@ -586,8 +590,11 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit(
   // 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;
-  if (PageTransition::StripQualifier(transition) == PageTransition::TYPED &&
-      !PageTransition::IsRedirect(transition))
+  PageTransition::Type transition_type =
+      PageTransition::StripQualifier(transition);
+  if ((transition_type == PageTransition::TYPED &&
+       !PageTransition::IsRedirect(transition)) ||
+      transition_type == PageTransition::KEYWORD_GENERATED)
     typed_increment = 1;
 
   // See if this URL is already in the DB.
@@ -639,6 +646,7 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit(
   // Broadcast a notification of the visit.
   if (visit_id) {
     URLVisitedDetails* details = new URLVisitedDetails;
+    details->transition = transition;
     details->row = url_info;
     BroadcastNotifications(NotificationType::HISTORY_URL_VISITED, details);
   }
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index 6201ec6..d8af551 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -257,6 +257,12 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   // added for each given URL at the last visit time in the URLRow.
   void AddPagesWithDetails(const std::vector<URLRow>& info);
 
+#if defined(UNIT_TEST)
+  HistoryDatabase* db() const { return db_.get(); }
+
+  ExpireHistoryBackend* expire_backend() { return &expirer_; }
+#endif
+
  private:
   friend class CommitLaterTask;  // The commit task needs to call Commit().
   friend class HistoryTest;  // So the unit tests can poke our innards.
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index f64c109..e6c2879 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -395,4 +395,53 @@ TEST_F(HistoryBackendTest, GetPageThumbnailAfterRedirects) {
   EXPECT_TRUE(data.get());
 }
 
+// Tests a handful of assertions for a navigation with a type of
+// KEYWORD_GENERATED.
+TEST_F(HistoryBackendTest, KeywordGenerated) {
+  ASSERT_TRUE(backend_.get());
+
+  GURL url("http://google.com");
+
+  Time visit_time = Time::Now() - base::TimeDelta::FromDays(1);
+  scoped_refptr<HistoryAddPageArgs> request(
+      new HistoryAddPageArgs(url, visit_time, NULL, 0, GURL(),
+                             HistoryService::RedirectList(),
+                             PageTransition::KEYWORD_GENERATED));
+  backend_->AddPage(request);
+
+  // A row should have been added for the url.
+  URLRow row;
+  URLID url_id = backend_->db()->GetRowForURL(url, &row);
+  ASSERT_NE(0, url_id);
+
+  // The typed count should be 1.
+  ASSERT_EQ(1, row.typed_count());
+
+  // KEYWORD_GENERATED urls should not be added to the segment db.
+  std::string segment_name = VisitSegmentDatabase::ComputeSegmentName(url);
+  EXPECT_EQ(0, backend_->db()->GetSegmentNamed(segment_name));
+
+  // One visit should be added.
+  VisitVector visits;
+  EXPECT_TRUE(backend_->db()->GetVisitsForURL(url_id, &visits));
+  EXPECT_EQ(1U, visits.size());
+
+  // But no visible visits.
+  visits.clear();
+  backend_->db()->GetVisibleVisitsInRange(
+      base::Time(), base::Time(), false, 1, &visits);
+  EXPECT_TRUE(visits.empty());
+
+  // Expire the visits.
+  backend_->expire_backend()->ExpireHistoryBetween(visit_time, Time::Now());
+
+  // The visit should have been nuked.
+  visits.clear();
+  EXPECT_TRUE(backend_->db()->GetVisitsForURL(url_id, &visits));
+  EXPECT_TRUE(visits.empty());
+
+  // As well as the url.
+  ASSERT_EQ(0, backend_->db()->GetRowForURL(url, &row));
+}
+
 }  // namespace history
diff --git a/chrome/browser/history/history_notifications.h b/chrome/browser/history/history_notifications.h
index a0ed455..4eb9289 100644
--- a/chrome/browser/history/history_notifications.h
+++ b/chrome/browser/history/history_notifications.h
@@ -23,8 +23,9 @@ struct HistoryDetails {
   virtual ~HistoryDetails() {}
 };
 
-// Details for NOTIFY_HISTORY_URL_VISITED.
+// Details for HISTORY_URL_VISITED.
 struct URLVisitedDetails : public HistoryDetails {
+  PageTransition::Type transition;
   URLRow row;
 };
 
diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc
index e64a09d..ed5e4e9 100644
--- a/chrome/browser/history/history_unittest.cc
+++ b/chrome/browser/history/history_unittest.cc
@@ -605,7 +605,7 @@ TEST_F(HistoryTest, Segments) {
   // Wait for processing.
   MessageLoop::current()->Run();
 
-  EXPECT_EQ(page_usage_data_->size(), 1U);
+  ASSERT_EQ(1U, page_usage_data_->size());
   EXPECT_TRUE(page_usage_data_[0]->GetURL() == existing_url);
   EXPECT_DOUBLE_EQ(3.0, page_usage_data_[0]->GetScore());
 
@@ -624,7 +624,7 @@ TEST_F(HistoryTest, Segments) {
   MessageLoop::current()->Run();
 
   // Make sure we still have one segment.
-  EXPECT_EQ(page_usage_data_->size(), 1U);
+  ASSERT_EQ(1U, page_usage_data_->size());
   EXPECT_TRUE(page_usage_data_[0]->GetURL() == existing_url);
 
   // Add a page linked from existing_url.
@@ -641,7 +641,7 @@ TEST_F(HistoryTest, Segments) {
   MessageLoop::current()->Run();
 
   // Make sure we still have one segment.
-  EXPECT_EQ(page_usage_data_->size(), 1U);
+  ASSERT_EQ(1U, page_usage_data_->size());
   EXPECT_TRUE(page_usage_data_[0]->GetURL() == existing_url);
 
   // However, the score should have increased.
diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc
index 4e796de..c118bf2 100644
--- a/chrome/browser/history/visit_database.cc
+++ b/chrome/browser/history/visit_database.cc
@@ -224,7 +224,8 @@ void VisitDatabase::GetVisibleVisitsInRange(Time begin_time, Time end_time,
       "SELECT" HISTORY_VISIT_ROW_FIELDS "FROM visits "
       "WHERE visit_time >= ? AND visit_time < ? "
       "AND (transition & ?) != 0 "  // CHAIN_END
-      "AND (transition & ?) NOT IN (?, ?) "  // NO SUBFRAME
+      "AND (transition & ?) NOT IN (?, ?, ?) "  // NO SUBFRAME or
+                                                // KEYWORD_GENERATED
       "ORDER BY visit_time DESC, id DESC");
   if (!statement.is_valid())
     return;
@@ -239,6 +240,7 @@ void VisitDatabase::GetVisibleVisitsInRange(Time begin_time, Time end_time,
   statement->bind_int(3, PageTransition::CORE_MASK);
   statement->bind_int(4, PageTransition::AUTO_SUBFRAME);
   statement->bind_int(5, PageTransition::MANUAL_SUBFRAME);
+  statement->bind_int(6, PageTransition::KEYWORD_GENERATED);
 
   std::set<URLID> found_urls;
   while (statement->step() == SQLITE_ROW) {