Changes to add duration into history database.

chrome/browser/history/visit_database.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/history/visit_database.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <set>
 
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/string_number_conversions.h"
 #include "base/timer.h"
 #include "base/stl_util.h"
 #include "chrome/browser/history/url_database.h"
 #include "chrome/browser/history/visit_filter.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/common/page_transition_types.h"
 #include "sql/statement.h"
 
 // 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," \

[GeorgeY, 2012/03/27 21:15:57]

nit: I think we should have 4 spaces indent here and on the next line.

[Wei Li, 2012/03/27 23:01:50]

Done.

+  "visit_duration "
 
 namespace history {
 
 // Performs analysis of all local browsing data in the visits table to
 // assess the feasibility of performing prerendering on that data.
 // Will emulate prerendering based on a simple heuristic: On each
 // pageview, pick most likely next page viewed in the next 5 minutes
 // based on historical data; keep a maximum of 5 prerenders; evict
 // prerenders older than 5 minutes or based on likelihood.  Will report
 // back hypothetical prerender rate & accuracy via histograms.
@@ skipping 300 matching lines @@
   visit_analysis_->Init();
   if (!GetDB().DoesTableExist("visits")) {
     if (!GetDB().Execute("CREATE TABLE visits("
         "id INTEGER PRIMARY KEY,"
         "url INTEGER NOT NULL," // key of the URL this corresponds to
         "visit_time INTEGER NOT NULL,"
         "from_visit INTEGER,"
         "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
-    // not worry about different database revisions, since old ones will
-    // continue to work.
-    //
-    // TODO(brettw) this should be removed once we think everybody has been
-    // updated (added early Mar 2008).
-    if (!GetDB().Execute("ALTER TABLE visits ADD COLUMN 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
+      // not worry about different database revisions, since old ones will
+      // continue to work.
+      //
+      // TODO(brettw) this should be removed once we think everybody has been
+      // updated (added early Mar 2008).
+      if (!GetDB().Execute("ALTER TABLE visits ADD COLUMN is_indexed BOOLEAN"))
+        return false;
+    }
+    if (!GetDB().DoesColumnExist("visits", "visit_duration")) {

[GeorgeY, 2012/03/27 21:15:57]

Why do we need to do here as well in addition to MigrateVisitsWithoutDuration()?

[Wei Li, 2012/03/27 23:01:50]

I used this to guard some manual errors such as version in meta_table changed
while the new column was not added. For normal uses, this should not happen, so
I removed it.

+      // Old versions don't have the visit_duration column, we can just add that
+      // without worrying about different database revisions since old ones will
+      // continue to work.
+      if (!GetDB().Execute("ALTER TABLE visits "
+          "ADD COLUMN visit_duration INTEGER DEFAULT 0 NOT NULL"))
+        return false;
+    }
   }
 
   // Visit source table contains the source information for all the visits. To
   // save space, we do not record those user browsed visits which would be the
   // majority in this table. Only other sources are recorded.
   // Due to the tight relationship between visit_source and visits table, they
   // should be created and dropped at the same time.
   if (!GetDB().DoesTableExist("visit_source")) {
     if (!GetDB().Execute("CREATE TABLE visit_source("
                          "id INTEGER PRIMARY KEY,source INTEGER NOT NULL)"))
@@ skipping 32 matching lines @@
 // Must be in sync with HISTORY_VISIT_ROW_FIELDS.
 // static
 void VisitDatabase::FillVisitRow(sql::Statement& statement, VisitRow* visit) {
   visit->visit_id = statement.ColumnInt64(0);
   visit->url_id = statement.ColumnInt64(1);
   visit->visit_time = base::Time::FromInternalValue(statement.ColumnInt64(2));
   visit->referring_visit = statement.ColumnInt64(3);
   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
 bool VisitDatabase::FillVisitVector(sql::Statement& statement,
                                     VisitVector* visits) {
   if (!statement.is_valid())
     return false;
 
   while (statement.Step()) {
     history::VisitRow visit;
     FillVisitRow(statement, &visit);
     visits->push_back(visit);
   }
 
   return statement.Succeeded();
 }
 
 VisitID VisitDatabase::AddVisit(VisitRow* visit, VisitSource source) {
   visit_analysis_->AddVisit(visit);
   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:  "
             << "url_id = " << visit->url_id;
     return 0;
   }
 
   visit->visit_id = GetDB().GetLastInsertRowId();
 
   if (source != SOURCE_BROWSED) {
     // Record the source of this visit when it is not browsed.
     sql::Statement statement1(GetDB().GetCachedStatement(SQL_FROM_HERE,
         "INSERT INTO visit_source (id, source) VALUES (?,?)"));
     statement1.BindInt64(0, visit->visit_id);
     statement1.BindInt64(1, source);
 
     if (!statement1.Run()) {
       VLOG(0) << "Failed to execute visit_source insert statement:  "
-              << "url_id = " << visit->visit_id;
+              << "id = " << visit->visit_id;
       return 0;
     }
   }
 
   return visit->visit_id;
 }
 
 void VisitDatabase::DeleteVisit(const VisitRow& visit) {
   // Patch around this visit. Any visits that this went to will now have their
   // "source" be the deleted visit's source.
@@ skipping 39 matching lines @@
 }
 
 bool VisitDatabase::UpdateVisitRow(const VisitRow& visit) {
   // Don't store inconsistent data to the database.
   DCHECK_NE(visit.visit_id, visit.referring_visit);
   if (visit.visit_id == visit.referring_visit)
     return false;
 
   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();
 }
 
 bool VisitDatabase::GetVisitsForURL(URLID url_id, VisitVector* visits) {
   visits->clear();
 
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "SELECT" HISTORY_VISIT_ROW_FIELDS
       "FROM visits "
@@ skipping 299 matching lines @@
 
     // Get the source entries out of the query result.
     while (statement.Step()) {
       std::pair<VisitID, VisitSource> source_entry(statement.ColumnInt64(0),
           static_cast<VisitSource>(statement.ColumnInt(1)));
       sources->insert(source_entry);
     }
   }
 }
 
+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 DEFAULT 0 NOT NULL"))
+      return false;
+  }
+  return true;
+}
+
 }  // namespace history