diff options
-rw-r--r-- | components/precache/content/precache_manager.cc | 26 | ||||
-rw-r--r-- | components/precache/content/precache_manager.h | 3 | ||||
-rw-r--r-- | components/precache/core/precache_database.cc | 12 | ||||
-rw-r--r-- | components/precache/core/precache_database.h | 17 | ||||
-rw-r--r-- | components/precache/core/precache_database_unittest.cc | 5 |
5 files changed, 37 insertions, 26 deletions
diff --git a/components/precache/content/precache_manager.cc b/components/precache/content/precache_manager.cc index cf0baea..a815b16 100644 --- a/components/precache/content/precache_manager.cc +++ b/components/precache/content/precache_manager.cc @@ -59,10 +59,16 @@ PrecacheManager::PrecacheManager( BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, base::Bind(base::IgnoreResult(&PrecacheDatabase::Init), - precache_database_, db_path)); + base::Unretained(precache_database_.get()), db_path)); } -PrecacheManager::~PrecacheManager() {} +PrecacheManager::~PrecacheManager() { + // DeleteSoon posts a non-nestable task to the task runner, so any previously + // posted tasks that rely on an Unretained precache_database_ will finish + // before it is deleted. + BrowserThread::DeleteSoon(BrowserThread::DB, FROM_HERE, + precache_database_.release()); +} bool PrecacheManager::IsInExperimentGroup() const { // Verify IsPrecachingAllowed() before calling FieldTrialList::FindFullName(). @@ -121,7 +127,8 @@ void PrecacheManager::StartPrecaching( BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, base::Bind(&PrecacheDatabase::DeleteExpiredPrecacheHistory, - precache_database_, base::Time::Now())); + base::Unretained(precache_database_.get()), + base::Time::Now())); // Request NumTopHosts() top hosts. Note that PrecacheFetcher is further // bound by the value of PrecacheConfigurationSettings.top_sites_count, as @@ -209,8 +216,9 @@ void PrecacheManager::RecordStatsForFetchInternal( // by precaching. BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, - base::Bind(&PrecacheDatabase::RecordURLPrefetch, precache_database_, - url, latency, fetch_time, size, was_cached)); + base::Bind(&PrecacheDatabase::RecordURLPrefetch, + base::Unretained(precache_database_.get()), url, latency, + fetch_time, size, was_cached)); } else { bool is_connection_cellular = net::NetworkChangeNotifier::IsConnectionCellular( @@ -218,8 +226,9 @@ void PrecacheManager::RecordStatsForFetchInternal( BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, - base::Bind(&PrecacheDatabase::RecordURLNonPrefetch, precache_database_, - url, latency, fetch_time, size, was_cached, host_rank, + base::Bind(&PrecacheDatabase::RecordURLNonPrefetch, + base::Unretained(precache_database_.get()), url, latency, + fetch_time, size, was_cached, host_rank, is_connection_cellular)); } } @@ -230,7 +239,8 @@ void PrecacheManager::ClearHistory() { // base::SequencedTaskRunner for details. BrowserThread::PostNonNestableTask( BrowserThread::DB, FROM_HERE, - base::Bind(&PrecacheDatabase::ClearHistory, precache_database_)); + base::Bind(&PrecacheDatabase::ClearHistory, + base::Unretained(precache_database_.get()))); } void PrecacheManager::Shutdown() { diff --git a/components/precache/content/precache_manager.h b/components/precache/content/precache_manager.h index 2735f98..7136713 100644 --- a/components/precache/content/precache_manager.h +++ b/components/precache/content/precache_manager.h @@ -16,7 +16,6 @@ #include "base/callback.h" #include "base/compiler_specific.h" #include "base/macros.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "components/history/core/browser/history_types.h" @@ -158,7 +157,7 @@ class PrecacheManager : public KeyedService, // The PrecacheDatabase for tracking precache metrics. Should only be used on // the DB thread. - const scoped_refptr<PrecacheDatabase> precache_database_; + scoped_ptr<PrecacheDatabase> precache_database_; // Flag indicating whether or not precaching is currently in progress. bool is_precaching_; diff --git a/components/precache/core/precache_database.cc b/components/precache/core/precache_database.cc index 42b110e..8cf4dac 100644 --- a/components/precache/core/precache_database.cc +++ b/components/precache/core/precache_database.cc @@ -26,16 +26,16 @@ const int kPrecacheHistoryExpiryPeriodDays = 60; namespace precache { -PrecacheDatabase::PrecacheDatabase() : is_flush_posted_(false) { +PrecacheDatabase::PrecacheDatabase() + : is_flush_posted_(false), weak_factory_(this) { // A PrecacheDatabase can be constructed on any thread. thread_checker_.DetachFromThread(); } PrecacheDatabase::~PrecacheDatabase() { - // Since the PrecacheDatabase is refcounted, it will only be deleted if there - // are no references remaining to it, meaning that it is not in use. Thus, it - // is safe to delete it, regardless of what thread we are on. - thread_checker_.DetachFromThread(); + // The destructor must not run on the UI thread, as it may trigger IO + // operations via sql::Connection's destructor. + DCHECK(thread_checker_.CalledOnValidThread()); } bool PrecacheDatabase::Init(const base::FilePath& db_path) { @@ -254,7 +254,7 @@ void PrecacheDatabase::MaybePostFlush() { // transaction. base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::Bind(&PrecacheDatabase::PostedFlush, - scoped_refptr<PrecacheDatabase>(this)), + weak_factory_.GetWeakPtr()), base::TimeDelta::FromSeconds(1)); is_flush_posted_ = true; } diff --git a/components/precache/core/precache_database.h b/components/precache/core/precache_database.h index 7c74f47..0f567b8 100644 --- a/components/precache/core/precache_database.h +++ b/components/precache/core/precache_database.h @@ -13,8 +13,8 @@ #include "base/callback.h" #include "base/containers/hash_tables.h" #include "base/macros.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/threading/thread_checker.h" #include "components/precache/core/precache_url_table.h" @@ -31,14 +31,16 @@ class Connection; namespace precache { -// Class that tracks information related to precaching. This class can be -// constructed or destroyed on any threads, but all other methods must be called -// on the same thread (e.g. the DB thread). -class PrecacheDatabase : public base::RefCountedThreadSafe<PrecacheDatabase> { +// Class that tracks information related to precaching. This class may be +// constructed on any thread, but all calls to, and destruction of this class +// must be done on the the DB thread. +class PrecacheDatabase { public: // A PrecacheDatabase can be constructed on any thread. PrecacheDatabase(); + ~PrecacheDatabase(); + // Initializes the precache database, using the specified database file path. // Init must be called before any other methods. bool Init(const base::FilePath& db_path); @@ -70,11 +72,8 @@ class PrecacheDatabase : public base::RefCountedThreadSafe<PrecacheDatabase> { bool is_connection_cellular); private: - friend class base::RefCountedThreadSafe<PrecacheDatabase>; friend class PrecacheDatabaseTest; - ~PrecacheDatabase(); - bool IsDatabaseAccessible() const; // Flushes any buffered write operations. |buffered_writes_| will be empty @@ -113,6 +112,8 @@ class PrecacheDatabase : public base::RefCountedThreadSafe<PrecacheDatabase> { // or destructor are called on the same thread. base::ThreadChecker thread_checker_; + base::WeakPtrFactory<PrecacheDatabase> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(PrecacheDatabase); }; diff --git a/components/precache/core/precache_database_unittest.cc b/components/precache/core/precache_database_unittest.cc index 65a4dae..36e2584 100644 --- a/components/precache/core/precache_database_unittest.cc +++ b/components/precache/core/precache_database_unittest.cc @@ -11,6 +11,7 @@ #include "base/containers/hash_tables.h" #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" +#include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "base/metrics/histogram_base.h" #include "base/test/histogram_tester.h" @@ -50,7 +51,7 @@ class PrecacheDatabaseTest : public testing::Test { protected: void SetUp() override { - precache_database_ = new PrecacheDatabase(); + precache_database_.reset(new PrecacheDatabase()); ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir()); base::FilePath db_path = scoped_temp_dir_.path().Append( @@ -107,7 +108,7 @@ class PrecacheDatabaseTest : public testing::Test { // to be set properly. base::MessageLoopForUI loop_; - scoped_refptr<PrecacheDatabase> precache_database_; + scoped_ptr<PrecacheDatabase> precache_database_; base::HistogramTester histograms_; base::HistogramTester::CountsMap expected_histogram_counts_; |