chrome/browser/sync_file_system/drive_metadata_store.cc - Issue 15410005: Deprecate DriveMetadataStore.batch_sync_origins

Unified Diff: chrome/browser/sync_file_system/drive_metadata_store.cc

Issue 15410005: Deprecate DriveMetadataStore.batch_sync_origins (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Windows test fix Created 7 years, 7 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

« chrome/browser/sync_file_system/drive_metadata_store.h ('K') | « chrome/browser/sync_file_system/drive_metadata_store.h ('k') | chrome/browser/sync_file_system/drive_metadata_store_unittest.cc » ('j') | chrome/browser/sync_file_system/drive_metadata_store_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/sync_file_system/drive_metadata_store.cc

diff --git a/chrome/browser/sync_file_system/drive_metadata_store.cc b/chrome/browser/sync_file_system/drive_metadata_store.cc

index 9c3519abf83b69c409e72c61dab586edc0a1a46b..670ac12e0f91f049f22f2560888a498e4f812f25 100644

--- a/chrome/browser/sync_file_system/drive_metadata_store.cc

+++ b/chrome/browser/sync_file_system/drive_metadata_store.cc

@@ -110,7 +110,6 @@ bool UpdateResourceIdMap(ResourceIdByOrigin* map,

class DriveMetadataDB {

public:

enum OriginSyncType {

- BATCH_SYNC_ORIGIN,

INCREMENTAL_SYNC_ORIGIN,

DISABLED_ORIGIN

};

@@ -151,11 +150,12 @@ class DriveMetadataDB {

const std::string& resource_id);

SyncStatusCode RemoveOrigin(const GURL& origin);

- SyncStatusCode GetOrigins(ResourceIdByOrigin* batch_sync_origins,

- ResourceIdByOrigin* incremental_sync_origins,

+ SyncStatusCode GetOrigins(ResourceIdByOrigin* incremental_sync_origins,

ResourceIdByOrigin* disabled_origins);

private:

+ friend class DriveMetadataStore;

bool CalledOnValidThread() const {

return task_runner_->RunsTasksOnCurrentThread();

}

@@ -172,7 +172,6 @@ struct DriveMetadataDBContents {

int64 largest_changestamp;

DriveMetadataStore::MetadataMap metadata_map;

std::string sync_root_directory_resource_id;

- ResourceIdByOrigin batch_sync_origins;

ResourceIdByOrigin incremental_sync_origins;

ResourceIdByOrigin disabled_origins;

};

@@ -186,7 +185,6 @@ SyncStatusCode InitializeDBOnFileThread(DriveMetadataDB* db,

contents->largest_changestamp = 0;

contents->metadata_map.clear();

- contents->batch_sync_origins.clear();

contents->incremental_sync_origins.clear();

contents->disabled_origins.clear();

@@ -213,8 +211,6 @@ std::string CreateKeyForOriginRoot(const GURL& origin,

DriveMetadataDB::OriginSyncType sync_type) {

DCHECK(origin.is_valid());

switch (sync_type) {

- case DriveMetadataDB::BATCH_SYNC_ORIGIN:

- return kDriveBatchSyncOriginKeyPrefix + origin.spec();

case DriveMetadataDB::INCREMENTAL_SYNC_ORIGIN:

return kDriveIncrementalSyncOriginKeyPrefix + origin.spec();

case DriveMetadataDB::DISABLED_ORIGIN:

@@ -294,13 +290,11 @@ void DriveMetadataStore::DidInitialize(const InitializationCallback& callback,

largest_changestamp_ = contents->largest_changestamp;

metadata_map_.swap(contents->metadata_map);

sync_root_directory_resource_id_ = contents->sync_root_directory_resource_id;

- batch_sync_origins_.swap(contents->batch_sync_origins);

incremental_sync_origins_.swap(contents->incremental_sync_origins);

disabled_origins_.swap(contents->disabled_origins);

// |largest_changestamp_| is set to 0 for a fresh empty database.

origin_by_resource_id_.clear();

- InsertReverseMap(batch_sync_origins_, &origin_by_resource_id_);

InsertReverseMap(incremental_sync_origins_, &origin_by_resource_id_);

InsertReverseMap(disabled_origins_, &origin_by_resource_id_);

@@ -341,31 +335,26 @@ void DriveMetadataStore::DidRestoreSyncRootDirectory(

void DriveMetadataStore::RestoreOrigins(

const SyncStatusCallback& callback) {

DCHECK(CalledOnValidThread());

- ResourceIdByOrigin* batch_sync_origins = new ResourceIdByOrigin;

ResourceIdByOrigin* incremental_sync_origins = new ResourceIdByOrigin;

ResourceIdByOrigin* disabled_origins = new ResourceIdByOrigin;

base::PostTaskAndReplyWithResult(

file_task_runner_, FROM_HERE,

base::Bind(&DriveMetadataDB::GetOrigins,

base::Unretained(db_.get()),

- batch_sync_origins,

incremental_sync_origins,

disabled_origins),

base::Bind(&DriveMetadataStore::DidRestoreOrigins,

AsWeakPtr(), callback,

- base::Owned(batch_sync_origins),

base::Owned(incremental_sync_origins),

base::Owned(disabled_origins)));

}

void DriveMetadataStore::DidRestoreOrigins(

const SyncStatusCallback& callback,

- ResourceIdByOrigin* batch_sync_origins,

ResourceIdByOrigin* incremental_sync_origins,

ResourceIdByOrigin* disabled_origins,

SyncStatusCode status) {

DCHECK(CalledOnValidThread());

- DCHECK(batch_sync_origins);

DCHECK(incremental_sync_origins);

DCHECK(disabled_origins);

@@ -375,18 +364,20 @@ void DriveMetadataStore::DidRestoreOrigins(

return;

}

- batch_sync_origins_.swap(*batch_sync_origins);

incremental_sync_origins_.swap(*incremental_sync_origins);

disabled_origins_.swap(*disabled_origins);

origin_by_resource_id_.clear();

- InsertReverseMap(batch_sync_origins_, &origin_by_resource_id_);

InsertReverseMap(incremental_sync_origins_, &origin_by_resource_id_);

InsertReverseMap(disabled_origins_, &origin_by_resource_id_);

callback.Run(status);

}

+leveldb::DB* DriveMetadataStore::level_db() {

+ return db_.get()->db_.get();

tzik 2013/05/27 09:23:54 former get() can be omitted?

calvinlo 2013/05/27 09:47:47 Done.

void DriveMetadataStore::SetLargestChangeStamp(

int64 largest_changestamp,

const SyncStatusCallback& callback) {

@@ -474,8 +465,20 @@ SyncStatusCode DriveMetadataStore::ReadEntry(const FileSystemURL& url,

void DriveMetadataStore::AddIncrementalSyncOrigin(

const GURL& origin,

const std::string& resource_id) {

- AddBatchSyncOrigin(origin, resource_id);

- MoveBatchSyncOriginToIncremental(origin);

+ DCHECK(CalledOnValidThread());

+ DCHECK(!IsIncrementalSyncOrigin(origin));

+ DCHECK(!IsOriginDisabled(origin));

+ DCHECK_EQ(SYNC_STATUS_OK, db_status_);

+ incremental_sync_origins_.insert(std::make_pair(origin, resource_id));

+ origin_by_resource_id_.insert(std::make_pair(resource_id, origin));

+ // Store a pair of |origin| and |resource_id| in the DB.

+ base::PostTaskAndReplyWithResult(

+ file_task_runner_, FROM_HERE,

+ base::Bind(&DriveMetadataDB::UpdateOriginAsIncrementalSync,

+ base::Unretained(db_.get()), origin, resource_id),

+ base::Bind(&DriveMetadataStore::UpdateDBStatus, AsWeakPtr()));

}

void DriveMetadataStore::SetSyncRootDirectory(const std::string& resource_id) {

@@ -498,10 +501,7 @@ void DriveMetadataStore::SetOriginRootDirectory(

DCHECK(IsKnownOrigin(origin));

DriveMetadataDB::OriginSyncType sync_type;

- if (UpdateResourceIdMap(&batch_sync_origins_, &origin_by_resource_id_,

- origin, resource_id)) {

- sync_type = DriveMetadataDB::BATCH_SYNC_ORIGIN;

- } else if (UpdateResourceIdMap(

+ if (UpdateResourceIdMap(

&incremental_sync_origins_, &origin_by_resource_id_,

origin, resource_id)) {

sync_type = DriveMetadataDB::INCREMENTAL_SYNC_ORIGIN;

@@ -520,14 +520,7 @@ void DriveMetadataStore::SetOriginRootDirectory(

bool DriveMetadataStore::IsKnownOrigin(const GURL& origin) const {

DCHECK(CalledOnValidThread());

- return IsBatchSyncOrigin(origin) ||

- IsIncrementalSyncOrigin(origin) ||

- IsOriginDisabled(origin);

-bool DriveMetadataStore::IsBatchSyncOrigin(const GURL& origin) const {

- DCHECK(CalledOnValidThread());

- return ContainsKey(batch_sync_origins_, origin);

+ return IsIncrementalSyncOrigin(origin) || IsOriginDisabled(origin);

}

bool DriveMetadataStore::IsIncrementalSyncOrigin(const GURL& origin) const {

@@ -574,13 +567,7 @@ void DriveMetadataStore::DisableOrigin(

std::string resource_id;

std::map<GURL, std::string>::iterator found =

- batch_sync_origins_.find(origin);

- if (found != batch_sync_origins_.end()) {

- resource_id = found->second;

- batch_sync_origins_.erase(found);

- }

- found = incremental_sync_origins_.find(origin);

+ incremental_sync_origins_.find(origin);

if (found != incremental_sync_origins_.end()) {

resource_id = found->second;

incremental_sync_origins_.erase(found);

@@ -609,8 +596,7 @@ void DriveMetadataStore::RemoveOrigin(

metadata_map_.erase(origin);

std::string resource_id;

- if (EraseIfExists(&batch_sync_origins_, origin, &resource_id) ||

- EraseIfExists(&incremental_sync_origins_, origin, &resource_id) ||

+ if (EraseIfExists(&incremental_sync_origins_, origin, &resource_id) ||

EraseIfExists(&disabled_origins_, origin, &resource_id)) {

origin_by_resource_id_.erase(resource_id);

}

@@ -629,48 +615,6 @@ void DriveMetadataStore::DidUpdateOrigin(

callback.Run(status);

}

-void DriveMetadataStore::AddBatchSyncOrigin(const GURL& origin,

- const std::string& resource_id) {

- DCHECK(CalledOnValidThread());

- DCHECK(!IsIncrementalSyncOrigin(origin));

- DCHECK(!IsOriginDisabled(origin));

- DCHECK_EQ(SYNC_STATUS_OK, db_status_);

- if (IsBatchSyncOrigin(origin))

- return;

- batch_sync_origins_.insert(std::make_pair(origin, resource_id));

- origin_by_resource_id_.insert(std::make_pair(resource_id, origin));

- // Store a pair of |origin| and |resource_id| in the DB.

- base::PostTaskAndReplyWithResult(

- file_task_runner_, FROM_HERE,

- base::Bind(&DriveMetadataDB::UpdateOriginAsBatchSync,

- base::Unretained(db_.get()), origin, resource_id),

- base::Bind(&DriveMetadataStore::UpdateDBStatus, AsWeakPtr()));

-void DriveMetadataStore::MoveBatchSyncOriginToIncremental(const GURL& origin) {

- DCHECK(CalledOnValidThread());

- DCHECK(IsBatchSyncOrigin(origin));

- DCHECK(!IsIncrementalSyncOrigin(origin));

- DCHECK(!IsOriginDisabled(origin));

- DCHECK_EQ(SYNC_STATUS_OK, db_status_);

- std::map<GURL, std::string>::iterator found =

- batch_sync_origins_.find(origin);

- incremental_sync_origins_.insert(std::make_pair(origin, found->second));

- // Store a pair of |origin| and |resource_id| in the DB.

- base::PostTaskAndReplyWithResult(

- file_task_runner_, FROM_HERE,

- base::Bind(&DriveMetadataDB::UpdateOriginAsIncrementalSync,

- base::Unretained(db_.get()), origin, found->second),

- base::Bind(&DriveMetadataStore::UpdateDBStatus, AsWeakPtr()));

- batch_sync_origins_.erase(found);

void DriveMetadataStore::UpdateDBStatus(SyncStatusCode status) {

DCHECK(CalledOnValidThread());

if (db_status_ != SYNC_STATUS_OK &&

@@ -748,10 +692,6 @@ std::string DriveMetadataStore::GetResourceIdForOrigin(

if (found != incremental_sync_origins_.end())

return found->second;

- found = batch_sync_origins_.find(origin);

- if (found != batch_sync_origins_.end())

- return found->second;

found = disabled_origins_.find(origin);

if (found != disabled_origins_.end())

return found->second;

@@ -763,10 +703,8 @@ void DriveMetadataStore::GetAllOrigins(std::vector<GURL>* origins) {

DCHECK(CalledOnValidThread());

DCHECK(origins);

origins->clear();

- origins->reserve(batch_sync_origins_.size() +

- incremental_sync_origins_.size() +

+ origins->reserve(incremental_sync_origins_.size() +

disabled_origins_.size());

- AddOriginsToVector(origins, batch_sync_origins_);

AddOriginsToVector(origins, incremental_sync_origins_);

AddOriginsToVector(origins, disabled_origins_);

}

@@ -827,7 +765,23 @@ SyncStatusCode DriveMetadataDB::Initialize(bool* created) {

}

db_.reset(db);

- return SYNC_STATUS_OK;

+ // Remove legacy batch sync origin entries that are no longer needed.

+ leveldb::WriteBatch batch;

+ scoped_ptr<leveldb::Iterator> batch_origin_itr(

+ db_->NewIterator(leveldb::ReadOptions()));

+ for (batch_origin_itr->SeekToFirst();

tzik 2013/05/27 09:23:54 Seek(kDriveBatchSyncOriginKeyPrefix)?

calvinlo 2013/05/27 09:47:47 Done.

+ batch_origin_itr->Valid();

+ batch_origin_itr->Next()) {

+ std::string key = batch_origin_itr->key().ToString();

+ if (key.find(kDriveBatchSyncOriginKeyPrefix) == std::string::npos)

tzik 2013/05/27 09:23:54 !StartsWithASCII?

+ continue;

tzik 2013/05/27 09:23:54 s/continue/break/? The contents are sorted by the

calvinlo 2013/05/27 09:47:47 Done.

+ batch.Delete(key);

+ }

+ status = db_->Write(leveldb::WriteOptions(), &batch);

+ return LevelDBStatusToSyncStatusCode(status);

}

SyncStatusCode DriveMetadataDB::ReadContents(

@@ -838,7 +792,6 @@ SyncStatusCode DriveMetadataDB::ReadContents(

contents->largest_changestamp = 0;

contents->metadata_map.clear();

- contents->batch_sync_origins.clear();

contents->incremental_sync_origins.clear();

scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));

@@ -863,8 +816,7 @@ SyncStatusCode DriveMetadataDB::ReadContents(

}

- SyncStatusCode status = GetOrigins(&contents->batch_sync_origins,

- &contents->incremental_sync_origins,

+ SyncStatusCode status = GetOrigins(&contents->incremental_sync_origins,

&contents->disabled_origins);

if (status != SYNC_STATUS_OK &&

status != SYNC_DATABASE_ERROR_NOT_FOUND)

@@ -1038,25 +990,12 @@ SyncStatusCode DriveMetadataDB::DeleteEntry(const FileSystemURL& url) {

return LevelDBStatusToSyncStatusCode(status);

}

-SyncStatusCode DriveMetadataDB::UpdateOriginAsBatchSync(

- const GURL& origin, const std::string& resource_id) {

- DCHECK(CalledOnValidThread());

- DCHECK(db_.get());

- leveldb::Status status = db_->Put(

- leveldb::WriteOptions(),

- CreateKeyForOriginRoot(origin, BATCH_SYNC_ORIGIN),

- resource_id);

- return LevelDBStatusToSyncStatusCode(status);

SyncStatusCode DriveMetadataDB::UpdateOriginAsIncrementalSync(

const GURL& origin, const std::string& resource_id) {

DCHECK(CalledOnValidThread());

DCHECK(db_.get());

leveldb::WriteBatch batch;

- batch.Delete(CreateKeyForOriginRoot(origin, BATCH_SYNC_ORIGIN));

batch.Delete(CreateKeyForOriginRoot(origin, DISABLED_ORIGIN));

batch.Put(CreateKeyForOriginRoot(origin, INCREMENTAL_SYNC_ORIGIN),

resource_id);

@@ -1070,10 +1009,11 @@ SyncStatusCode DriveMetadataDB::EnableOrigin(

DCHECK(CalledOnValidThread());

DCHECK(db_.get());

+ // No DB entry as enabled origins go back to

+ // DriveFileSyncService.pending_batch_sync_origins only.

leveldb::WriteBatch batch;

batch.Delete(CreateKeyForOriginRoot(origin, INCREMENTAL_SYNC_ORIGIN));

batch.Delete(CreateKeyForOriginRoot(origin, DISABLED_ORIGIN));

- batch.Put(CreateKeyForOriginRoot(origin, BATCH_SYNC_ORIGIN), resource_id);

leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch);

return LevelDBStatusToSyncStatusCode(status);

@@ -1085,7 +1025,6 @@ SyncStatusCode DriveMetadataDB::DisableOrigin(

DCHECK(db_.get());

leveldb::WriteBatch batch;

- batch.Delete(CreateKeyForOriginRoot(origin_to_disable, BATCH_SYNC_ORIGIN));

batch.Delete(CreateKeyForOriginRoot(origin_to_disable,

INCREMENTAL_SYNC_ORIGIN));

batch.Put(CreateKeyForOriginRoot(origin_to_disable, DISABLED_ORIGIN),

@@ -1114,7 +1053,6 @@ SyncStatusCode DriveMetadataDB::RemoveOrigin(const GURL& origin_to_remove) {

DCHECK(CalledOnValidThread());

leveldb::WriteBatch batch;

- batch.Delete(CreateKeyForOriginRoot(origin_to_remove, BATCH_SYNC_ORIGIN));

batch.Delete(CreateKeyForOriginRoot(origin_to_remove,

INCREMENTAL_SYNC_ORIGIN));

batch.Delete(CreateKeyForOriginRoot(origin_to_remove, DISABLED_ORIGIN));

@@ -1138,7 +1076,6 @@ SyncStatusCode DriveMetadataDB::RemoveOrigin(const GURL& origin_to_remove) {

}

SyncStatusCode DriveMetadataDB::GetOrigins(

- ResourceIdByOrigin* batch_sync_origins,

ResourceIdByOrigin* incremental_sync_origins,

ResourceIdByOrigin* disabled_origins) {

DCHECK(CalledOnValidThread());

@@ -1146,21 +1083,6 @@ SyncStatusCode DriveMetadataDB::GetOrigins(

scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));

- // Get batch sync origins from the DB.

- for (itr->Seek(kDriveBatchSyncOriginKeyPrefix);

- itr->Valid(); itr->Next()) {

- std::string key = itr->key().ToString();

- if (!StartsWithASCII(key, kDriveBatchSyncOriginKeyPrefix, true))

- break;

- GURL origin(std::string(

- key.begin() + arraysize(kDriveBatchSyncOriginKeyPrefix) - 1,

- key.end()));

- DCHECK(origin.is_valid());

- bool result = batch_sync_origins->insert(

- std::make_pair(origin, itr->value().ToString())).second;

- DCHECK(result);

- }

// Get incremental sync origins from the DB.

for (itr->Seek(kDriveIncrementalSyncOriginKeyPrefix);

itr->Valid(); itr->Next()) {