chrome/browser/net/sqlite_persistent_cookie_store.cc - Issue 12314015: Make SQLitePersistentCookieStore run on injected SequencedTaskRunners vs named BrowserThreads.

Unified Diff: chrome/browser/net/sqlite_persistent_cookie_store.cc

Issue 12314015: Make SQLitePersistentCookieStore run on injected SequencedTaskRunners vs named BrowserThreads. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Fix perf tests. Created 7 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « chrome/browser/net/sqlite_persistent_cookie_store.h ('k') | chrome/browser/net/sqlite_persistent_cookie_store_perftest.cc » ('j') | chrome/browser/net/sqlite_persistent_cookie_store_perftest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/net/sqlite_persistent_cookie_store.cc

diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.cc b/chrome/browser/net/sqlite_persistent_cookie_store.cc

index 09b37926555e1b02674042cd67244df0c3aa0538..9211e4103eadc6a0a8205e608f3290282dd72452 100644

--- a/chrome/browser/net/sqlite_persistent_cookie_store.cc

+++ b/chrome/browser/net/sqlite_persistent_cookie_store.cc

@@ -14,17 +14,18 @@

#include "base/callback.h"

#include "base/file_path.h"

#include "base/file_util.h"

+#include "base/location.h"

#include "base/logging.h"

#include "base/memory/ref_counted.h"

#include "base/memory/scoped_ptr.h"

#include "base/metrics/histogram.h"

+#include "base/sequenced_task_runner.h"

#include "base/string_util.h"

#include "base/synchronization/lock.h"

#include "base/threading/thread.h"

mmenke 2013/02/20 20:58:30 Don't think this is needed.

erikwright (departed) 2013/02/21 20:11:43 Done.

#include "base/time.h"

#include "chrome/browser/diagnostics/sqlite_diagnostics.h"

#include "chrome/browser/net/clear_on_exit_policy.h"

-#include "content/public/browser/browser_thread.h"

#include "googleurl/src/gurl.h"

#include "net/base/registry_controlled_domains/registry_controlled_domain.h"

#include "net/cookies/canonical_cookie.h"

@@ -35,36 +36,38 @@

#include "third_party/sqlite/sqlite3.h"

using base::Time;

-using content::BrowserThread;

// This class is designed to be shared between any calling threads and the

// database thread. It batches operations and commits them on a timer.

// SQLitePersistentCookieStore::Load is called to load all cookies. It

// delegates to Backend::Load, which posts a Backend::LoadAndNotifyOnDBThread

-// task to the DB thread. This task calls Backend::ChainLoadCookies(), which

-// repeatedly posts itself to the DB thread to load each eTLD+1's cookies in

-// separate tasks. When this is complete, Backend::CompleteLoadOnIOThread is

-// posted to the IO thread, which notifies the caller of

+// task to the background thread. This task calls Backend::ChainLoadCookies(),

mmenke 2013/02/20 20:58:30 SequencedTaskRunners aren't guaranteed to be threa

erikwright (departed) 2013/02/21 20:11:43 Done.

+// which repeatedly posts itself to the BG thread to load each eTLD+1's cookies

+// in separate tasks. When this is complete, Backend::CompleteLoadOnIOThread is

+// posted to the client thread, which notifies the caller of

// SQLitePersistentCookieStore::Load that the load is complete.

// If a priority load request is invoked via SQLitePersistentCookieStore::

// LoadCookiesForKey, it is delegated to Backend::LoadCookiesForKey, which posts

-// Backend::LoadKeyAndNotifyOnDBThread to the DB thread. That routine loads just

+// Backend::LoadKeyAndNotifyOnDBThread to the BG thread. That routine loads just

// that single domain key (eTLD+1)'s cookies, and posts a Backend::

-// CompleteLoadForKeyOnIOThread to the IO thread to notify the caller of

+// CompleteLoadForKeyOnIOThread to the client thread to notify the caller of

// SQLitePersistentCookieStore::LoadCookiesForKey that that load is complete.

// Subsequent to loading, mutations may be queued by any thread using

// AddCookie, UpdateCookieAccessTime, and DeleteCookie. These are flushed to

-// disk on the DB thread every 30 seconds, 512 operations, or call to Flush(),

+// disk on the BG thread every 30 seconds, 512 operations, or call to Flush(),

// whichever occurs first.

class SQLitePersistentCookieStore::Backend

: public base::RefCountedThreadSafe<SQLitePersistentCookieStore::Backend> {

public:

- Backend(const base::FilePath& path,

- bool restore_old_session_cookies,

- ClearOnExitPolicy* clear_on_exit_policy)

+ Backend(

+ const base::FilePath& path,

+ const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,

+ const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,

+ bool restore_old_session_cookies,

+ ClearOnExitPolicy* clear_on_exit_policy)

: path_(path),

db_(NULL),

num_pending_(0),

@@ -74,6 +77,8 @@ class SQLitePersistentCookieStore::Backend

restore_old_session_cookies_(restore_old_session_cookies),

clear_on_exit_policy_(clear_on_exit_policy),

num_cookies_read_(0),

+ client_task_runner_(client_task_runner),

+ background_task_runner_(background_task_runner),

num_priority_waiting_(0),

total_priority_requests_(0) {

}

@@ -216,6 +221,11 @@ class SQLitePersistentCookieStore::Backend

void KillDatabase();

void ScheduleKillDatabase();

+ void PostBackgroundTask(const tracked_objects::Location& origin,

+ const base::Closure& task);

+ void PostClientTask(const tracked_objects::Location& origin,

+ const base::Closure& task);

base::FilePath path_;

scoped_ptr<sql::Connection> db_;

sql::MetaTable meta_table_;

@@ -262,6 +272,9 @@ class SQLitePersistentCookieStore::Backend

// thread.

int num_cookies_read_;

+ scoped_refptr<base::SequencedTaskRunner> client_task_runner_;

+ scoped_refptr<base::SequencedTaskRunner> background_task_runner_;

// Guards the following metrics-related properties (only accessed when

// starting/completing priority loads or completing the total load).

base::Lock metrics_lock_;

@@ -374,10 +387,9 @@ void SQLitePersistentCookieStore::Backend::Load(

const LoadedCallback& loaded_callback) {

// This function should be called only once per instance.

DCHECK(!db_.get());

- BrowserThread::PostTask(

- BrowserThread::DB, FROM_HERE,

- base::Bind(&Backend::LoadAndNotifyOnDBThread, this, loaded_callback,

- base::Time::Now()));

+ PostBackgroundTask(FROM_HERE, base::Bind(

+ &Backend::LoadAndNotifyOnDBThread, this,

+ loaded_callback, base::Time::Now()));

}

void SQLitePersistentCookieStore::Backend::LoadCookiesForKey(

@@ -391,17 +403,14 @@ void SQLitePersistentCookieStore::Backend::LoadCookiesForKey(

total_priority_requests_++;

}

- BrowserThread::PostTask(

- BrowserThread::DB, FROM_HERE,

- base::Bind(&Backend::LoadKeyAndNotifyOnDBThread, this,

- key,

- loaded_callback,

- base::Time::Now()));

+ PostBackgroundTask(FROM_HERE, base::Bind(

+ &Backend::LoadKeyAndNotifyOnDBThread,

+ this, key, loaded_callback, base::Time::Now()));

}

void SQLitePersistentCookieStore::Backend::LoadAndNotifyOnDBThread(

const LoadedCallback& loaded_callback, const base::Time& posted_at) {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

IncrementTimeDelta increment(&cookie_load_duration_);

UMA_HISTOGRAM_CUSTOM_TIMES(

@@ -411,10 +420,8 @@ void SQLitePersistentCookieStore::Backend::LoadAndNotifyOnDBThread(

50);

if (!InitializeDatabase()) {

- BrowserThread::PostTask(

- BrowserThread::IO, FROM_HERE,

- base::Bind(&SQLitePersistentCookieStore::Backend::CompleteLoadOnIOThread,

- this, loaded_callback, false));

+ PostClientTask(FROM_HERE, base::Bind(

+ &Backend::CompleteLoadOnIOThread, this, loaded_callback, false));

} else {

ChainLoadCookies(loaded_callback);

}

@@ -424,7 +431,7 @@ void SQLitePersistentCookieStore::Backend::LoadKeyAndNotifyOnDBThread(

const std::string& key,

const LoadedCallback& loaded_callback,

const base::Time& posted_at) {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

IncrementTimeDelta increment(&cookie_load_duration_);

UMA_HISTOGRAM_CUSTOM_TIMES(

@@ -445,11 +452,9 @@ void SQLitePersistentCookieStore::Backend::LoadKeyAndNotifyOnDBThread(

}

- BrowserThread::PostTask(

- BrowserThread::IO, FROM_HERE,

- base::Bind(

- &SQLitePersistentCookieStore::Backend::CompleteLoadForKeyOnIOThread,

- this, loaded_callback, success));

+ PostClientTask(FROM_HERE, base::Bind(

+ &SQLitePersistentCookieStore::Backend::CompleteLoadForKeyOnIOThread,

+ this, loaded_callback, success));

}

void SQLitePersistentCookieStore::Backend::CompleteLoadForKeyOnIOThread(

@@ -477,7 +482,7 @@ void SQLitePersistentCookieStore::Backend::ReportMetricsOnDBThread() {

}

void SQLitePersistentCookieStore::Backend::ReportMetrics() {

- BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, base::Bind(

+ PostBackgroundTask(FROM_HERE, base::Bind(

&SQLitePersistentCookieStore::Backend::ReportMetricsOnDBThread, this));

{

@@ -509,7 +514,7 @@ void SQLitePersistentCookieStore::Backend::CompleteLoadOnIOThread(

void SQLitePersistentCookieStore::Backend::Notify(

const LoadedCallback& loaded_callback,

bool load_success) {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

+ DCHECK(client_task_runner_->RunsTasksOnCurrentThread());

std::vector<net::CanonicalCookie*> cookies;

{

@@ -521,7 +526,7 @@ void SQLitePersistentCookieStore::Backend::Notify(

}

bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

if (initialized_ || corruption_detected_) {

// Return false if we were previously initialized but the DB has since been

@@ -608,7 +613,7 @@ bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {

void SQLitePersistentCookieStore::Backend::ChainLoadCookies(

const LoadedCallback& loaded_callback) {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

IncrementTimeDelta increment(&cookie_load_duration_);

bool load_success = true;

@@ -628,14 +633,11 @@ void SQLitePersistentCookieStore::Backend::ChainLoadCookies(

// then post a DB task to continue chain-load;

// Otherwise notify on IO thread.

if (load_success && keys_to_load_.size() > 0) {

- BrowserThread::PostTask(

- BrowserThread::DB, FROM_HERE,

- base::Bind(&Backend::ChainLoadCookies, this, loaded_callback));

+ PostBackgroundTask(FROM_HERE, base::Bind(

+ &Backend::ChainLoadCookies, this, loaded_callback));

} else {

- BrowserThread::PostTask(

- BrowserThread::IO, FROM_HERE,

- base::Bind(&SQLitePersistentCookieStore::Backend::CompleteLoadOnIOThread,

- this, loaded_callback, load_success));

+ PostClientTask(FROM_HERE, base::Bind(

+ &Backend::CompleteLoadOnIOThread, this, loaded_callback, load_success));

if (load_success && !restore_old_session_cookies_)

DeleteSessionCookiesOnStartup();

}

@@ -643,7 +645,7 @@ void SQLitePersistentCookieStore::Backend::ChainLoadCookies(

bool SQLitePersistentCookieStore::Backend::LoadCookiesForDomains(

const std::set<std::string>& domains) {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

sql::Statement smt;

if (restore_old_session_cookies_) {

@@ -830,7 +832,7 @@ void SQLitePersistentCookieStore::Backend::BatchOperation(

static const int kCommitIntervalMs = 30 * 1000;

// Commit right away if we have more than 512 outstanding operations.

static const size_t kCommitAfterBatchSize = 512;

- DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(!background_task_runner_->RunsTasksOnCurrentThread());

// We do a full copy of the cookie here, and hopefully just here.

scoped_ptr<PendingOperation> po(new PendingOperation(op, cc));

@@ -844,20 +846,19 @@ void SQLitePersistentCookieStore::Backend::BatchOperation(

if (num_pending == 1) {

// We've gotten our first entry for this batch, fire off the timer.

- BrowserThread::PostDelayedTask(

- BrowserThread::DB, FROM_HERE,

- base::Bind(&Backend::Commit, this),

- base::TimeDelta::FromMilliseconds(kCommitIntervalMs));

+ if (!background_task_runner_->PostDelayedTask(

+ FROM_HERE, base::Bind(&Backend::Commit, this),

+ base::TimeDelta::FromMilliseconds(kCommitIntervalMs))) {

+ NOTREACHED() << "background_task_runner_ is not running.";

+ }

} else if (num_pending == kCommitAfterBatchSize) {

// We've reached a big enough batch, fire off a commit now.

- BrowserThread::PostTask(

- BrowserThread::DB, FROM_HERE,

- base::Bind(&Backend::Commit, this));

+ PostBackgroundTask(FROM_HERE, base::Bind(&Backend::Commit, this));

}

void SQLitePersistentCookieStore::Backend::Commit() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

PendingOperationsList ops;

{

@@ -947,14 +948,14 @@ void SQLitePersistentCookieStore::Backend::Commit() {

void SQLitePersistentCookieStore::Backend::Flush(

const base::Closure& callback) {

- DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::DB));

- BrowserThread::PostTask(

- BrowserThread::DB, FROM_HERE, base::Bind(&Backend::Commit, this));

+ DCHECK(!background_task_runner_->RunsTasksOnCurrentThread());

+ PostBackgroundTask(FROM_HERE, base::Bind(&Backend::Commit, this));

if (!callback.is_null()) {

// We want the completion task to run immediately after Commit() returns.

// Posting it from here means there is less chance of another task getting

// onto the message queue first, than if we posted it from Commit() itself.

- BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, callback);

+ PostBackgroundTask(FROM_HERE, callback);

}

@@ -962,18 +963,17 @@ void SQLitePersistentCookieStore::Backend::Flush(

// pending commit timer or Load operations holding references on us, but if/when

// this fires we will already have been cleaned up and it will be ignored.

void SQLitePersistentCookieStore::Backend::Close() {

- if (BrowserThread::CurrentlyOn(BrowserThread::DB)) {

+ if (background_task_runner_->RunsTasksOnCurrentThread()) {

InternalBackgroundClose();

} else {

// Must close the backend on the background thread.

- BrowserThread::PostTask(

- BrowserThread::DB, FROM_HERE,

- base::Bind(&Backend::InternalBackgroundClose, this));

+ PostBackgroundTask(FROM_HERE,

+ base::Bind(&Backend::InternalBackgroundClose, this));

}

void SQLitePersistentCookieStore::Backend::InternalBackgroundClose() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

// Commit any pending operations

Commit();

@@ -987,7 +987,7 @@ void SQLitePersistentCookieStore::Backend::InternalBackgroundClose() {

}

void SQLitePersistentCookieStore::Backend::DeleteSessionCookiesOnShutdown() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

if (!db_.get())

return;

@@ -1028,18 +1028,17 @@ void SQLitePersistentCookieStore::Backend::DeleteSessionCookiesOnShutdown() {

}

void SQLitePersistentCookieStore::Backend::ScheduleKillDatabase() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

corruption_detected_ = true;

// Don't just do the close/delete here, as we are being called by |db| and

// that seems dangerous.

- MessageLoop::current()->PostTask(

- FROM_HERE, base::Bind(&Backend::KillDatabase, this));

+ PostBackgroundTask(FROM_HERE, base::Bind(&Backend::KillDatabase, this));

}

void SQLitePersistentCookieStore::Backend::KillDatabase() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

if (db_.get()) {

// This Backend will now be in-memory only. In a future run we will recreate

@@ -1057,17 +1056,38 @@ void SQLitePersistentCookieStore::Backend::SetForceKeepSessionState() {

}

void SQLitePersistentCookieStore::Backend::DeleteSessionCookiesOnStartup() {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

+ DCHECK(background_task_runner_->RunsTasksOnCurrentThread());

if (!db_->Execute("DELETE FROM cookies WHERE persistent == 0"))

LOG(WARNING) << "Unable to delete session cookies.";

}

+void SQLitePersistentCookieStore::Backend::PostBackgroundTask(

+ const tracked_objects::Location& origin, const base::Closure& task) {

+ if (!background_task_runner_->PostTask(origin, task)) {

+ NOTREACHED() << "Failed to post task from " << origin.ToString()

+ << " to background_task_runner_.";

+ }

+void SQLitePersistentCookieStore::Backend::PostClientTask(

+ const tracked_objects::Location& origin, const base::Closure& task) {

+ if (!client_task_runner_->PostTask(origin, task)) {

+ NOTREACHED() << "Failed to post task from " << origin.ToString()

+ << " to client_task_runner_.";

+ }

SQLitePersistentCookieStore::SQLitePersistentCookieStore(

const base::FilePath& path,

+ const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,

+ const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,

bool restore_old_session_cookies,

ClearOnExitPolicy* clear_on_exit_policy)

- : backend_(

- new Backend(path, restore_old_session_cookies, clear_on_exit_policy)) {

+ : backend_(new Backend(path,

+ client_task_runner,

+ background_task_runner,

+ restore_old_session_cookies,

+ clear_on_exit_policy)) {

}

void SQLitePersistentCookieStore::Load(const LoadedCallback& loaded_callback) {