Adding more histograms to ResourcePrefetchPredictor.

chrome/browser/predictors/predictor_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/predictors/predictor_database.h"
 
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ skipping 29 matching lines @@
 
   // Opens the database file from the profile path. Separated from the
   // constructor to ease construction/destruction of this object on one thread
   // but database access on the DB thread.
   void Initialize();
   void LogDatabaseStats();  //  DB Thread.
 
   // Cancels pending DB transactions. Should only be called on the UI thread.
   void SetCancelled();
 
+  Profile* const profile_;

[dominich, 2012/08/30 16:07:31]

caching the profile pointer is rarely the right thing to do.

[Shishir, 2012/08/30 17:56:42]

Doesn't this make sense for a profile keyed service though? The service should
be shut down before the profile is destroyed.

   FilePath db_path_;
   sql::Connection db_;
 
   // TODO(shishir): These tables may not need to be refcounted. Maybe move them
   // to using a WeakPtr instead.
   scoped_refptr<AutocompleteActionPredictorTable> autocomplete_table_;
   scoped_refptr<ResourcePrefetchPredictorTables> resource_prefetch_tables_;
 
   DISALLOW_COPY_AND_ASSIGN(PredictorDatabaseInternal);
 };
 
 
 PredictorDatabaseInternal::PredictorDatabaseInternal(Profile* profile)
-    : db_path_(profile->GetPath().Append(kPredictorDatabaseName)),
+    : profile_(profile),
+      db_path_(profile->GetPath().Append(kPredictorDatabaseName)),
       autocomplete_table_(new AutocompleteActionPredictorTable()),
       resource_prefetch_tables_(new ResourcePrefetchPredictorTables()) {
 }
 
 PredictorDatabaseInternal::~PredictorDatabaseInternal() {
 }
 
 void PredictorDatabaseInternal::Initialize() {
   CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
   db_.set_exclusive_locking();
@@ skipping 17 matching lines @@
 
 void PredictorDatabaseInternal::LogDatabaseStats() {
   CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
 
   int64 db_size;
   bool success = file_util::GetFileSize(db_path_, &db_size);
   DCHECK(success) << "Failed to get file size for " << db_path_.value();
   UMA_HISTOGRAM_MEMORY_KB("PredictorDatabase.DatabaseSizeKB",
                           static_cast<int>(db_size / 1024));
 
-  autocomplete_table_->LogDatabaseStats();
-  resource_prefetch_tables_->LogDatabaseStats();
+  autocomplete_table_->LogDatabaseStats(profile_);
+  resource_prefetch_tables_->LogDatabaseStats(profile_);

[dominich, 2012/08/30 16:07:31]

given you know what table you're calling this on, i don't see why it shouldn't
check for the feature to be enabled before calling it. It's certainly better
than caching and passing down the profile pointer (which might be null/shutting
down/etc)

[Shishir, 2012/08/30 17:56:42]

Done.

 }
 
 PredictorDatabase::PredictorDatabase(Profile* profile)
     : db_(new PredictorDatabaseInternal(profile)) {
   content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
       base::Bind(&PredictorDatabaseInternal::Initialize, db_));
 }
 
 PredictorDatabase::~PredictorDatabase() {
 }
@@ skipping 10 matching lines @@
 scoped_refptr<ResourcePrefetchPredictorTables>
     PredictorDatabase::resource_prefetch_tables() {
   return db_->resource_prefetch_tables_;
 }
 
 sql::Connection* PredictorDatabase::GetDatabase() {
   return &db_->db_;
 }
 
 }  // namespace predictors