diff options
author | kinuko@chromium.org <kinuko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-13 23:52:08 +0000 |
---|---|---|
committer | kinuko@chromium.org <kinuko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-13 23:52:08 +0000 |
commit | 3b6107cf3d5377e793b84ff57ce82cda16ab5d44 (patch) | |
tree | 23cc4ef80e7535f158b944a80c76b82c347dbc38 /chrome/browser/sync_file_system | |
parent | a92325c89bb24f8605b6e10b537d3a27e1d89481 (diff) | |
download | chromium_src-3b6107cf3d5377e793b84ff57ce82cda16ab5d44.zip chromium_src-3b6107cf3d5377e793b84ff57ce82cda16ab5d44.tar.gz chromium_src-3b6107cf3d5377e793b84ff57ce82cda16ab5d44.tar.bz2 |
Add SNAPSHOT sync mode to LocalFileSyncContext::PrepareForSync
So that local-to-remote sync can use snapshot file without
locking the file for the entire sync period.
The actual snapshotting part is not implemented yet.
This one corresponds to the first item in this design note: https://docs.google.com/a/chromium.org/document/d/1-oIGAGakMq0YS_3dfG11YHjfYRgOvP7kK7JagP39qLM/view#heading=h.rs5x2f5z1e34
BUG=175693
TEST=LocalFileSyncContextTest.PrepareSyncWhileWriting
Review URL: https://chromiumcodereview.appspot.com/24106002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223177 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync_file_system')
8 files changed, 279 insertions, 34 deletions
diff --git a/chrome/browser/sync_file_system/local/canned_syncable_file_system.cc b/chrome/browser/sync_file_system/local/canned_syncable_file_system.cc index d1d4e6a..d17f159 100644 --- a/chrome/browser/sync_file_system/local/canned_syncable_file_system.cc +++ b/chrome/browser/sync_file_system/local/canned_syncable_file_system.cc @@ -15,6 +15,7 @@ #include "base/run_loop.h" #include "base/single_thread_task_runner.h" #include "base/task_runner_util.h" +#include "chrome/browser/sync_file_system/file_change.h" #include "chrome/browser/sync_file_system/local/local_file_change_tracker.h" #include "chrome/browser/sync_file_system/local/local_file_sync_context.h" #include "chrome/browser/sync_file_system/local/sync_file_system_backend.h" @@ -498,7 +499,7 @@ quota::QuotaStatusCode CannedSyncableFileSystem::GetUsageAndQuota( void CannedSyncableFileSystem::GetChangedURLsInTracker( FileSystemURLSet* urls) { - return RunOnThread( + RunOnThread( file_task_runner_.get(), FROM_HERE, base::Bind(&LocalFileChangeTracker::GetAllChangedURLs, @@ -508,7 +509,7 @@ void CannedSyncableFileSystem::GetChangedURLsInTracker( void CannedSyncableFileSystem::ClearChangeForURLInTracker( const FileSystemURL& url) { - return RunOnThread( + RunOnThread( file_task_runner_.get(), FROM_HERE, base::Bind(&LocalFileChangeTracker::ClearChangesForURL, @@ -516,6 +517,17 @@ void CannedSyncableFileSystem::ClearChangeForURLInTracker( url)); } +void CannedSyncableFileSystem::GetChangesForURLInTracker( + const FileSystemURL& url, + FileChangeList* changes) { + RunOnThread( + file_task_runner_.get(), + FROM_HERE, + base::Bind(&LocalFileChangeTracker::GetChangesForURL, + base::Unretained(backend()->change_tracker()), + url, changes)); +} + SyncFileSystemBackend* CannedSyncableFileSystem::backend() { return SyncFileSystemBackend::GetBackend(file_system_context_); } diff --git a/chrome/browser/sync_file_system/local/canned_syncable_file_system.h b/chrome/browser/sync_file_system/local/canned_syncable_file_system.h index 440e779..3851a5a 100644 --- a/chrome/browser/sync_file_system/local/canned_syncable_file_system.h +++ b/chrome/browser/sync_file_system/local/canned_syncable_file_system.h @@ -45,6 +45,7 @@ class QuotaManager; namespace sync_file_system { +class FileChangeList; class LocalFileSyncContext; class SyncFileSystemBackend; @@ -140,6 +141,8 @@ class CannedSyncableFileSystem // ChangeTracker related methods. They run on file task runner. void GetChangedURLsInTracker(fileapi::FileSystemURLSet* urls); void ClearChangeForURLInTracker(const fileapi::FileSystemURL& url); + void GetChangesForURLInTracker(const fileapi::FileSystemURL& url, + FileChangeList* changes); SyncFileSystemBackend* backend(); fileapi::FileSystemOperationRunner* operation_runner(); diff --git a/chrome/browser/sync_file_system/local/local_file_change_tracker.cc b/chrome/browser/sync_file_system/local/local_file_change_tracker.cc index 7cc1653..3b42634 100644 --- a/chrome/browser/sync_file_system/local/local_file_change_tracker.cc +++ b/chrome/browser/sync_file_system/local/local_file_change_tracker.cc @@ -150,9 +150,8 @@ void LocalFileChangeTracker::GetChangesForURL( void LocalFileChangeTracker::ClearChangesForURL(const FileSystemURL& url) { DCHECK(file_task_runner_->RunsTasksOnCurrentThread()); - // TODO(nhiroki): propagate the error code (see http://crbug.com/152127). ClearDirtyOnDatabase(url); - + mirror_changes_.erase(url); FileChangeMap::iterator found = changes_.find(url); if (found == changes_.end()) return; @@ -161,6 +160,39 @@ void LocalFileChangeTracker::ClearChangesForURL(const FileSystemURL& url) { UpdateNumChanges(); } +void LocalFileChangeTracker::CreateFreshMirrorForURL( + const fileapi::FileSystemURL& url) { + DCHECK(!ContainsKey(mirror_changes_, url)); + mirror_changes_[url] = ChangeInfo(); +} + +void LocalFileChangeTracker::RemoveMirrorAndCommitChangesForURL( + const fileapi::FileSystemURL& url) { + FileChangeMap::iterator found = mirror_changes_.find(url); + if (found == mirror_changes_.end()) + return; + mirror_changes_.erase(found); + + if (ContainsKey(changes_, url)) + MarkDirtyOnDatabase(url); + else + ClearDirtyOnDatabase(url); + UpdateNumChanges(); +} + +void LocalFileChangeTracker::ResetToMirrorAndCommitChangesForURL( + const fileapi::FileSystemURL& url) { + FileChangeMap::iterator found = mirror_changes_.find(url); + if (found == mirror_changes_.end() || found->second.change_list.empty()) { + ClearChangesForURL(url); + return; + } + const ChangeInfo& info = found->second; + change_seqs_[info.change_seq] = url; + changes_[url] = info; + RemoveMirrorAndCommitChangesForURL(url); +} + SyncStatusCode LocalFileChangeTracker::Initialize( FileSystemContext* file_system_context) { DCHECK(file_task_runner_->RunsTasksOnCurrentThread()); @@ -188,6 +220,7 @@ void LocalFileChangeTracker::GetAllChangedURLs(FileSystemURLSet* urls) { void LocalFileChangeTracker::DropAllChanges() { changes_.clear(); change_seqs_.clear(); + mirror_changes_.clear(); } SyncStatusCode LocalFileChangeTracker::MarkDirtyOnDatabase( @@ -278,18 +311,30 @@ SyncStatusCode LocalFileChangeTracker::CollectLastDirtyChanges( void LocalFileChangeTracker::RecordChange( const FileSystemURL& url, const FileChange& change) { DCHECK(file_task_runner_->RunsTasksOnCurrentThread()); - ChangeInfo& info = changes_[url]; - if (info.change_seq >= 0) - change_seqs_.erase(info.change_seq); + int change_seq = current_change_seq_++; + RecordChangeToChangeMaps(url, change, change_seq, &changes_, &change_seqs_); + if (ContainsKey(mirror_changes_, url)) + RecordChangeToChangeMaps(url, change, change_seq, &mirror_changes_, NULL); + UpdateNumChanges(); +} + +void LocalFileChangeTracker::RecordChangeToChangeMaps( + const FileSystemURL& url, + const FileChange& change, + int new_change_seq, + FileChangeMap* changes, + ChangeSeqMap* change_seqs) { + ChangeInfo& info = (*changes)[url]; + if (info.change_seq >= 0 && change_seqs) + change_seqs->erase(info.change_seq); info.change_list.Update(change); if (info.change_list.empty()) { - changes_.erase(url); - UpdateNumChanges(); + changes->erase(url); return; } - info.change_seq = current_change_seq_++; - change_seqs_[info.change_seq] = url; - UpdateNumChanges(); + info.change_seq = new_change_seq; + if (change_seqs) + (*change_seqs)[info.change_seq] = url; } // TrackerDB ------------------------------------------------------------------- diff --git a/chrome/browser/sync_file_system/local/local_file_change_tracker.h b/chrome/browser/sync_file_system/local/local_file_change_tracker.h index 916c9b2..93470d1 100644 --- a/chrome/browser/sync_file_system/local/local_file_change_tracker.h +++ b/chrome/browser/sync_file_system/local/local_file_change_tracker.h @@ -75,6 +75,17 @@ class LocalFileChangeTracker // Clears the pending changes recorded in this tracker for |url|. void ClearChangesForURL(const fileapi::FileSystemURL& url); + // Creates a fresh (empty) in-memory record for |url|. + // Note that new changes are recorded to the mirror too. + void CreateFreshMirrorForURL(const fileapi::FileSystemURL& url); + + // Removes a mirror for |url|, and commits the change status to database. + void RemoveMirrorAndCommitChangesForURL(const fileapi::FileSystemURL& url); + + // Resets the changes to the ones recorded in mirror for |url|, and + // commits the updated change status to database. + void ResetToMirrorAndCommitChangesForURL(const fileapi::FileSystemURL& url); + // Called by FileSyncService at the startup time to restore last dirty changes // left after the last shutdown (if any). SyncStatusCode Initialize(fileapi::FileSystemContext* file_system_context); @@ -123,6 +134,12 @@ class LocalFileChangeTracker void RecordChange(const fileapi::FileSystemURL& url, const FileChange& change); + static void RecordChangeToChangeMaps(const fileapi::FileSystemURL& url, + const FileChange& change, + int change_seq, + FileChangeMap* changes, + ChangeSeqMap* change_seqs); + bool initialized_; scoped_refptr<base::SequencedTaskRunner> file_task_runner_; @@ -130,6 +147,9 @@ class LocalFileChangeTracker FileChangeMap changes_; ChangeSeqMap change_seqs_; + // For mirrors. + FileChangeMap mirror_changes_; + scoped_ptr<TrackerDB> tracker_db_; // Change sequence number. Briefly gives a hint about the order of changes, diff --git a/chrome/browser/sync_file_system/local/local_file_sync_context.cc b/chrome/browser/sync_file_system/local/local_file_sync_context.cc index bfea72b..227ce74 100644 --- a/chrome/browser/sync_file_system/local/local_file_sync_context.cc +++ b/chrome/browser/sync_file_system/local/local_file_sync_context.cc @@ -129,17 +129,52 @@ void LocalFileSyncContext::ClearChangesForURL( ui_task_runner_->PostTask(FROM_HERE, done_callback); } +void LocalFileSyncContext::CommitChangeStatusForURL( + fileapi::FileSystemContext* file_system_context, + const fileapi::FileSystemURL& url, + SyncStatusCode sync_finish_status, + const base::Closure& done_callback) { + DCHECK(file_system_context); + if (!file_system_context->default_file_task_runner()-> + RunsTasksOnCurrentThread()) { + file_system_context->default_file_task_runner()->PostTask( + FROM_HERE, + base::Bind(&LocalFileSyncContext::CommitChangeStatusForURL, + this, make_scoped_refptr(file_system_context), + url, sync_finish_status, done_callback)); + return; + } + + SyncFileSystemBackend* backend = + SyncFileSystemBackend::GetBackend(file_system_context); + DCHECK(backend); + DCHECK(backend->change_tracker()); + + if (sync_finish_status == SYNC_STATUS_OK || + sync_finish_status == SYNC_STATUS_HAS_CONFLICT) { + // Commit the in-memory mirror change. + backend->change_tracker()->ResetToMirrorAndCommitChangesForURL(url); + } else { + // Abort in-memory mirror change. + backend->change_tracker()->RemoveMirrorAndCommitChangesForURL(url); + } + + // Call the completion callback on UI thread. + ui_task_runner_->PostTask(FROM_HERE, done_callback); +} + void LocalFileSyncContext::ClearSyncFlagForURL(const FileSystemURL& url) { // This is initially called on UI thread and to be relayed to IO thread. io_task_runner_->PostTask( FROM_HERE, base::Bind(&LocalFileSyncContext::EnableWritingOnIOThread, - this, url)); + this, url, true /* may_have_updates */)); } void LocalFileSyncContext::PrepareForSync( FileSystemContext* file_system_context, const FileSystemURL& url, + SyncMode sync_mode, const LocalFileSyncInfoCallback& callback) { // This is initially called on UI thread and to be relayed to IO thread. if (!io_task_runner_->RunsTasksOnCurrentThread()) { @@ -147,7 +182,8 @@ void LocalFileSyncContext::PrepareForSync( io_task_runner_->PostTask( FROM_HERE, base::Bind(&LocalFileSyncContext::PrepareForSync, this, - make_scoped_refptr(file_system_context), url, callback)); + make_scoped_refptr(file_system_context), url, + sync_mode, callback)); return; } DCHECK(io_task_runner_->RunsTasksOnCurrentThread()); @@ -161,7 +197,7 @@ void LocalFileSyncContext::PrepareForSync( this, make_scoped_refptr(file_system_context), syncable ? SYNC_STATUS_OK : SYNC_STATUS_FILE_BUSY, - url, callback)); + url, sync_mode, callback)); } void LocalFileSyncContext::RegisterURLForWaitingSync( @@ -614,8 +650,9 @@ void LocalFileSyncContext::TryPrepareForLocalSync( std::deque<FileSystemURL>* remaining = new std::deque<FileSystemURL>; remaining->swap(*urls); + // TODO(kinuko): Call PrepareForSync with SYNC_SNAPSHOT when it becomes ready. PrepareForSync( - file_system_context, url, + file_system_context, url, SYNC_EXCLUSIVE, base::Bind(&LocalFileSyncContext::DidTryPrepareForLocalSync, this, make_scoped_refptr(file_system_context), base::Owned(remaining), callback)); @@ -640,6 +677,7 @@ void LocalFileSyncContext::DidGetWritingStatusForSync( FileSystemContext* file_system_context, SyncStatusCode status, const FileSystemURL& url, + SyncMode sync_mode, const LocalFileSyncInfoCallback& callback) { // This gets called on UI thread and relays the task on FILE thread. DCHECK(file_system_context); @@ -654,7 +692,7 @@ void LocalFileSyncContext::DidGetWritingStatusForSync( FROM_HERE, base::Bind(&LocalFileSyncContext::DidGetWritingStatusForSync, this, make_scoped_refptr(file_system_context), - status, url, callback)); + status, url, sync_mode, callback)); return; } @@ -670,12 +708,16 @@ void LocalFileSyncContext::DidGetWritingStatusForSync( FileSystemFileUtil* file_util = file_system_context->sandbox_delegate()->sync_file_util(); DCHECK(file_util); + base::PlatformFileError file_error = file_util->GetFileInfo( make_scoped_ptr( new FileSystemOperationContext(file_system_context)).get(), url, &file_info, &platform_path); + if (file_error == base::PLATFORM_FILE_OK && sync_mode == SYNC_SNAPSHOT) { + // TODO(kinuko): creates a snapshot file. + } if (status == SYNC_STATUS_OK && file_error != base::PLATFORM_FILE_OK && file_error != base::PLATFORM_FILE_ERROR_NOT_FOUND) @@ -689,6 +731,7 @@ void LocalFileSyncContext::DidGetWritingStatusForSync( else if (file_info.is_directory) file_type = SYNC_FILE_TYPE_DIRECTORY; + // TODO(kinuko): returns the snapshot file path if snapshot is available. LocalFileSyncInfo sync_file_info; sync_file_info.url = url; sync_file_info.local_file_path = platform_path; @@ -697,18 +740,40 @@ void LocalFileSyncContext::DidGetWritingStatusForSync( sync_file_info.metadata.last_modified = file_info.last_modified; sync_file_info.changes = changes; + if (status == SYNC_STATUS_OK) { + if (!changes.empty()) { + // Now we create an empty mirror change record for URL (and we record + // changes to both mirror and original records during sync), so that + // we can reset to the mirror when the sync succeeds. + backend->change_tracker()->CreateFreshMirrorForURL(url); + } + + // 'Unlock' the file if sync_mode is not SYNC_EXCLUSIVE. + if (sync_mode != SYNC_EXCLUSIVE) { + io_task_runner_->PostTask( + FROM_HERE, + base::Bind(&LocalFileSyncContext::EnableWritingOnIOThread, + this, url, false /* may_have_updates */)); + } + } + ui_task_runner_->PostTask(FROM_HERE, base::Bind(callback, status, sync_file_info)); } void LocalFileSyncContext::EnableWritingOnIOThread( - const FileSystemURL& url) { + const FileSystemURL& url, + bool may_have_updates) { DCHECK(io_task_runner_->RunsTasksOnCurrentThread()); if (!sync_status()) { // The service might have been shut down. return; } sync_status()->EndSyncing(url); + + if (!may_have_updates) + return; + // Since a sync has finished the number of changes must have been updated. origins_with_pending_changes_.insert(url.origin()); ScheduleNotifyChangesUpdatedOnIOThread(); @@ -723,7 +788,7 @@ void LocalFileSyncContext::DidApplyRemoteChange( FROM_HERE, base::Bind(callback_on_ui, PlatformFileErrorToSyncStatusCode(file_error))); - EnableWritingOnIOThread(url); + EnableWritingOnIOThread(url, true /* may_have_updates */); } void LocalFileSyncContext::DidGetFileMetadata( diff --git a/chrome/browser/sync_file_system/local/local_file_sync_context.h b/chrome/browser/sync_file_system/local/local_file_sync_context.h index 778802d..7a4cfe7 100644 --- a/chrome/browser/sync_file_system/local/local_file_sync_context.h +++ b/chrome/browser/sync_file_system/local/local_file_sync_context.h @@ -50,6 +50,11 @@ class LocalFileSyncContext : public base::RefCountedThreadSafe<LocalFileSyncContext>, public LocalFileSyncStatus::Observer { public: + enum SyncMode { + SYNC_EXCLUSIVE, + SYNC_SNAPSHOT, + }; + typedef base::Callback<void( SyncStatusCode status, const LocalFileSyncInfo& sync_file_info)> LocalFileSyncInfoCallback; @@ -87,6 +92,16 @@ class LocalFileSyncContext const fileapi::FileSystemURL& url, const base::Closure& done_callback); + // Updates the on-disk dirty flag for |url| in the tracker DB. + // This will clear the dirty flag if |sync_finish_status| is SYNC_STATUS_OK + // or SYNC_STATUS_HAS_CONFLICT. + // |done_callback| is called when the changes are committed. + void CommitChangeStatusForURL( + fileapi::FileSystemContext* file_system_context, + const fileapi::FileSystemURL& url, + SyncStatusCode sync_finish_status, + const base::Closure& done_callback); + // A local or remote sync has been finished (either successfully or // with an error). Clears the internal sync flag and enable writing for |url|. // This method must be called on UI thread. @@ -95,11 +110,16 @@ class LocalFileSyncContext // Prepares for sync |url| by disabling writes on |url|. // If the target |url| is being written and cannot start sync it // returns SYNC_STATUS_WRITING status code via |callback|. - // Otherwise it disables writes, marks the |url| syncing and returns - // the current change set made on |url|. + // Otherwise returns the current change sets made on |url|. + // + // If |sync_mode| is SYNC_EXCLUSIVE this leaves the target file. + // If |sync_mode| is SYNC_SNAPSHOT this creates a snapshot (if the + // target file is not deleted) and unlocks the file before returning. + // // This method must be called on UI thread. void PrepareForSync(fileapi::FileSystemContext* file_system_context, const fileapi::FileSystemURL& url, + SyncMode sync_mode, const LocalFileSyncInfoCallback& callback); // Registers |url| to wait until sync is enabled for |url|. @@ -224,10 +244,12 @@ class LocalFileSyncContext fileapi::FileSystemContext* file_system_context, SyncStatusCode status, const fileapi::FileSystemURL& url, + SyncMode sync_mode, const LocalFileSyncInfoCallback& callback); // Helper routine for ClearSyncFlagForURL. - void EnableWritingOnIOThread(const fileapi::FileSystemURL& url); + void EnableWritingOnIOThread(const fileapi::FileSystemURL& url, + bool may_have_updates); void DidRemoveExistingEntryForApplyRemoteChange( fileapi::FileSystemContext* file_system_context, diff --git a/chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc b/chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc index 3fcb46e..f3c7611 100644 --- a/chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc +++ b/chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc @@ -7,6 +7,7 @@ #include <vector> #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/file_util.h" #include "base/files/file_path.h" #include "base/message_loop/message_loop.h" @@ -85,6 +86,7 @@ class LocalFileSyncContextTest : public testing::Test { sync_context_->PrepareForSync( file_system_context, url, + LocalFileSyncContext::SYNC_EXCLUSIVE, base::Bind(&LocalFileSyncContextTest::DidPrepareForSync, base::Unretained(this), metadata, changes)); } @@ -188,6 +190,68 @@ class LocalFileSyncContextTest : public testing::Test { async_modify_finished_ = true; } + void SimulateFinishSync(FileSystemContext* file_system_context, + const FileSystemURL& url, + SyncStatusCode status) { + sync_context_->CommitChangeStatusForURL( + file_system_context, + url, + status, + base::Bind(&LocalFileSyncContext::ClearSyncFlagForURL, + sync_context_, url)); + base::MessageLoop::current()->RunUntilIdle(); + } + + void PrepareForSync_Basic(LocalFileSyncContext::SyncMode sync_mode, + SyncStatusCode simulate_sync_finish_status) { + CannedSyncableFileSystem file_system(GURL(kOrigin1), + io_task_runner_.get(), + file_task_runner_.get()); + file_system.SetUp(); + sync_context_ = new LocalFileSyncContext( + ui_task_runner_.get(), io_task_runner_.get()); + ASSERT_EQ(SYNC_STATUS_OK, + file_system.MaybeInitializeFileSystemContext( + sync_context_.get())); + ASSERT_EQ(base::PLATFORM_FILE_OK, file_system.OpenFileSystem()); + + const FileSystemURL kFile(file_system.URL("file")); + EXPECT_EQ(base::PLATFORM_FILE_OK, file_system.CreateFile(kFile)); + + SyncFileMetadata metadata; + FileChangeList changes; + EXPECT_EQ(SYNC_STATUS_OK, + PrepareForSync(file_system.file_system_context(), kFile, + &metadata, &changes)); + EXPECT_EQ(1U, changes.size()); + EXPECT_TRUE(changes.list().back().IsFile()); + EXPECT_TRUE(changes.list().back().IsAddOrUpdate()); + + // We should see the same set of changes. + file_system.GetChangesForURLInTracker(kFile, &changes); + EXPECT_EQ(1U, changes.size()); + EXPECT_TRUE(changes.list().back().IsFile()); + EXPECT_TRUE(changes.list().back().IsAddOrUpdate()); + + SimulateFinishSync(file_system.file_system_context(), kFile, + simulate_sync_finish_status); + + file_system.GetChangesForURLInTracker(kFile, &changes); + if (simulate_sync_finish_status == SYNC_STATUS_OK) { + // The change's cleared. + EXPECT_TRUE(changes.empty()); + } else { + EXPECT_EQ(1U, changes.size()); + EXPECT_TRUE(changes.list().back().IsFile()); + EXPECT_TRUE(changes.list().back().IsAddOrUpdate()); + } + + sync_context_->ShutdownOnUIThread(); + sync_context_ = NULL; + + file_system.TearDown(); + } + ScopedEnableSyncFSDirectoryOperation enable_directory_operation_; // These need to remain until the very end. @@ -336,6 +400,26 @@ TEST_F(LocalFileSyncContextTest, MultipleFileSystemContexts) { file_system2.TearDown(); } +TEST_F(LocalFileSyncContextTest, PrepareSync_SyncSuccess_Exclusive) { + PrepareForSync_Basic(LocalFileSyncContext::SYNC_EXCLUSIVE, + SYNC_STATUS_OK); +} + +TEST_F(LocalFileSyncContextTest, PrepareSync_SyncSuccess_Snapshot) { + PrepareForSync_Basic(LocalFileSyncContext::SYNC_SNAPSHOT, + SYNC_STATUS_OK); +} + +TEST_F(LocalFileSyncContextTest, PrepareSync_SyncFailure_Exclusive) { + PrepareForSync_Basic(LocalFileSyncContext::SYNC_EXCLUSIVE, + SYNC_STATUS_FAILED); +} + +TEST_F(LocalFileSyncContextTest, PrepareSync_SyncFailure_Snapshot) { + PrepareForSync_Basic(LocalFileSyncContext::SYNC_SNAPSHOT, + SYNC_STATUS_FAILED); +} + // LocalFileSyncContextTest.PrepareSyncWhileWriting is flaky on android. // http://crbug.com/239793 #if defined(OS_ANDROID) diff --git a/chrome/browser/sync_file_system/local/local_file_sync_service.cc b/chrome/browser/sync_file_system/local/local_file_sync_service.cc index a6e5414..33fbb34 100644 --- a/chrome/browser/sync_file_system/local/local_file_sync_service.cc +++ b/chrome/browser/sync_file_system/local/local_file_sync_service.cc @@ -236,6 +236,7 @@ void LocalFileSyncService::PrepareForProcessRemoteChange( DCHECK(ContainsKey(origin_to_contexts_, url.origin())); sync_context_->PrepareForSync( origin_to_contexts_[url.origin()], url, + LocalFileSyncContext::SYNC_EXCLUSIVE, base::Bind(&PrepareForProcessRemoteChangeCallbackAdapter, callback)); } @@ -418,18 +419,11 @@ void LocalFileSyncService::ProcessNextChangeForURL( const FileSystemURL& url = sync_file_info.url; if (status != SYNC_STATUS_OK || changes.empty()) { - if (status == SYNC_STATUS_OK || status == SYNC_STATUS_HAS_CONFLICT) { - // Clear the recorded changes for the URL if the sync was successfull - // OR has failed due to conflict (so that we won't stick to the same - // conflicting file again and again). - DCHECK(ContainsKey(origin_to_contexts_, url.origin())); - sync_context_->ClearChangesForURL( - origin_to_contexts_[url.origin()], url, - base::Bind(&LocalFileSyncService::RunLocalSyncCallback, - AsWeakPtr(), status, url)); - return; - } - RunLocalSyncCallback(status, url); + DCHECK(ContainsKey(origin_to_contexts_, url.origin())); + sync_context_->CommitChangeStatusForURL( + origin_to_contexts_[url.origin()], url, status, + base::Bind(&LocalFileSyncService::RunLocalSyncCallback, + AsWeakPtr(), status, url)); return; } |