Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(93)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 18612002: Extract Drive related code to drive::MountPointProviderDelegate. (Closed)

Created:
7 years, 5 months ago by hidehiko
Modified:
7 years, 5 months ago
Reviewers:
satorux1, kinaba
CC:
chromium-reviews, nkostylev+watch_chromium.org, tfarina, oshima+watch_chromium.org, tzik+watch_chromium.org, kinuko+watch, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

Extract Drive related code to drive::MountPointProviderDelegate. This CL adds Delegate interface to CrosMountPointProvider and its implementation for drive file system drive::MountPointProviderDelegate. This is the preparation to remove the RemoteFileSystemProxy, which is only used for Chrome OS, and its related code in ExternalMountPoints from webkit/browser/fileapi. BUG=253837 TEST=Ran browser_tests --gtest_filter="*FileSystemExtensionApiTest*:*FileManagerBrowserTest*" Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210161

Patch Set 1 : #

Total comments: 4

Patch Set 2 : #

Total comments: 2

Patch Set 3 : Rebase #

Patch Set 4 : #

Total comments: 2

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+243 lines, -45 lines) Patch
M chrome/browser/chrome_content_browser_client.cc View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/drive/mount_point_provider_delegate.h View 1 1 chunk +52 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/drive/mount_point_provider_delegate.cc View 1 chunk +92 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/fileapi/cros_mount_point_provider.h View 1 3 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/fileapi/cros_mount_point_provider.cc View 1 8 chunks +13 lines, -37 lines 0 comments Download
A chrome/browser/chromeos/fileapi/cros_mount_point_provider_delegate.h View 1 1 chunk +65 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc View 1 2 3 4 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/chromeos/fileapi/remote_file_system_operation.h View 1 2 3 4 2 chunks +4 lines, -5 lines 0 comments Download
M chrome/chrome_browser_chromeos.gypi View 1 2 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
hidehiko
Satoru, could you review files under c/b/chromeos/fileapi as an OWNER? Kazuhiro, could you review all ...
7 years, 5 months ago (2013-07-03 05:25:08 UTC) #1
satorux1
https://codereview.chromium.org/18612002/diff/1001/chrome/browser/chromeos/fileapi/cros_mount_point_provider.h File chrome/browser/chromeos/fileapi/cros_mount_point_provider.h (right): https://codereview.chromium.org/18612002/diff/1001/chrome/browser/chromeos/fileapi/cros_mount_point_provider.h#newcode73 chrome/browser/chromeos/fileapi/cros_mount_point_provider.h:73: // dependency in the header file. I'm confused drive::MountPointProviderDelegate ...
7 years, 5 months ago (2013-07-03 05:44:29 UTC) #2
hidehiko
Thank you for your review. PTAL? https://codereview.chromium.org/18612002/diff/1001/chrome/browser/chromeos/fileapi/cros_mount_point_provider.h File chrome/browser/chromeos/fileapi/cros_mount_point_provider.h (right): https://codereview.chromium.org/18612002/diff/1001/chrome/browser/chromeos/fileapi/cros_mount_point_provider.h#newcode73 chrome/browser/chromeos/fileapi/cros_mount_point_provider.h:73: // dependency in ...
7 years, 5 months ago (2013-07-03 08:37:01 UTC) #3
satorux1
> In order to remove the classes only used for Chrome OS from > webkit/browser/fileapi ...
7 years, 5 months ago (2013-07-04 01:32:45 UTC) #4
hidehiko
Thank you for your review. Updated the CL description. PTAL? https://codereview.chromium.org/18612002/diff/21001/chrome/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc File chrome/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc (right): https://codereview.chromium.org/18612002/diff/21001/chrome/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc#newcode42 ...
7 years, 5 months ago (2013-07-04 04:10:48 UTC) #5
satorux1
LGTM. The description is a lot clearer now.
7 years, 5 months ago (2013-07-04 04:12:29 UTC) #6
kinaba
lgtm https://codereview.chromium.org/18612002/diff/51001/chrome/browser/chromeos/fileapi/remote_file_system_operation.h File chrome/browser/chromeos/fileapi/remote_file_system_operation.h (right): https://codereview.chromium.org/18612002/diff/51001/chrome/browser/chromeos/fileapi/remote_file_system_operation.h#newcode32 chrome/browser/chromeos/fileapi/remote_file_system_operation.h:32: RemoteFileSystemOperation( nit: explicit?
7 years, 5 months ago (2013-07-04 05:47:43 UTC) #7
hidehiko
Thank you for your review. Sending to CQ. https://codereview.chromium.org/18612002/diff/51001/chrome/browser/chromeos/fileapi/remote_file_system_operation.h File chrome/browser/chromeos/fileapi/remote_file_system_operation.h (right): https://codereview.chromium.org/18612002/diff/51001/chrome/browser/chromeos/fileapi/remote_file_system_operation.h#newcode32 chrome/browser/chromeos/fileapi/remote_file_system_operation.h:32: RemoteFileSystemOperation( ...
7 years, 5 months ago (2013-07-04 05:51:31 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hidehiko@chromium.org/18612002/48002
7 years, 5 months ago (2013-07-04 05:52:20 UTC) #9
commit-bot: I haz the power
7 years, 5 months ago (2013-07-04 09:14:01 UTC) #10
Message was sent while issue was closed. 
Change committed as 210161

Powered by Google App Engine
This is Rietveld 408576698