Skip to content

Commit

Permalink
Move DatabaseTracker and DatabaseMessageFilter off of FILE thread
Browse files Browse the repository at this point in the history 
As part of the Great TaskScheduler Migration, move 'Database' (i.e.
WebSQL) operations in the browser off of the FILE thread to a
dedicated task runner.

Unit tests still override the task runner; that will be tackled in
a follow-up.

Bug: 739945
Change-Id: I7308c18fa6ff60686a3cffa62d35eb9719935477
Reviewed-on: https://chromium-review.googlesource.com/563649
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489043}
  • Loading branch information
@inexorabletash
inexorabletash authored and Commit Bot committed Jul 24, 2017
1 parent 5c626f8 commit 607cb14
Show file tree
Hide file tree
Showing 19 changed files with 376 additions and 311 deletions.
95 changes: 40 additions & 55 deletions chrome/browser/browsing_data/browsing_data_database_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,66 +49,52 @@ BrowsingDataDatabaseHelper::BrowsingDataDatabaseHelper(Profile* profile)
BrowsingDataDatabaseHelper::~BrowsingDataDatabaseHelper() {
}

void BrowsingDataDatabaseHelper::StartFetching(const FetchCallback& callback) {
void BrowsingDataDatabaseHelper::StartFetching(FetchCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(!callback.is_null());

BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE,
base::BindOnce(&BrowsingDataDatabaseHelper::FetchDatabaseInfoOnFileThread,
this, callback));
base::PostTaskAndReplyWithResult(
tracker_->task_runner(), FROM_HERE,
base::BindOnce(
[](storage::DatabaseTracker* tracker) {
std::list<DatabaseInfo> result;
std::vector<storage::OriginInfo> origins_info;
if (tracker->GetAllOriginsInfo(&origins_info)) {
for (const storage::OriginInfo& origin : origins_info) {
DatabaseIdentifier identifier =
DatabaseIdentifier::Parse(origin.GetOriginIdentifier());
// Non-websafe state is not considered browsing data.
if (!BrowsingDataHelper::HasWebScheme(identifier.ToOrigin()))
continue;
std::vector<base::string16> databases;
origin.GetAllDatabaseNames(&databases);
for (const base::string16& db : databases) {
base::FilePath file_path = tracker->GetFullDBFilePath(
origin.GetOriginIdentifier(), db);
base::File::Info file_info;
if (base::GetFileInfo(file_path, &file_info)) {
result.push_back(DatabaseInfo(
identifier, base::UTF16ToUTF8(db),
base::UTF16ToUTF8(origin.GetDatabaseDescription(db)),
file_info.size, file_info.last_modified));
}
}
}
}
return result;
},
base::RetainedRef(tracker_)),
std::move(callback));
}

void BrowsingDataDatabaseHelper::DeleteDatabase(const std::string& origin,
const std::string& name) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE,
base::BindOnce(&BrowsingDataDatabaseHelper::DeleteDatabaseOnFileThread,
this, origin, name));
}

void BrowsingDataDatabaseHelper::FetchDatabaseInfoOnFileThread(
const FetchCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::FILE);
DCHECK(!callback.is_null());

std::list<DatabaseInfo> result;
std::vector<storage::OriginInfo> origins_info;
if (tracker_.get() && tracker_->GetAllOriginsInfo(&origins_info)) {
for (const storage::OriginInfo& origin : origins_info) {
DatabaseIdentifier identifier =
DatabaseIdentifier::Parse(origin.GetOriginIdentifier());
if (!BrowsingDataHelper::HasWebScheme(identifier.ToOrigin()))
continue; // Non-websafe state is not considered browsing data.
std::vector<base::string16> databases;
origin.GetAllDatabaseNames(&databases);
for (const base::string16& db : databases) {
base::FilePath file_path =
tracker_->GetFullDBFilePath(origin.GetOriginIdentifier(), db);
base::File::Info file_info;
if (base::GetFileInfo(file_path, &file_info)) {
result.push_back(
DatabaseInfo(identifier, base::UTF16ToUTF8(db),
base::UTF16ToUTF8(origin.GetDatabaseDescription(db)),
file_info.size, file_info.last_modified));
}
}
}
}

BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
base::BindOnce(callback, result));
}

void BrowsingDataDatabaseHelper::DeleteDatabaseOnFileThread(
const std::string& origin,
const std::string& name) {
DCHECK_CURRENTLY_ON(BrowserThread::FILE);
if (!tracker_.get())
return;
tracker_->DeleteDatabase(origin, base::UTF8ToUTF16(name),
net::CompletionCallback());
tracker_->task_runner()->PostTask(
FROM_HERE, base::BindOnce(base::IgnoreResult(
&storage::DatabaseTracker::DeleteDatabase),
tracker_, origin, base::UTF8ToUTF16(name),
net::CompletionCallback()));
}

CannedBrowsingDataDatabaseHelper::PendingDatabaseInfo::PendingDatabaseInfo(
Expand Down Expand Up @@ -162,8 +148,7 @@ CannedBrowsingDataDatabaseHelper::GetPendingDatabaseInfo() {
return pending_database_info_;
}

void CannedBrowsingDataDatabaseHelper::StartFetching(
const FetchCallback& callback) {
void CannedBrowsingDataDatabaseHelper::StartFetching(FetchCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(!callback.is_null());

Expand All @@ -177,7 +162,7 @@ void CannedBrowsingDataDatabaseHelper::StartFetching(
}

BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
base::BindOnce(callback, result));
base::BindOnce(std::move(callback), result));
}

void CannedBrowsingDataDatabaseHelper::DeleteDatabase(
Expand Down
18 changes: 6 additions & 12 deletions chrome/browser/browsing_data/browsing_data_database_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,18 @@ class BrowsingDataDatabaseHelper
base::Time last_modified;
};

using FetchCallback = base::Callback<void(const std::list<DatabaseInfo>&)>;
using FetchCallback =
base::OnceCallback<void(const std::list<DatabaseInfo>&)>;

explicit BrowsingDataDatabaseHelper(Profile* profile);

// Starts the fetching process, which will notify its completion via
// callback.
// This must be called only in the UI thread.
virtual void StartFetching(const FetchCallback& callback);
virtual void StartFetching(FetchCallback callback);

// Requests a single database to be deleted in the FILE thread. This must be
// called in the UI thread.
// Requests a single database to be deleted. This must be called in the UI
// thread.
virtual void DeleteDatabase(const std::string& origin_identifier,
const std::string& name);

Expand All @@ -66,13 +67,6 @@ class BrowsingDataDatabaseHelper
virtual ~BrowsingDataDatabaseHelper();

private:
// Enumerates all databases. This must be called in the FILE thread.
void FetchDatabaseInfoOnFileThread(const FetchCallback& callback);

// Delete a single database file. This must be called in the FILE thread.
void DeleteDatabaseOnFileThread(const std::string& origin,
const std::string& name);

scoped_refptr<storage::DatabaseTracker> tracker_;

DISALLOW_COPY_AND_ASSIGN(BrowsingDataDatabaseHelper);
Expand Down Expand Up @@ -118,7 +112,7 @@ class CannedBrowsingDataDatabaseHelper : public BrowsingDataDatabaseHelper {
const std::set<PendingDatabaseInfo>& GetPendingDatabaseInfo();

// BrowsingDataDatabaseHelper implementation.
void StartFetching(const FetchCallback& callback) override;
void StartFetching(FetchCallback callback) override;
void DeleteDatabase(const std::string& origin_identifier,
const std::string& name) override;

Expand Down
10 changes: 3 additions & 7 deletions chrome/browser/browsing_data/mock_browsing_data_database_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,13 @@ MockBrowsingDataDatabaseHelper::MockBrowsingDataDatabaseHelper(
MockBrowsingDataDatabaseHelper::~MockBrowsingDataDatabaseHelper() {
}

void MockBrowsingDataDatabaseHelper::StartFetching(
const FetchCallback& callback) {
ASSERT_FALSE(callback.is_null());
ASSERT_TRUE(callback_.is_null());
callback_ = callback;
void MockBrowsingDataDatabaseHelper::StartFetching(FetchCallback callback) {
callback_ = std::move(callback);
}

void MockBrowsingDataDatabaseHelper::DeleteDatabase(
const std::string& origin,
const std::string& name) {
ASSERT_FALSE(callback_.is_null());
std::string key = origin + ":" + name;
ASSERT_TRUE(base::ContainsKey(databases_, key));
last_deleted_origin_ = origin;
Expand All @@ -48,7 +44,7 @@ void MockBrowsingDataDatabaseHelper::AddDatabaseSamples() {
}

void MockBrowsingDataDatabaseHelper::Notify() {
callback_.Run(response_);
std::move(callback_).Run(response_);
}

void MockBrowsingDataDatabaseHelper::Reset() {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/browsing_data/mock_browsing_data_database_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class MockBrowsingDataDatabaseHelper : public BrowsingDataDatabaseHelper {
public:
explicit MockBrowsingDataDatabaseHelper(Profile* profile);

void StartFetching(const FetchCallback& callback) override;
void StartFetching(FetchCallback callback) override;

void DeleteDatabase(const std::string& origin,
const std::string& name) override;
Expand Down
68 changes: 44 additions & 24 deletions chrome/browser/extensions/extension_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4602,6 +4602,24 @@ class ExtensionCookieCallback {
base::WeakPtrFactory<base::MessageLoop> weak_factory_;
};

namespace {
// Helper to create (open, close, verify) a WebSQL database.
// Must be run on the DatabaseTracker's task runner.
void CreateDatabase(storage::DatabaseTracker* db_tracker,
const std::string& origin_id) {
DCHECK(db_tracker->task_runner()->RunsTasksInCurrentSequence());
base::string16 db_name = base::UTF8ToUTF16("db");
base::string16 description = base::UTF8ToUTF16("db_description");
int64_t size;
db_tracker->DatabaseOpened(origin_id, db_name, description, 1, &size);
db_tracker->DatabaseClosed(origin_id, db_name);
std::vector<storage::OriginInfo> origins;
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(1U, origins.size());
EXPECT_EQ(origin_id, origins[0].GetOriginIdentifier());
}
} // namespace

// Verifies extension state is removed upon uninstall.
TEST_F(ExtensionServiceTest, ClearExtensionData) {
InitializeEmptyExtensionService();
Expand Down Expand Up @@ -4639,15 +4657,10 @@ TEST_F(ExtensionServiceTest, ClearExtensionData) {
storage::DatabaseTracker* db_tracker =
BrowserContext::GetDefaultStoragePartition(profile())
->GetDatabaseTracker();
base::string16 db_name = base::UTF8ToUTF16("db");
base::string16 description = base::UTF8ToUTF16("db_description");
int64_t size;
db_tracker->DatabaseOpened(origin_id, db_name, description, 1, &size);
db_tracker->DatabaseClosed(origin_id, db_name);
std::vector<storage::OriginInfo> origins;
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(1U, origins.size());
EXPECT_EQ(origin_id, origins[0].GetOriginIdentifier());
db_tracker->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&CreateDatabase, base::Unretained(db_tracker), origin_id));
content::RunAllBlockingPoolTasksUntilIdle();

// Create local storage. We only simulate this by creating the backing files.
// Note: This test depends on details of how the dom_storage library
Expand Down Expand Up @@ -4689,9 +4702,15 @@ TEST_F(ExtensionServiceTest, ClearExtensionData) {
EXPECT_EQ(0U, callback.list_.size());

// The database should have vanished as well.
origins.clear();
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(0U, origins.size());
db_tracker->task_runner()->PostTask(
FROM_HERE, base::BindOnce(
[](storage::DatabaseTracker* db_tracker) {
std::vector<storage::OriginInfo> origins;
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(0U, origins.size());
},
base::Unretained(db_tracker)));
content::RunAllBlockingPoolTasksUntilIdle();

// Check that the LSO file has been removed.
EXPECT_FALSE(base::PathExists(lso_file_path));
Expand Down Expand Up @@ -4760,15 +4779,10 @@ TEST_F(ExtensionServiceTest, ClearAppData) {
storage::DatabaseTracker* db_tracker =
BrowserContext::GetDefaultStoragePartition(profile())
->GetDatabaseTracker();
base::string16 db_name = base::UTF8ToUTF16("db");
base::string16 description = base::UTF8ToUTF16("db_description");
int64_t size;
db_tracker->DatabaseOpened(origin_id, db_name, description, 1, &size);
db_tracker->DatabaseClosed(origin_id, db_name);
std::vector<storage::OriginInfo> origins;
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(1U, origins.size());
EXPECT_EQ(origin_id, origins[0].GetOriginIdentifier());
db_tracker->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&CreateDatabase, base::Unretained(db_tracker), origin_id));
content::RunAllBlockingPoolTasksUntilIdle();

// Create local storage. We only simulate this by creating the backing files.
// Note: This test depends on details of how the dom_storage library
Expand Down Expand Up @@ -4822,9 +4836,15 @@ TEST_F(ExtensionServiceTest, ClearAppData) {
EXPECT_EQ(0U, callback.list_.size());

// The database should have vanished as well.
origins.clear();
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(0U, origins.size());
db_tracker->task_runner()->PostTask(
FROM_HERE, base::BindOnce(
[](storage::DatabaseTracker* db_tracker) {
std::vector<storage::OriginInfo> origins;
db_tracker->GetAllOriginsInfo(&origins);
EXPECT_EQ(0U, origins.size());
},
base::Unretained(db_tracker)));
content::RunAllBlockingPoolTasksUntilIdle();

// Check that the LSO file has been removed.
EXPECT_FALSE(base::PathExists(lso_file_path));
Expand Down
9 changes: 7 additions & 2 deletions content/browser/browser_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,16 @@ void BrowserContext::EnsureResourceContextInitialized(BrowserContext* context) {
}

void BrowserContext::SaveSessionState(BrowserContext* browser_context) {
GetDefaultStoragePartition(browser_context)->GetDatabaseTracker()->
SetForceKeepSessionState();
StoragePartition* storage_partition =
BrowserContext::GetDefaultStoragePartition(browser_context);

storage::DatabaseTracker* database_tracker =
storage_partition->GetDatabaseTracker();
database_tracker->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&storage::DatabaseTracker::SetForceKeepSessionState,
make_scoped_refptr(database_tracker)));

if (BrowserThread::IsMessageLoopValid(BrowserThread::IO)) {
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
Expand Down
Loading

0 comments on commit 607cb14

Please sign in to comment.