summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--components/precache/content/precache_manager.cc26
-rw-r--r--components/precache/content/precache_manager.h3
-rw-r--r--components/precache/core/precache_database.cc12
-rw-r--r--components/precache/core/precache_database.h17
-rw-r--r--components/precache/core/precache_database_unittest.cc5
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_;