chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc - Issue 23578026: Use SNAPSHOT sync mode for LocalSync

Unified Diff: chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc

Issue 23578026: Use SNAPSHOT sync mode for LocalSync (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: addressed comments Created 7 years, 3 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

« no previous file with comments | « chrome/browser/sync_file_system/local/local_file_sync_context.cc ('k') | chrome/browser/sync_file_system/local/local_file_sync_service.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/sync_file_system/local/local_file_sync_context_unittest.cc

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 f3c7611683192a48c912faf81923c3f5bd10096c..302adcd4583310704a40f17e5be8038c9c4713b1 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

@@ -26,6 +26,7 @@

#include "webkit/browser/fileapi/file_system_context.h"

#include "webkit/browser/fileapi/file_system_operation_runner.h"

#include "webkit/browser/fileapi/isolated_context.h"

+#include "webkit/common/blob/scoped_file.h"

#define FPL FILE_PATH_LITERAL

@@ -57,6 +58,8 @@ class LocalFileSyncContextTest : public testing::Test {

virtual void SetUp() OVERRIDE {

RegisterSyncableFileSystem();

+ ASSERT_TRUE(dir_.CreateUniqueTempDir());

io_thread_.reset(new base::Thread("Thread_IO"));

io_thread_->StartWithOptions(

base::Thread::Options(base::MessageLoop::TYPE_IO, 0));

@@ -77,8 +80,10 @@ class LocalFileSyncContextTest : public testing::Test {

void StartPrepareForSync(FileSystemContext* file_system_context,

const FileSystemURL& url,

+ LocalFileSyncContext::SyncMode sync_mode,

SyncFileMetadata* metadata,

- FileChangeList* changes) {

+ FileChangeList* changes,

+ webkit_blob::ScopedFile* snapshot) {

ASSERT_TRUE(changes != NULL);

ASSERT_FALSE(has_inflight_prepare_for_sync_);

status_ = SYNC_STATUS_UNKNOWN;

@@ -86,39 +91,49 @@ class LocalFileSyncContextTest : public testing::Test {

sync_context_->PrepareForSync(

file_system_context,

url,

- LocalFileSyncContext::SYNC_EXCLUSIVE,

+ sync_mode,

base::Bind(&LocalFileSyncContextTest::DidPrepareForSync,

- base::Unretained(this), metadata, changes));

+ base::Unretained(this), metadata, changes, snapshot));

}

SyncStatusCode PrepareForSync(FileSystemContext* file_system_context,

const FileSystemURL& url,

+ LocalFileSyncContext::SyncMode sync_mode,

SyncFileMetadata* metadata,

- FileChangeList* changes) {

- StartPrepareForSync(file_system_context, url, metadata, changes);

+ FileChangeList* changes,

+ webkit_blob::ScopedFile* snapshot) {

+ StartPrepareForSync(file_system_context, url, sync_mode,

+ metadata, changes, snapshot);

base::MessageLoop::current()->Run();

return status_;

}

- base::Closure GetPrepareForSyncClosure(FileSystemContext* file_system_context,

- const FileSystemURL& url,

- SyncFileMetadata* metadata,

- FileChangeList* changes) {

+ base::Closure GetPrepareForSyncClosure(

+ FileSystemContext* file_system_context,

+ const FileSystemURL& url,

+ LocalFileSyncContext::SyncMode sync_mode,

+ SyncFileMetadata* metadata,

+ FileChangeList* changes,

+ webkit_blob::ScopedFile* snapshot) {

return base::Bind(&LocalFileSyncContextTest::StartPrepareForSync,

base::Unretained(this),

base::Unretained(file_system_context),

- url, metadata, changes);

+ url, sync_mode, metadata, changes, snapshot);

}

void DidPrepareForSync(SyncFileMetadata* metadata_out,

FileChangeList* changes_out,

+ webkit_blob::ScopedFile* snapshot_out,

SyncStatusCode status,

- const LocalFileSyncInfo& sync_file_info) {

+ const LocalFileSyncInfo& sync_file_info,

+ scoped_ptr<webkit_blob::ScopedFile> snapshot) {

ASSERT_TRUE(ui_task_runner_->RunsTasksOnCurrentThread());

has_inflight_prepare_for_sync_ = false;

status_ = status;

*metadata_out = sync_file_info.metadata;

*changes_out = sync_file_info.changes;

+ if (snapshot_out && snapshot)

+ *snapshot_out = snapshot->Pass();

base::MessageLoop::current()->Quit();

}

@@ -134,7 +149,9 @@ class LocalFileSyncContextTest : public testing::Test {

SyncFileMetadata metadata;

FileChangeList changes;

EXPECT_EQ(SYNC_STATUS_OK,

- PrepareForSync(file_system_context, url, &metadata, &changes));

+ PrepareForSync(file_system_context, url,

+ LocalFileSyncContext::SYNC_EXCLUSIVE,

+ &metadata, &changes, NULL));

EXPECT_EQ(expected_file_type, metadata.file_type);

status_ = SYNC_STATUS_UNKNOWN;

@@ -209,7 +226,7 @@ class LocalFileSyncContextTest : public testing::Test {

file_task_runner_.get());

file_system.SetUp();

sync_context_ = new LocalFileSyncContext(

- ui_task_runner_.get(), io_task_runner_.get());

+ dir_.path(), ui_task_runner_.get(), io_task_runner_.get());

ASSERT_EQ(SYNC_STATUS_OK,

file_system.MaybeInitializeFileSystemContext(

sync_context_.get()));

@@ -222,7 +239,7 @@ class LocalFileSyncContextTest : public testing::Test {

FileChangeList changes;

EXPECT_EQ(SYNC_STATUS_OK,

PrepareForSync(file_system.file_system_context(), kFile,

- &metadata, &changes));

+ sync_mode, &metadata, &changes, NULL));

EXPECT_EQ(1U, changes.size());

EXPECT_TRUE(changes.list().back().IsFile());

EXPECT_TRUE(changes.list().back().IsAddOrUpdate());

@@ -252,8 +269,73 @@ class LocalFileSyncContextTest : public testing::Test {

file_system.TearDown();

}

+ void PrepareForSync_WriteDuringSync(

+ LocalFileSyncContext::SyncMode sync_mode) {

+ CannedSyncableFileSystem file_system(GURL(kOrigin1),

+ io_task_runner_.get(),

+ file_task_runner_.get());

+ file_system.SetUp();

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), 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;

+ webkit_blob::ScopedFile snapshot;

+ EXPECT_EQ(SYNC_STATUS_OK,

+ PrepareForSync(file_system.file_system_context(), kFile,

+ sync_mode, &metadata, &changes, &snapshot));

+ EXPECT_EQ(1U, changes.size());

+ EXPECT_TRUE(changes.list().back().IsFile());

+ EXPECT_TRUE(changes.list().back().IsAddOrUpdate());

+ EXPECT_EQ(sync_mode == LocalFileSyncContext::SYNC_SNAPSHOT,

+ !snapshot.path().empty());

+ // Tracker keeps 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());

+ StartModifyFileOnIOThread(&file_system, kFile);

+ if (sync_mode == LocalFileSyncContext::SYNC_SNAPSHOT) {

+ // Write should succeed.

+ EXPECT_EQ(base::PLATFORM_FILE_OK, WaitUntilModifyFileIsDone());

+ } else {

+ base::MessageLoop::current()->RunUntilIdle();

+ EXPECT_FALSE(async_modify_finished_);

+ }

+ SimulateFinishSync(file_system.file_system_context(), kFile,

+ SYNC_STATUS_OK);

+ EXPECT_EQ(base::PLATFORM_FILE_OK, WaitUntilModifyFileIsDone());

+ // Sync succeeded, but the other change that was made during or

+ // after sync is recorded.

+ file_system.GetChangesForURLInTracker(kFile, &changes);

+ 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_;

+ base::ScopedTempDir dir_;

// These need to remain until the very end.

scoped_ptr<base::Thread> io_thread_;

scoped_ptr<base::Thread> file_thread_;

earthdok 2013/09/17 12:39:26 It looks like there's a race condition between the

@@ -273,7 +355,8 @@ class LocalFileSyncContextTest : public testing::Test {

TEST_F(LocalFileSyncContextTest, ConstructAndDestruct) {

sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ new LocalFileSyncContext(

+ dir_.path(), ui_task_runner_.get(), io_task_runner_.get());

sync_context_->ShutdownOnUIThread();

}

@@ -283,8 +366,8 @@ TEST_F(LocalFileSyncContextTest, InitializeFileSystemContext) {

file_task_runner_.get());

file_system.SetUp();

- sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), ui_task_runner_.get(), io_task_runner_.get());

// Initializes file_system using |sync_context_|.

EXPECT_EQ(SYNC_STATUS_OK,

@@ -328,8 +411,8 @@ TEST_F(LocalFileSyncContextTest, MultipleFileSystemContexts) {

file_system1.SetUp();

file_system2.SetUp();

- sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), ui_task_runner_.get(), io_task_runner_.get());

// Initializes file_system1 and file_system2.

EXPECT_EQ(SYNC_STATUS_OK,

@@ -376,7 +459,8 @@ TEST_F(LocalFileSyncContextTest, MultipleFileSystemContexts) {

FileChangeList changes;

EXPECT_EQ(SYNC_STATUS_OK,

PrepareForSync(file_system1.file_system_context(), kURL1,

- &metadata, &changes));

+ LocalFileSyncContext::SYNC_EXCLUSIVE,

+ &metadata, &changes, NULL));

EXPECT_EQ(1U, changes.size());

EXPECT_TRUE(changes.list().back().IsFile());

EXPECT_TRUE(changes.list().back().IsAddOrUpdate());

@@ -386,7 +470,8 @@ TEST_F(LocalFileSyncContextTest, MultipleFileSystemContexts) {

changes.clear();

EXPECT_EQ(SYNC_STATUS_OK,

PrepareForSync(file_system2.file_system_context(), kURL2,

- &metadata, &changes));

+ LocalFileSyncContext::SYNC_EXCLUSIVE,

+ &metadata, &changes, NULL));

EXPECT_EQ(1U, changes.size());

EXPECT_FALSE(changes.list().back().IsFile());

EXPECT_TRUE(changes.list().back().IsAddOrUpdate());

@@ -420,6 +505,14 @@ TEST_F(LocalFileSyncContextTest, PrepareSync_SyncFailure_Snapshot) {

SYNC_STATUS_FAILED);

}

+TEST_F(LocalFileSyncContextTest, PrepareSync_WriteDuringSync_Exclusive) {

+ PrepareForSync_WriteDuringSync(LocalFileSyncContext::SYNC_EXCLUSIVE);

+TEST_F(LocalFileSyncContextTest, PrepareSync_WriteDuringSync_Snapshot) {

+ PrepareForSync_WriteDuringSync(LocalFileSyncContext::SYNC_SNAPSHOT);

// LocalFileSyncContextTest.PrepareSyncWhileWriting is flaky on android.

// http://crbug.com/239793

#if defined(OS_ANDROID)

@@ -432,8 +525,8 @@ TEST_F(LocalFileSyncContextTest, MAYBE_PrepareSyncWhileWriting) {

io_task_runner_.get(),

file_task_runner_.get());

file_system.SetUp();

- sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), ui_task_runner_.get(), io_task_runner_.get());

EXPECT_EQ(SYNC_STATUS_OK,

file_system.MaybeInitializeFileSystemContext(sync_context_.get()));

@@ -452,8 +545,9 @@ TEST_F(LocalFileSyncContextTest, MAYBE_PrepareSyncWhileWriting) {

metadata.file_type = SYNC_FILE_TYPE_UNKNOWN;

FileChangeList changes;

EXPECT_EQ(SYNC_STATUS_FILE_BUSY,

- PrepareForSync(file_system.file_system_context(),

- kURL1, &metadata, &changes));

+ PrepareForSync(file_system.file_system_context(), kURL1,

+ LocalFileSyncContext::SYNC_EXCLUSIVE,

+ &metadata, &changes, NULL));

EXPECT_EQ(SYNC_FILE_TYPE_FILE, metadata.file_type);

// Register PrepareForSync method to be invoked when kURL1 becomes

@@ -462,8 +556,9 @@ TEST_F(LocalFileSyncContextTest, MAYBE_PrepareSyncWhileWriting) {

metadata.file_type = SYNC_FILE_TYPE_UNKNOWN;

changes.clear();

sync_context_->RegisterURLForWaitingSync(

- kURL1, GetPrepareForSyncClosure(file_system.file_system_context(),

- kURL1, &metadata, &changes));

+ kURL1, GetPrepareForSyncClosure(file_system.file_system_context(), kURL1,

+ LocalFileSyncContext::SYNC_EXCLUSIVE,

+ &metadata, &changes, NULL));

// Wait for the completion.

EXPECT_EQ(base::PLATFORM_FILE_OK, WaitUntilModifyFileIsDone());

@@ -492,8 +587,8 @@ TEST_F(LocalFileSyncContextTest, ApplyRemoteChangeForDeletion) {

file_task_runner_.get());

file_system.SetUp();

- sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), 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());

@@ -581,8 +676,8 @@ TEST_F(LocalFileSyncContextTest, ApplyRemoteChangeForAddOrUpdate) {

file_task_runner_.get());

file_system.SetUp();

- sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), 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());

@@ -730,8 +825,8 @@ TEST_F(LocalFileSyncContextTest, ApplyRemoteChangeForAddOrUpdate_NoParent) {

file_task_runner_.get());

file_system.SetUp();

- sync_context_ =

- new LocalFileSyncContext(ui_task_runner_.get(), io_task_runner_.get());

+ sync_context_ = new LocalFileSyncContext(

+ dir_.path(), 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());