Skip to content

Commit

Permalink
Use SNAPSHOT sync mode for LocalSync
Browse files Browse the repository at this point in the history 
LocalFileSyncService keeps the returned snapshot around until
ApplyLocalChange finishes, so this change must be transparent
to the remote side.

Previous review URL: https://chromiumcodereview.appspot.com/23578026

BUG=175693, 294499
TEST=LocalFileSyncContextTest.PrepareSync_WriteDuringSync_*
TEST=DriveFileSyncServiceSyncTest.*
TEST=manual

Review URL: https://chromiumcodereview.appspot.com/24251003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224159 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
kinuko@chromium.org committed Sep 19, 2013
1 parent 7b9fcf4 commit bd79735
Show file tree
Hide file tree
Showing 14 changed files with 278 additions and 88 deletions.
15 changes: 3 additions & 12 deletions chrome/browser/sync_file_system/drive_backend_v1/drive_file_sync_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,6 @@ typedef RemoteFileSyncService::OriginStatusMap OriginStatusMap;
namespace {

const base::FilePath::CharType kTempDirName[] = FILE_PATH_LITERAL("tmp");
const base::FilePath::CharType kSyncFileSystemDir[] =
FILE_PATH_LITERAL("Sync FileSystem");
const base::FilePath::CharType kSyncFileSystemDirDev[] =
FILE_PATH_LITERAL("Sync FileSystem Dev");

const base::FilePath::CharType* GetSyncFileSystemDir() {
return IsSyncFSDirectoryOperationEnabled()
? kSyncFileSystemDirDev : kSyncFileSystemDir;
}

void EmptyStatusCallback(SyncStatusCode status) {}

Expand Down Expand Up @@ -351,14 +342,14 @@ void DriveFileSyncService::Initialize(

task_manager_ = task_manager.Pass();

temporary_file_dir_ =
profile_->GetPath().Append(GetSyncFileSystemDir()).Append(kTempDirName);
temporary_file_dir_ = sync_file_system::GetSyncFileSystemDir(
profile_->GetPath()).Append(kTempDirName);

api_util_.reset(new drive_backend::APIUtil(profile_, temporary_file_dir_));
api_util_->AddObserver(this);

metadata_store_.reset(new DriveMetadataStore(
profile_->GetPath().Append(GetSyncFileSystemDir()),
GetSyncFileSystemDir(profile_->GetPath()),
content::BrowserThread::GetMessageLoopProxyForThread(
content::BrowserThread::FILE).get()));

Expand Down
1 change: 1 addition & 0 deletions chrome/browser/sync_file_system/local/local_file_change_tracker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ class LocalFileChangeTracker
friend class CannedSyncableFileSystem;
friend class LocalFileChangeTrackerTest;
friend class LocalFileSyncContext;
friend class LocalFileSyncContextTest;
friend class SyncableFileSystemTest;

struct ChangeInfo {
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/sync_file_system/local/local_file_change_tracker_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class LocalFileChangeTrackerTest : public testing::Test {
file_system_.SetUp();

sync_context_ =
new LocalFileSyncContext(base::MessageLoopProxy::current().get(),
new LocalFileSyncContext(base::FilePath(),
base::MessageLoopProxy::current().get(),
base::MessageLoopProxy::current().get());
ASSERT_EQ(
sync_file_system::SYNC_STATUS_OK,
Expand Down
85 changes: 65 additions & 20 deletions chrome/browser/sync_file_system/local/local_file_sync_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "chrome/browser/sync_file_system/local/local_file_sync_context.h"

#include "base/bind.h"
#include "base/file_util.h"
#include "base/location.h"
#include "base/platform_file.h"
#include "base/single_thread_task_runner.h"
Expand All @@ -21,6 +22,7 @@
#include "webkit/browser/fileapi/file_system_file_util.h"
#include "webkit/browser/fileapi/file_system_operation_context.h"
#include "webkit/browser/fileapi/file_system_operation_runner.h"
#include "webkit/common/blob/scoped_file.h"
#include "webkit/common/fileapi/file_system_util.h"

using fileapi::FileSystemContext;
Expand All @@ -37,12 +39,16 @@ const int kMaxConcurrentSyncableOperation = 3;
const int kNotifyChangesDurationInSec = 1;
const int kMaxURLsToFetchForLocalSync = 5;

const base::FilePath::CharType kSnapshotDir[] = FILE_PATH_LITERAL("snapshots");

} // namespace

LocalFileSyncContext::LocalFileSyncContext(
const base::FilePath& base_path,
base::SingleThreadTaskRunner* ui_task_runner,
base::SingleThreadTaskRunner* io_task_runner)
: ui_task_runner_(ui_task_runner),
: local_base_path_(base_path.Append(FILE_PATH_LITERAL("local"))),
ui_task_runner_(ui_task_runner),
io_task_runner_(io_task_runner),
shutdown_on_ui_(false),
mock_notify_changes_duration_in_sec_(-1) {
Expand Down Expand Up @@ -159,6 +165,12 @@ void LocalFileSyncContext::CommitChangeStatusForURL(
backend->change_tracker()->RemoveMirrorAndCommitChangesForURL(url);
}

// We've been keeping it in writing mode, so clear the writing counter
// to unblock sync activities.
io_task_runner_->PostTask(
FROM_HERE, base::Bind(&LocalFileSyncContext::EndWritingOnIOThread,
this, url));

// Call the completion callback on UI thread.
ui_task_runner_->PostTask(FROM_HERE, done_callback);
}
Expand All @@ -167,8 +179,8 @@ 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, true /* may_have_updates */));
base::Bind(&LocalFileSyncContext::ClearSyncFlagOnIOThread,
this, url, false /* keep_url_in_writing */));
}

void LocalFileSyncContext::PrepareForSync(
Expand Down Expand Up @@ -552,6 +564,10 @@ SyncStatusCode LocalFileSyncContext::InitializeChangeTrackerOnFileThread(
iter != urls.end(); ++iter) {
origins_with_changes->insert(iter->origin());
}

// Creates snapshot directory.
file_util::CreateDirectory(local_base_path_.Append(kSnapshotDir));

return status;
}

Expand Down Expand Up @@ -635,13 +651,14 @@ void LocalFileSyncContext::TryPrepareForLocalSync(
DCHECK(urls);

if (shutdown_on_ui_) {
callback.Run(SYNC_STATUS_ABORT, LocalFileSyncInfo());
callback.Run(SYNC_STATUS_ABORT, LocalFileSyncInfo(),
scoped_ptr<webkit_blob::ScopedFile>());
return;
}

if (urls->empty()) {
callback.Run(SYNC_STATUS_NO_CHANGE_TO_SYNC,
LocalFileSyncInfo());
callback.Run(SYNC_STATUS_NO_CHANGE_TO_SYNC, LocalFileSyncInfo(),
scoped_ptr<webkit_blob::ScopedFile>());
return;
}

Expand All @@ -650,9 +667,8 @@ 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, SYNC_EXCLUSIVE,
file_system_context, url, SYNC_SNAPSHOT,
base::Bind(&LocalFileSyncContext::DidTryPrepareForLocalSync,
this, make_scoped_refptr(file_system_context),
base::Owned(remaining), callback));
Expand All @@ -663,10 +679,11 @@ void LocalFileSyncContext::DidTryPrepareForLocalSync(
std::deque<FileSystemURL>* remaining_urls,
const LocalFileSyncInfoCallback& callback,
SyncStatusCode status,
const LocalFileSyncInfo& sync_file_info) {
const LocalFileSyncInfo& sync_file_info,
scoped_ptr<webkit_blob::ScopedFile> snapshot) {
DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
if (status != SYNC_STATUS_FILE_BUSY) {
callback.Run(status, sync_file_info);
callback.Run(status, sync_file_info, snapshot.Pass());
return;
}
// Recursively call TryPrepareForLocalSync with remaining_urls.
Expand All @@ -685,7 +702,8 @@ void LocalFileSyncContext::DidGetWritingStatusForSync(
RunsTasksOnCurrentThread()) {
DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
if (shutdown_on_ui_) {
callback.Run(SYNC_STATUS_ABORT, LocalFileSyncInfo());
callback.Run(SYNC_STATUS_ABORT, LocalFileSyncInfo(),
scoped_ptr<webkit_blob::ScopedFile>());
return;
}
file_system_context->default_file_task_runner()->PostTask(
Expand Down Expand Up @@ -715,9 +733,21 @@ void LocalFileSyncContext::DidGetWritingStatusForSync(
url,
&file_info,
&platform_path);

scoped_ptr<webkit_blob::ScopedFile> snapshot;
if (file_error == base::PLATFORM_FILE_OK && sync_mode == SYNC_SNAPSHOT) {
// TODO(kinuko): creates a snapshot file.
base::FilePath snapshot_path;
file_util::CreateTemporaryFileInDir(local_base_path_.Append(kSnapshotDir),
&snapshot_path);
if (base::CopyFile(platform_path, snapshot_path)) {
platform_path = snapshot_path;
snapshot.reset(new webkit_blob::ScopedFile(
snapshot_path,
webkit_blob::ScopedFile::DELETE_ON_SCOPE_OUT,
file_system_context->default_file_task_runner()));
}
}

if (status == SYNC_STATUS_OK &&
file_error != base::PLATFORM_FILE_OK &&
file_error != base::PLATFORM_FILE_ERROR_NOT_FOUND)
Expand All @@ -731,7 +761,6 @@ 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;
Expand All @@ -749,36 +778,52 @@ void LocalFileSyncContext::DidGetWritingStatusForSync(
}

// 'Unlock' the file if sync_mode is not SYNC_EXCLUSIVE.
// (But keep it in writing status so that no other sync starts on
// the same URL)
if (sync_mode != SYNC_EXCLUSIVE) {
io_task_runner_->PostTask(
FROM_HERE,
base::Bind(&LocalFileSyncContext::EnableWritingOnIOThread,
this, url, false /* may_have_updates */));
base::Bind(&LocalFileSyncContext::ClearSyncFlagOnIOThread,
this, url, true /* keep_url_in_writing */));
}
}

ui_task_runner_->PostTask(FROM_HERE,
base::Bind(callback, status, sync_file_info));
base::Bind(callback, status, sync_file_info,
base::Passed(&snapshot)));
}

void LocalFileSyncContext::EnableWritingOnIOThread(
void LocalFileSyncContext::ClearSyncFlagOnIOThread(
const FileSystemURL& url,
bool may_have_updates) {
bool keep_url_in_writing) {
DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
if (!sync_status()) {
// The service might have been shut down.
return;
}
sync_status()->EndSyncing(url);

if (!may_have_updates)
if (keep_url_in_writing) {
// The caller will hold shared lock on this one.
sync_status()->StartWriting(url);
return;
}

// Since a sync has finished the number of changes must have been updated.
origins_with_pending_changes_.insert(url.origin());
ScheduleNotifyChangesUpdatedOnIOThread();
}

void LocalFileSyncContext::EndWritingOnIOThread(
const FileSystemURL& url) {
DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
if (!sync_status()) {
// The service might have been shut down.
return;
}
sync_status()->EndWriting(url);
}

void LocalFileSyncContext::DidApplyRemoteChange(
const FileSystemURL& url,
const SyncStatusCallback& callback_on_ui,
Expand All @@ -788,7 +833,7 @@ void LocalFileSyncContext::DidApplyRemoteChange(
FROM_HERE,
base::Bind(callback_on_ui,
PlatformFileErrorToSyncStatusCode(file_error)));
EnableWritingOnIOThread(url, true /* may_have_updates */);
ClearSyncFlagOnIOThread(url, false /* keep_url_in_writing */);
}

void LocalFileSyncContext::DidGetFileMetadata(
Expand Down
29 changes: 23 additions & 6 deletions chrome/browser/sync_file_system/local/local_file_sync_context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class FileSystemContext;
class FileSystemURL;
}

namespace webkit_blob {
class ScopedFile;
}

namespace sync_file_system {

class FileChange;
Expand All @@ -56,14 +60,17 @@ class LocalFileSyncContext
};

typedef base::Callback<void(
SyncStatusCode status, const LocalFileSyncInfo& sync_file_info)>
SyncStatusCode status,
const LocalFileSyncInfo& sync_file_info,
scoped_ptr<webkit_blob::ScopedFile> snapshot)>
LocalFileSyncInfoCallback;

typedef base::Callback<void(SyncStatusCode status,
bool has_pending_changes)>
HasPendingLocalChangeCallback;

LocalFileSyncContext(base::SingleThreadTaskRunner* ui_task_runner,
LocalFileSyncContext(const base::FilePath& base_path,
base::SingleThreadTaskRunner* ui_task_runner,
base::SingleThreadTaskRunner* io_task_runner);

// Initializes |file_system_context| for syncable file operations
Expand Down Expand Up @@ -237,7 +244,8 @@ class LocalFileSyncContext
std::deque<fileapi::FileSystemURL>* remaining_urls,
const LocalFileSyncInfoCallback& callback,
SyncStatusCode status,
const LocalFileSyncInfo& sync_file_info);
const LocalFileSyncInfo& sync_file_info,
scoped_ptr<webkit_blob::ScopedFile> snapshot);

// Callback routine for PrepareForSync and GetFileForLocalSync.
void DidGetWritingStatusForSync(
Expand All @@ -247,9 +255,16 @@ class LocalFileSyncContext
SyncMode sync_mode,
const LocalFileSyncInfoCallback& callback);

// Helper routine for ClearSyncFlagForURL.
void EnableWritingOnIOThread(const fileapi::FileSystemURL& url,
bool may_have_updates);
// Helper routine for sync/writing flag handling, primarily for
// ClearSyncFlagForURL but is also called by other internal routines.
// If |keep_url_in_writing| is true this increments the writing counter
// for |url| (after clearing syncing flag), so that the caller can
// continue to hold a shared lock on the URL.
// (The counter must be manually decremented by EndWritingOnIOThread
// if that's the case)
void ClearSyncFlagOnIOThread(const fileapi::FileSystemURL& url,
bool keep_url_in_writing);
void EndWritingOnIOThread(const fileapi::FileSystemURL& url);

void DidRemoveExistingEntryForApplyRemoteChange(
fileapi::FileSystemContext* file_system_context,
Expand Down Expand Up @@ -279,6 +294,8 @@ class LocalFileSyncContext
const StatusCallback& callback,
base::PlatformFileError error);

const base::FilePath local_base_path_;

scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;

Expand Down
Loading

0 comments on commit bd79735

Please sign in to comment.