Issue 99383004: [SyncFS] Refine network error handling

Issue 99383004: [SyncFS] Refine network error handling (Closed)

Created:
7 years ago by tzik

Modified:
7 years ago

Reviewers:
kinuko, nhiroki

CC:
chromium-reviews, kinuko+watch, tzik

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

[SyncFS] Refine network error handling BUG=240165 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=240549

Patch Set 1 #

Patch Set 2 : buildfix #

Created: 7 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+59 lines, -17 lines)			Patch
M	chrome/browser/sync_file_system/drive_backend/folder_creator.cc	View		2 chunks	+12 lines, -5 lines	0 comments	Download
M	chrome/browser/sync_file_system/drive_backend/list_changes_task.cc	View	1	2 chunks	+9 lines, -1 line	0 comments	Download
M	chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer.cc	View		1 chunk	+3 lines, -2 lines	0 comments	Download
M	chrome/browser/sync_file_system/drive_backend/remote_to_local_syncer.h	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/sync_file_system/drive_backend/remote_to_local_syncer.cc	View	1	3 chunks	+32 lines, -7 lines	0 comments	Download
M	chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc	View	1	2 chunks	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

kinuko

lgtm 2013/12/13 1:05 <tzik@chromium.org>: > Reviewers: kinuko, nhiroki, > > Message: > PTL > > ...

7 years ago (2013-12-13 01:16:07 UTC) #3

lgtm
2013/12/13 1:05 <tzik@chromium.org>:

> Reviewers: kinuko, nhiroki,
>
> Message:
> PTL
>
> Description:
> [SyncFS] Refine network error handing
>
> BUG=
>
> Please review this at https://codereview.chromium.org/99383004/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+57, -16 lines):
>   M chrome/browser/sync_file_system/drive_backend/folder_creator.cc
>   M chrome/browser/sync_file_system/drive_backend/list_changes_task.cc
>   M chrome/browser/sync_file_system/drive_backend/local_to_
> remote_syncer.cc
>   M chrome/browser/sync_file_system/drive_backend/remote_
> to_local_syncer.cc
>   M chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc
>
>
> Index: chrome/browser/sync_file_system/drive_backend/folder_creator.cc
> diff --git a/chrome/browser/sync_file_system/drive_backend/folder_creator.cc
> b/chrome/browser/sync_file_system/drive_backend/folder_creator.cc
> index b5beb1a8649a5c037f2a84e27bdcfa3cd73ca338..
> 42b90e3fe43e806d6b6378b5bebacd3fa8956543 100644
> --- a/chrome/browser/sync_file_system/drive_backend/folder_creator.cc
> +++ b/chrome/browser/sync_file_system/drive_backend/folder_creator.cc
> @@ -51,9 +51,9 @@ void FolderCreator::DidCreateFolder(
>      const FileIDCallback& callback,
>      google_apis::GDataErrorCode error,
>      scoped_ptr<google_apis::ResourceEntry> entry) {
> -  if (error != google_apis::HTTP_SUCCESS &&
> -      error != google_apis::HTTP_CREATED) {
> -    callback.Run(std::string(), GDataErrorCodeToSyncStatusCode(error));
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK) {
> +    callback.Run(std::string(), status);
>      return;
>    }
>
> @@ -69,8 +69,15 @@ void FolderCreator::DidListFolders(
>      ScopedVector<google_apis::ResourceEntry> candidates,
>      google_apis::GDataErrorCode error,
>      scoped_ptr<google_apis::ResourceList> resource_list) {
> -  if (error != google_apis::HTTP_SUCCESS) {
> -    callback.Run(std::string(), GDataErrorCodeToSyncStatusCode(error));
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK) {
> +    callback.Run(std::string(), status);
> +    return;
> +  }
> +
> +  if (!resource_list) {
> +    NOTREACHED();
> +    callback.Run(std::string(), SYNC_STATUS_FAILED);
>      return;
>    }
>
> Index: chrome/browser/sync_file_system/drive_backend/list_changes_task.cc
> diff --git
a/chrome/browser/sync_file_system/drive_backend/list_changes_task.cc
> b/chrome/browser/sync_file_system/drive_backend/list_changes_task.cc
> index 9a35a93cb345eafeae1f6223cba3351d0b7f77ff..
> 35917e3fc006ec98b99e684a5c0b35d6ce93da0d 100644
> --- a/chrome/browser/sync_file_system/drive_backend/list_changes_task.cc
> +++ b/chrome/browser/sync_file_system/drive_backend/list_changes_task.cc
> @@ -60,12 +60,19 @@ void ListChangesTask::DidListChanges(
>      const SyncStatusCallback& callback,
>      google_apis::GDataErrorCode error,
>      scoped_ptr<google_apis::ResourceList> resource_list) {
> -  if (error != google_apis::HTTP_SUCCESS) {
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK) {
>      util::Log(logging::LOG_ERROR, FROM_HERE, "Failed to fetch change
> list.");
>      callback.Run(SYNC_STATUS_NETWORK_ERROR);
>      return;
>    }
>
> +  if (!resource_list) {
> +    NOTREACHED();
> +    callback.Run(SYNC_STATUS_FAILED);
> +    return;
> +  }
> +
>    change_list_.reserve(change_list_.size() +
> resource_list->entries().size());
>    for (size_t i = 0; i < resource_list->entries().size(); ++i) {
>      change_list_.push_back(ConvertResourceEntryToChangeResource(
> Index: chrome/browser/sync_file_system/drive_backend/local_to_
> remote_syncer.cc
> diff --git
a/chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer.cc
> b/chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer.cc
> index 04949d3ebc273527b8c0f8abddac3c936be9420a..
> 5c731dc1c3fc69b721c662f53e665c5294455738 100644
> --- a/chrome/browser/sync_file_system/drive_backend/local_to_
> remote_syncer.cc
> +++ b/chrome/browser/sync_file_system/drive_backend/local_to_
> remote_syncer.cc
> @@ -303,11 +303,12 @@ void LocalToRemoteSyncer::DeleteRemoteFile(
>  void LocalToRemoteSyncer::DidDeleteRemoteFile(
>      const SyncStatusCallback& callback,
>      google_apis::GDataErrorCode error) {
> -  if (error != google_apis::HTTP_SUCCESS &&
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK &&
>        error != google_apis::HTTP_NOT_FOUND &&
>        error != google_apis::HTTP_PRECONDITION &&
>        error != google_apis::HTTP_CONFLICT) {
> -    callback.Run(GDataErrorCodeToSyncStatusCode(error));
> +    callback.Run(status);
>      return;
>    }
>
> Index: chrome/browser/sync_file_system/drive_backend/remote_
> to_local_syncer.cc
> diff --git
a/chrome/browser/sync_file_system/drive_backend/remote_to_local_syncer.cc
> b/chrome/browser/sync_file_system/drive_backend/remote_to_local_syncer.cc
> index f5e14dd6641a273f7d0f50e162f20bfaffc320ad..
> a4194cc5342ec2e5d899bf274a798ce9ec7d0c28 100644
> --- a/chrome/browser/sync_file_system/drive_backend/remote_
> to_local_syncer.cc
> +++ b/chrome/browser/sync_file_system/drive_backend/remote_
> to_local_syncer.cc
> @@ -280,15 +280,32 @@ void RemoteToLocalSyncer::
> HandleMissingRemoteMetadata(
>        dirty_tracker_->file_id(),
>        base::Bind(&RemoteToLocalSyncer::DidGetRemoteMetadata,
>                   weak_ptr_factory_.GetWeakPtr(),
> -                 callback,
> -                 metadata_database()->GetLargestKnownChangeID()));
> +                 callback));
>  }
>
>  void RemoteToLocalSyncer::DidGetRemoteMetadata(
>      const SyncStatusCallback& callback,
> -    int64 change_id,
>      google_apis::GDataErrorCode error,
>      scoped_ptr<google_apis::ResourceEntry> entry) {
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK &&
> +      error != google_apis::HTTP_NOT_FOUND) {
> +    callback.Run(status);
> +    return;
> +  }
> +
> +  if (error == google_apis::HTTP_NOT_FOUND) {
> +    metadata_database()->UpdateByDeletedFile(
> +        dirty_tracker_->file_id(), callback);
> +    return;
> +  }
> +
> +  if (!entry) {
> +    NOTREACHED();
> +    callback.Run(SYNC_STATUS_FAILED);
> +    return;
> +  }
> +
>    metadata_database()->UpdateByFileResource(
>        *drive::util::ConvertResourceEntryToFileResource(*entry),
>        base::Bind(&RemoteToLocalSyncer::DidUpdateDatabaseForRemoteMeta
> data,
> @@ -518,8 +535,15 @@ void RemoteToLocalSyncer::DidListFolderContent(
>      scoped_ptr<FileIDList> children,
>      google_apis::GDataErrorCode error,
>      scoped_ptr<google_apis::ResourceList> resource_list) {
> -  if (error != google_apis::HTTP_SUCCESS) {
> -    callback.Run(GDataErrorCodeToSyncStatusCode(error));
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK) {
> +    callback.Run(status);
> +    return;
> +  }
> +
> +  if (!resource_list) {
> +    NOTREACHED();
> +    callback.Run(SYNC_STATUS_FAILED);
>      return;
>    }
>
> @@ -659,8 +683,9 @@ void RemoteToLocalSyncer::DidDownloadFile(const
> SyncStatusCallback& callback,
>                                            webkit_blob::ScopedFile file,
>                                            google_apis::GDataErrorCode
> error,
>                                            const base::FilePath&) {
> -  if (error != google_apis::HTTP_SUCCESS) {
> -    callback.Run(GDataErrorCodeToSyncStatusCode(error));
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK) {
> +    callback.Run(status);
>      return;
>    }
>
> Index: chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc
> diff --git
a/chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc
> b/chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc
> index 35bd732ff5cbeb6f2e4aa6635fa46d0fe14f8a84..
> 61abbb8b1b8c6a1c34a94933be078dc946d4c8a5 100644
> --- a/chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc
> +++ b/chrome/browser/sync_file_system/drive_backend/uninstall_app_task.cc
> @@ -70,7 +70,8 @@ void UninstallAppTask::Run(const SyncStatusCallback&
> callback) {
>  void UninstallAppTask::DidDeleteAppRoot(const SyncStatusCallback&
> callback,
>                                          int64 change_id,
>                                          google_apis::GDataErrorCode
> error) {
> -  if (error != google_apis::HTTP_SUCCESS &&
> +  SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
> +  if (status != SYNC_STATUS_OK &&
>        error != google_apis::HTTP_NOT_FOUND) {
>      callback.Run(SYNC_STATUS_FAILED);
>      return;
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tzik@chromium.org/99383004/20001

7 years ago (2013-12-13 01:23:50 UTC) #4

Message was sent while issue was closed.

Change committed as 240549

Expand Messages | Collapse Messages