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

Issue 2505993005: Move FS related mojo methods to ArcFileSystemInstance. (Closed)

Created:
4 years, 1 month ago by Shuhei Takahashi
Modified:
4 years ago
CC:
Aaron Boodman, abarth-chromium, chromium-reviews, darin (slow to review), elijahtaylor+arcwatch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yusukes+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move FS related mojo methods to ArcFileSystemInstance. Existing methods are suffixed with "Deprecated". Note that new methods will be soon implemented in Android side but they are not available at this moment. BUG=chromium:666231 TEST=unit_tests --gtest_filter='ArcContentFileSystem*' Committed: https://crrev.com/9b3835940eb71e1e9b3bfe2a7f60f381bcf63238 Cr-Commit-Position: refs/heads/master@{#434603}

Patch Set 1 #

Total comments: 6

Patch Set 2 : Add "Deprecated" suffix to IPC methods. #

Patch Set 3 : Fix unit tests. #

Patch Set 4 : . #

Total comments: 5

Patch Set 5 : Define version constants. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -51 lines) Patch
M chrome/browser/chromeos/arc/fileapi/arc_content_file_system_async_file_util_unittest.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc View 1 2 2 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/arc/fileapi/intent_helper_util.h View 1 1 chunk +11 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc View 1 2 3 4 3 chunks +20 lines, -24 lines 0 comments Download
M components/arc/common/file_system.mojom View 1 1 chunk +11 lines, -0 lines 0 comments Download
M components/arc/common/intent_helper.mojom View 1 2 chunks +4 lines, -5 lines 0 comments Download
M components/arc/test/fake_intent_helper_instance.h View 1 2 2 chunks +6 lines, -4 lines 0 comments Download
M components/arc/test/fake_intent_helper_instance.cc View 1 2 2 chunks +4 lines, -4 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 33 (21 generated)
Shuhei Takahashi
rickyz: PTAL yusukes, hashimoto: FYI
4 years, 1 month ago (2016-11-18 07:49:38 UTC) #4
Luis Héctor Chávez
drive-by https://codereview.chromium.org/2505993005/diff/1/components/arc/common/intent_helper.mojom File components/arc/common/intent_helper.mojom (right): https://codereview.chromium.org/2505993005/diff/1/components/arc/common/intent_helper.mojom#newcode120 components/arc/common/intent_helper.mojom:120: [MinVersion=15] GetFileSize@11(string url) => (int64 size); Can you ...
4 years, 1 month ago (2016-11-18 16:37:53 UTC) #8
Yusuke Sato
lgtm with nits: https://codereview.chromium.org/2505993005/diff/1/components/arc/common/file_system.mojom File components/arc/common/file_system.mojom (right): https://codereview.chromium.org/2505993005/diff/1/components/arc/common/file_system.mojom#newcode14 components/arc/common/file_system.mojom:14: RequestMediaScan@0(array<string> paths); move to L23 to ...
4 years, 1 month ago (2016-11-18 19:09:38 UTC) #9
rickyz (no longer on Chrome)
lgtm
4 years, 1 month ago (2016-11-18 22:53:32 UTC) #10
Shuhei Takahashi
PTAL https://codereview.chromium.org/2505993005/diff/1/components/arc/common/file_system.mojom File components/arc/common/file_system.mojom (right): https://codereview.chromium.org/2505993005/diff/1/components/arc/common/file_system.mojom#newcode14 components/arc/common/file_system.mojom:14: RequestMediaScan@0(array<string> paths); On 2016/11/18 19:09:38, Yusuke Sato wrote: ...
4 years ago (2016-11-21 11:06:29 UTC) #21
Yusuke Sato
lgtm https://codereview.chromium.org/2505993005/diff/60001/chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc File chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc (right): https://codereview.chromium.org/2505993005/diff/60001/chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc#newcode33 chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc:33: "GetFileSizeDeprecated", 15); nit: not your fault, but can ...
4 years ago (2016-11-21 16:52:30 UTC) #22
Yusuke Sato
https://codereview.chromium.org/2505993005/diff/60001/chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc File chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc (right): https://codereview.chromium.org/2505993005/diff/60001/chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc#newcode33 chrome/browser/chromeos/arc/fileapi/intent_helper_util.cc:33: "GetFileSizeDeprecated", 15); On 2016/11/21 16:52:30, Yusuke Sato wrote: > ...
4 years ago (2016-11-21 17:49:01 UTC) #23
Shuhei Takahashi
Thanks for reviewing! Since it's soon after the branch cut, I'll submit this change early ...
4 years ago (2016-11-22 08:55:42 UTC) #24
Luis Héctor Chávez
lgtm
4 years ago (2016-11-22 18:54:01 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2505993005/80001
4 years ago (2016-11-28 04:08:56 UTC) #28
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years ago (2016-11-28 04:36:49 UTC) #31
commit-bot: I haz the power
4 years ago (2016-11-28 04:39:21 UTC) #33
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/9b3835940eb71e1e9b3bfe2a7f60f381bcf63238
Cr-Commit-Position: refs/heads/master@{#434603}

Powered by Google App Engine
This is Rietveld 408576698