Revert 130005 - Committing:

Changes to add duration into history database. This completes Issue 9605037 which can compute suggestion sites based on time slicing as well as visit duration. A new database table was added for those new fields we may need to experiment on.
Original cl: https://chromiumcodereview.appspot.com/9789001/
comment: HistoryProfileTest.TypicalProfileVersion fails because of the trybots inability to patch the History file - it runs fine locally.
Created by Wei Li weili@chromium.org
BUG=none
TEST=unit-tests
TBR=brettw

Review URL: https://chromiumcodereview.appspot.com/9963038

TBR=georgey@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9956046

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

12 files changed, 11 insertions, 184 deletions

diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index a7fe776..1006640 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -385,14 +385,6 @@ void HistoryService::SetPageTitle(const GURL& url,
   ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::SetPageTitle, url, title);
 }
 
-void HistoryService::UpdateWithPageEndTime(const void* host,
-                                           int32 page_id,
-                                           const GURL& url,
-                                           Time end_ts) {
-  ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::UpdateWithPageEndTime,
-                    host, page_id, url, end_ts);
-}
-
 void HistoryService::AddPageWithDetails(const GURL& url,
                                         const string16& title,
                                         int visit_count,
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index 3f2b32d..230eb4f4 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -220,17 +220,6 @@ class HistoryService : public CancelableRequestProvider,
   // title in the full text index.
   void SetPageTitle(const GURL& url, const string16& title);
 
-  // Updates the history database with a page's ending time stamp information.
-  // The page can be identified by the combination of the pointer to
-  // a RenderProcessHost, the page id and the url.
-  //
-  // The given pointer will not be dereferenced, it is only used for
-  // identification purposes, hence it is a void*.
-  void UpdateWithPageEndTime(const void* host,
-                             int32 page_id,
-                             const GURL& url,
-                             base::Time end_ts);
-
   // Indexing ------------------------------------------------------------------
 
   // Notifies history of the body text of the given recently-visited URL.
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index d603493..c0ce78e 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -367,29 +367,6 @@ SegmentID HistoryBackend::UpdateSegments(
   return segment_id;
 }
 
-void HistoryBackend::UpdateWithPageEndTime(const void* host,
-                                           int32 page_id,
-                                           const GURL& url,
-                                           Time end_ts) {
-  // Will be filled with the URL ID and the visit ID of the last addition.
-  VisitID visit_id = tracker_.GetLastVisit(host, page_id, url);
-  UpdateVisitDuration(visit_id, end_ts);
-}
-
-void HistoryBackend::UpdateVisitDuration(VisitID visit_id, const Time end_ts) {
-  if (!db_.get())
-    return;
-
-  // Get the starting visit_time for visit_id.
-  VisitRow visit_row;
-  if (db_->GetRowForVisit(visit_id, &visit_row)) {
-    // We should never have a negative duration time even when time is skewed.
-    visit_row.visit_duration = end_ts > visit_row.visit_time ?
-        end_ts - visit_row.visit_time : TimeDelta::FromMicroseconds(0);
-    db_->UpdateVisitRow(visit_row);
-  }
-}
-
 void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
   if (!db_.get())
     return;
@@ -464,9 +441,6 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
     if (!is_keyword_generated) {
       UpdateSegments(request->url, from_visit_id, last_ids.second, t,
                      last_recorded_time_);
-
-      // Update the referrer's duration.
-      UpdateVisitDuration(from_visit_id, last_recorded_time_);
     }
   } else {
     // Redirect case. Add the redirect chain.
@@ -536,9 +510,6 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
         // Update the segment for this visit.
         UpdateSegments(request->redirects[redirect_index],
                        from_visit_id, last_ids.second, t, last_recorded_time_);
-
-        // Update the visit_details for this visit.
-        UpdateVisitDuration(from_visit_id, last_recorded_time_);
       }
 
       // Subsequent transitions in the redirect list must all be sever
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index 317a6cb..90d29fc 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -136,18 +136,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   virtual void SetPageTitle(const GURL& url, const string16& title);
   void AddPageNoVisitForBookmark(const GURL& url);
 
-  // Updates the database backend with a page's ending time stamp information.
-  // The page can be identified by the combination of the pointer to
-  // a RenderProcessHost, the page id and the url.
-  //
-  // The given pointer will not be dereferenced, it is only used for
-  // identification purposes, hence it is a void*.
-  void UpdateWithPageEndTime(const void* host,
-                             int32 page_id,
-                             const GURL& url,
-                             base::Time end_ts);
-
-
   // Indexing ------------------------------------------------------------------
 
   void SetPageContents(const GURL& url, const string16& contents);
@@ -403,8 +391,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, GetFaviconForURL);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
                            CloneFaviconIsRestrictedToSameDomain);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, QueryFilteredURLs);
-  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, UpdateVisitDuration);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest,
+                           QueryFilteredURLs);
 
   friend class ::TestingProfile;
 
@@ -447,9 +435,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   void GetRedirectsToSpecificVisit(
       VisitID cur_visit, history::RedirectList* redirects);
 
-  // Update the visit_duration information in visits table.
-  void UpdateVisitDuration(VisitID visit_id, const base::Time end_ts);
-
   // Thumbnail Helpers ---------------------------------------------------------
 
   // When a simple GetMostRecentRedirectsFrom() fails, this method is
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index 4bd9d29..b4c0716 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -1385,52 +1385,4 @@ TEST_F(HistoryBackendTest, QueryFilteredURLs) {
             get_most_visited_list()[0].url.spec());
 }
 
-TEST_F(HistoryBackendTest, UpdateVisitDuration) {
-  // This unit test will test adding and deleting visit details information.
-  ASSERT_TRUE(backend_.get());
-
-  GURL url1("http://www.cnn.com");
-  std::vector<VisitInfo> visit_info1, visit_info2;
-  Time start_ts = Time::Now() - base::TimeDelta::FromDays(5);
-  Time end_ts = start_ts + base::TimeDelta::FromDays(2);
-  visit_info1.push_back(VisitInfo(start_ts, content::PAGE_TRANSITION_LINK));
-
-  GURL url2("http://www.example.com");
-  visit_info2.push_back(VisitInfo(Time::Now() - base::TimeDelta::FromDays(10),
-                                  content::PAGE_TRANSITION_LINK));
-
-  // Clear all history.
-  backend_->DeleteAllHistory();
-
-  // Add the visits.
-  backend_->AddVisits(url1, visit_info1, history::SOURCE_BROWSED);
-  backend_->AddVisits(url2, visit_info2, history::SOURCE_BROWSED);
-
-  // Verify the entries for both visits were added in visit_details.
-  VisitVector visits1, visits2;
-  URLRow row;
-  URLID url_id1 = backend_->db()->GetRowForURL(url1, &row);
-  ASSERT_TRUE(backend_->db()->GetVisitsForURL(url_id1, &visits1));
-  ASSERT_EQ(1U, visits1.size());
-  EXPECT_EQ(0, visits1[0].visit_duration.ToInternalValue());
-
-  URLID url_id2 = backend_->db()->GetRowForURL(url2, &row);
-  ASSERT_TRUE(backend_->db()->GetVisitsForURL(url_id2, &visits2));
-  ASSERT_EQ(1U, visits2.size());
-  EXPECT_EQ(0, visits2[0].visit_duration.ToInternalValue());
-
-  // Update the visit to cnn.com.
-  backend_->UpdateVisitDuration(visits1[0].visit_id, end_ts);
-
-  // Check the duration for visiting cnn.com was correctly updated.
-  ASSERT_TRUE(backend_->db()->GetVisitsForURL(url_id1, &visits1));
-  ASSERT_EQ(1U, visits1.size());
-  base::TimeDelta expected_duration = end_ts - start_ts;
-  EXPECT_EQ(expected_duration.ToInternalValue(),
-            visits1[0].visit_duration.ToInternalValue());
-
-  // Remove the visit to cnn.com.
-  ASSERT_TRUE(backend_->RemoveVisits(visits1));
-}
-
 }  // namespace history
diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc
index f4c3aa1..5a36951 100644
--- a/chrome/browser/history/history_database.cc
+++ b/chrome/browser/history/history_database.cc
@@ -28,7 +28,7 @@ namespace {
 // Current version number. We write databases at the "current" version number,
 // but any previous version that can read the "compatible" one can make do with
 // or database without *too* many bad effects.
-static const int kCurrentVersionNumber = 21;
+static const int kCurrentVersionNumber = 20;
 static const int kCompatibleVersionNumber = 16;
 static const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold";
 
@@ -311,17 +311,6 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion(
     meta_table_.SetValue(kNeedsThumbnailMigrationKey, 1);
   }
 
-  if (cur_version == 20) {
-    // This is the version prior to adding the visit_duration field in visits
-    // database. We need to migrate the database.
-    if (!MigrateVisitsWithoutDuration()) {
-      LOG(WARNING) << "Unable to update history database to version 21.";
-      return sql::INIT_FAILURE;
-    }
-    ++cur_version;
-    meta_table_.SetVersionNumber(cur_version);
-  }
-
   // When the version is too old, we just try to continue anyway, there should
   // not be a released product that makes a database too old for us to handle.
   LOG_IF(WARNING, cur_version < GetCurrentVersion()) <<
diff --git a/chrome/browser/history/history_tab_helper.cc b/chrome/browser/history/history_tab_helper.cc
index 180db54..83a5356 100644
--- a/chrome/browser/history/history_tab_helper.cc
+++ b/chrome/browser/history/history_tab_helper.cc
@@ -171,21 +171,3 @@ HistoryService* HistoryTabHelper::GetHistoryService() {
 
   return profile->GetHistoryService(Profile::IMPLICIT_ACCESS);
 }
-
-void HistoryTabHelper::WebContentsDestroyed(WebContents* tab) {
-  // We update the history for this URL.
-  // The content returned from web_contents() has been destroyed by now.
-  // We need to use tab value directly.
-  Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
-  if (profile->IsOffTheRecord())
-    return;
-
-  HistoryService* hs = profile->GetHistoryService(Profile::IMPLICIT_ACCESS);
-  if (hs) {
-    NavigationEntry* entry = tab->GetController().GetLastCommittedEntry();
-    if (entry) {
-      hs->UpdateWithPageEndTime(tab, entry->GetPageID(), tab->GetURL(),
-                                base::Time::Now());
-    }
-  }
-}
diff --git a/chrome/browser/history/history_tab_helper.h b/chrome/browser/history/history_tab_helper.h
index f4d73d6..9433600 100644
--- a/chrome/browser/history/history_tab_helper.h
+++ b/chrome/browser/history/history_tab_helper.h
@@ -50,7 +50,6 @@ class HistoryTabHelper : public content::WebContentsObserver,
   virtual void DidNavigateAnyFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
-  virtual void WebContentsDestroyed(content::WebContents* tab) OVERRIDE;
 
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 30ded66..823a836 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -237,12 +237,6 @@ class VisitRow {
   // change.
   bool is_indexed;
 
-  // Record how much time a user has this visit starting from the user
-  // opened this visit to the user closed or ended this visit.
-  // This includes both active and inactive time as long as
-  // the visit was present.
-  base::TimeDelta visit_duration;
-
   // Compares two visits based on dates, for sorting.
   bool operator<(const VisitRow& other) {
     return visit_time < other.visit_time;
diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc
index 959cde3..b58573c 100644
--- a/chrome/browser/history/visit_database.cc
+++ b/chrome/browser/history/visit_database.cc
@@ -19,8 +19,7 @@
 
 // Rows, in order, of the visit table.
 #define HISTORY_VISIT_ROW_FIELDS \
-    " id,url,visit_time,from_visit,transition,segment_id,is_indexed," \
-    "visit_duration "
+  " id,url,visit_time,from_visit,transition,segment_id,is_indexed "
 
 namespace history {
 
@@ -40,8 +39,7 @@ bool VisitDatabase::InitVisitTable() {
         "transition INTEGER DEFAULT 0 NOT NULL,"
         "segment_id INTEGER,"
         // True when we have indexed data for this visit.
-        "is_indexed BOOLEAN,"
-        "visit_duration INTEGER DEFAULT 0 NOT NULL)"))
+        "is_indexed BOOLEAN)"))
       return false;
   } else if (!GetDB().DoesColumnExist("visits", "is_indexed")) {
     // Old versions don't have the is_indexed column, we can just add that and
@@ -104,8 +102,6 @@ void VisitDatabase::FillVisitRow(sql::Statement& statement, VisitRow* visit) {
   visit->transition = content::PageTransitionFromInt(statement.ColumnInt(4));
   visit->segment_id = statement.ColumnInt64(5);
   visit->is_indexed = !!statement.ColumnInt(6);
-  visit->visit_duration =
-      base::TimeDelta::FromInternalValue(statement.ColumnInt64(7));
 }
 
 // static
@@ -126,15 +122,14 @@ bool VisitDatabase::FillVisitVector(sql::Statement& statement,
 VisitID VisitDatabase::AddVisit(VisitRow* visit, VisitSource source) {
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "INSERT INTO visits "
-      "(url, visit_time, from_visit, transition, segment_id, is_indexed, "
-      "visit_duration) VALUES (?,?,?,?,?,?,?)"));
+      "(url, visit_time, from_visit, transition, segment_id, is_indexed) "
+      "VALUES (?,?,?,?,?,?)"));
   statement.BindInt64(0, visit->url_id);
   statement.BindInt64(1, visit->visit_time.ToInternalValue());
   statement.BindInt64(2, visit->referring_visit);
   statement.BindInt64(3, visit->transition);
   statement.BindInt64(4, visit->segment_id);
   statement.BindInt64(5, visit->is_indexed);
-  statement.BindInt64(6, visit->visit_duration.ToInternalValue());
 
   if (!statement.Run()) {
     VLOG(0) << "Failed to execute visit insert statement:  "
@@ -153,7 +148,7 @@ VisitID VisitDatabase::AddVisit(VisitRow* visit, VisitSource source) {
 
     if (!statement1.Run()) {
       VLOG(0) << "Failed to execute visit_source insert statement:  "
-              << "id = " << visit->visit_id;
+              << "url_id = " << visit->visit_id;
       return 0;
     }
   }
@@ -213,16 +208,15 @@ bool VisitDatabase::UpdateVisitRow(const VisitRow& visit) {
 
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE visits SET "
-      "url=?,visit_time=?,from_visit=?,transition=?,segment_id=?,is_indexed=?,"
-      "visit_duration=? WHERE id=?"));
+      "url=?,visit_time=?,from_visit=?,transition=?,segment_id=?,is_indexed=? "
+      "WHERE id=?"));
   statement.BindInt64(0, visit.url_id);
   statement.BindInt64(1, visit.visit_time.ToInternalValue());
   statement.BindInt64(2, visit.referring_visit);
   statement.BindInt64(3, visit.transition);
   statement.BindInt64(4, visit.segment_id);
   statement.BindInt64(5, visit.is_indexed);
-  statement.BindInt64(6, visit.visit_duration.ToInternalValue());
-  statement.BindInt64(7, visit.visit_id);
+  statement.BindInt64(6, visit.visit_id);
 
   return statement.Run();
 }
@@ -542,20 +536,4 @@ void VisitDatabase::GetVisitsSource(const VisitVector& visits,
   }
 }
 
-bool VisitDatabase::MigrateVisitsWithoutDuration() {
-  if (!GetDB().DoesTableExist("visits")) {
-    NOTREACHED() << " Visits table should exist before migration";
-    return false;
-  }
-
-  if (!GetDB().DoesColumnExist("visits", "visit_duration")) {
-    // Old versions don't have the visit_duration column, we modify the table
-    // to add that field.
-    if (!GetDB().Execute("ALTER TABLE visits "
-        "ADD COLUMN visit_duration INTEGER DEFAULT 0 NOT NULL"))
-      return false;
-  }
-  return true;
-}
-
 }  // namespace history
diff --git a/chrome/browser/history/visit_database.h b/chrome/browser/history/visit_database.h
index 988f0c7..9f56bd5 100644
--- a/chrome/browser/history/visit_database.h
+++ b/chrome/browser/history/visit_database.h
@@ -188,10 +188,6 @@ class VisitDatabase {
   // hasn't happened yet.
   static bool FillVisitVector(sql::Statement& statement, VisitVector* visits);
 
-  // Called by the derived classes to migrate the older visits table which
-  // don't have visit_duration column yet.
-  bool MigrateVisitsWithoutDuration();
-
  private:
 
   DISALLOW_COPY_AND_ASSIGN(VisitDatabase);
diff --git a/chrome/test/data/profiles/typical_history/Default/History b/chrome/test/data/profiles/typical_history/Default/History
index 87280f9..00299e3 100644
--- a/chrome/test/data/profiles/typical_history/Default/History
+++ b/chrome/test/data/profiles/typical_history/Default/History