Reapply Wei Li's patch

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

Revert "Revert 130005 - Committing:"

This reverts commit 5e027b2d0c6749b0181fa01156f9080dac10ad95.

BUG=none
TEST=unit-tests
TBR=brettw,weili

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

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

12 files changed, 184 insertions, 11 deletions

diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index 1006640..a7fe776 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -385,6 +385,14 @@ 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 230eb4f4..3f2b32d 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -220,6 +220,17 @@ 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 c0ce78e..d603493 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -367,6 +367,29 @@ 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;
@@ -441,6 +464,9 @@ 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.
@@ -510,6 +536,9 @@ 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 90d29fc..317a6cb 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -136,6 +136,18 @@ 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);
@@ -391,8 +403,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, QueryFilteredURLs);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, UpdateVisitDuration);
 
   friend class ::TestingProfile;
 
@@ -435,6 +447,9 @@ 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 b4c0716..4bd9d29 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -1385,4 +1385,52 @@ 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 5a36951..f4c3aa1 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 = 20;
+static const int kCurrentVersionNumber = 21;
 static const int kCompatibleVersionNumber = 16;
 static const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold";
 
@@ -311,6 +311,17 @@ 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 83a5356..180db54 100644
--- a/chrome/browser/history/history_tab_helper.cc
+++ b/chrome/browser/history/history_tab_helper.cc
@@ -171,3 +171,21 @@ 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 9433600..f4d73d6 100644
--- a/chrome/browser/history/history_tab_helper.h
+++ b/chrome/browser/history/history_tab_helper.h
@@ -50,6 +50,7 @@ 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 823a836..30ded66 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -237,6 +237,12 @@ 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 b58573c..959cde3 100644
--- a/chrome/browser/history/visit_database.cc
+++ b/chrome/browser/history/visit_database.cc
@@ -19,7 +19,8 @@
 
 // Rows, in order, of the visit table.
 #define HISTORY_VISIT_ROW_FIELDS \
-  " id,url,visit_time,from_visit,transition,segment_id,is_indexed "
+    " id,url,visit_time,from_visit,transition,segment_id,is_indexed," \
+    "visit_duration "
 
 namespace history {
 
@@ -39,7 +40,8 @@ bool VisitDatabase::InitVisitTable() {
         "transition INTEGER DEFAULT 0 NOT NULL,"
         "segment_id INTEGER,"
         // True when we have indexed data for this visit.
-        "is_indexed BOOLEAN)"))
+        "is_indexed BOOLEAN,"
+        "visit_duration INTEGER DEFAULT 0 NOT NULL)"))
       return false;
   } else if (!GetDB().DoesColumnExist("visits", "is_indexed")) {
     // Old versions don't have the is_indexed column, we can just add that and
@@ -102,6 +104,8 @@ 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
@@ -122,14 +126,15 @@ 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) "
-      "VALUES (?,?,?,?,?,?)"));
+      "(url, visit_time, from_visit, transition, segment_id, is_indexed, "
+      "visit_duration) 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:  "
@@ -148,7 +153,7 @@ VisitID VisitDatabase::AddVisit(VisitRow* visit, VisitSource source) {
 
     if (!statement1.Run()) {
       VLOG(0) << "Failed to execute visit_source insert statement:  "
-              << "url_id = " << visit->visit_id;
+              << "id = " << visit->visit_id;
       return 0;
     }
   }
@@ -208,15 +213,16 @@ 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=? "
-      "WHERE id=?"));
+      "url=?,visit_time=?,from_visit=?,transition=?,segment_id=?,is_indexed=?,"
+      "visit_duration=? 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_id);
+  statement.BindInt64(6, visit.visit_duration.ToInternalValue());
+  statement.BindInt64(7, visit.visit_id);
 
   return statement.Run();
 }
@@ -536,4 +542,20 @@ 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 9f56bd5..988f0c7 100644
--- a/chrome/browser/history/visit_database.h
+++ b/chrome/browser/history/visit_database.h
@@ -188,6 +188,10 @@ 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 00299e3..272b923 100644
--- a/chrome/test/data/profiles/typical_history/Default/History
+++ b/chrome/test/data/profiles/typical_history/Default/History