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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1239043002: Add support for actions for multiple file selection. (Closed)

Created:
5 years, 5 months ago by mtomasz
Modified:
5 years, 2 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, asvitkine+watch_chromium.org, rginda+watch_chromium.org, jlklein+watch-closure_chromium.org, oshima+watch_chromium.org, vitalyp+closure_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, dbeam+watch-closure_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add support for actions for multiple file selection. Actions should be provided for selection, as it's common for users to select more than one file and perform some operation them. Eg. pinning 10 selected files. NOPRESUBMIT=true TBR=isherman@chromium.org TEST=unit_tests: *FileSystemProvider*Action* BUG=None Committed: https://crrev.com/365c582fcb8c3424ad7356c6893d342a490502e2 Cr-Commit-Position: refs/heads/master@{#351065}

Patch Set 1 #

Patch Set 2 : Fixed tests. #

Patch Set 3 : Better documentation. #

Total comments: 4

Patch Set 4 : Rebased. #

Patch Set 5 : Fixed. #

Total comments: 2

Patch Set 6 : Fixed uniques. #

Total comments: 2

Patch Set 7 : Fixed. #

Patch Set 8 : Fixed externs. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+265 lines, -154 lines) Patch
M chrome/browser/chromeos/extensions/file_manager/private_api_misc.h View 1 3 chunks +14 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc View 1 2 3 4 5 7 chunks +88 lines, -34 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/execute_action.h View 1 2 3 4 5 6 3 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/execute_action.cc View 1 2 3 4 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/execute_action_unittest.cc View 1 2 3 4 7 chunks +14 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/get_actions.h View 1 2 3 4 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/get_actions.cc View 1 2 3 4 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/get_actions_unittest.cc View 1 2 3 4 7 chunks +15 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system.h View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system.cc View 1 2 3 4 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h View 1 2 3 4 1 chunk +7 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/throttled_file_system.h View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/throttled_file_system.cc View 1 2 3 4 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/common/extensions/api/file_manager_private.idl View 1 2 3 2 chunks +10 lines, -10 lines 0 comments Download
M chrome/common/extensions/api/file_manager_private_internal.idl View 1 2 chunks +6 lines, -6 lines 0 comments Download
M chrome/common/extensions/api/file_system_provider.idl View 1 2 4 chunks +11 lines, -9 lines 0 comments Download
M chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js View 1 2 3 1 chunk +12 lines, -7 lines 0 comments Download
M chrome/test/data/extensions/api_test/file_system_provider/execute_action/test.js View 1 4 chunks +8 lines, -6 lines 0 comments Download
M chrome/test/data/extensions/api_test/file_system_provider/get_actions/test.js View 1 4 chunks +33 lines, -8 lines 0 comments Download
M extensions/browser/extension_function_histogram_value.h View 1 2 3 1 chunk +2 lines, -0 lines 2 comments Download
M third_party/closure_compiler/externs/file_manager_private.js View 1 2 3 4 5 6 7 1 chunk +8 lines, -8 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 36 (12 generated)
mtomasz
@benwells: PTAL at idl. Let me know if we need an API review for this ...
5 years, 5 months ago (2015-07-22 09:59:54 UTC) #2
fukino
https://codereview.chromium.org/1239043002/diff/40001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc (right): https://codereview.chromium.org/1239043002/diff/40001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc#newcode109 chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:109: std::set<base::FilePath>* paths, optional nit: Can we use vector for ...
5 years, 5 months ago (2015-07-23 04:52:18 UTC) #3
benwells
idl lgtm
5 years, 4 months ago (2015-08-03 02:49:28 UTC) #4
mtomasz
https://codereview.chromium.org/1239043002/diff/40001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc (right): https://codereview.chromium.org/1239043002/diff/40001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc#newcode109 chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:109: std::set<base::FilePath>* paths, On 2015/07/23 04:52:18, fukino wrote: > optional ...
5 years, 3 months ago (2015-09-17 05:03:46 UTC) #5
mtomasz
@fukino: Dones. PTAL thanks!
5 years, 3 months ago (2015-09-17 05:05:40 UTC) #7
fukino
https://codereview.chromium.org/1239043002/diff/70001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc (right): https://codereview.chromium.org/1239043002/diff/70001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc#newcode142 chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:142: // Erase duplicates. Is it guaranteed that paths are ...
5 years, 3 months ago (2015-09-17 05:23:04 UTC) #8
mtomasz
https://codereview.chromium.org/1239043002/diff/70001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc (right): https://codereview.chromium.org/1239043002/diff/70001/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc#newcode142 chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:142: // Erase duplicates. On 2015/09/17 05:23:04, fukino wrote: > ...
5 years, 3 months ago (2015-09-17 06:12:40 UTC) #9
fukino
lgtm. Thank you!
5 years, 3 months ago (2015-09-17 06:30:14 UTC) #10
mtomasz
@isherman: TBR for the histogram renames. Thanks. Thanks!
5 years, 3 months ago (2015-09-17 06:32:57 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1239043002/90001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1239043002/90001
5 years, 3 months ago (2015-09-17 06:34:36 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/100872)
5 years, 3 months ago (2015-09-17 06:44:39 UTC) #16
Dan Beam
https://codereview.chromium.org/1239043002/diff/90001/chrome/browser/chromeos/file_system_provider/operations/execute_action.h File chrome/browser/chromeos/file_system_provider/operations/execute_action.h (right): https://codereview.chromium.org/1239043002/diff/90001/chrome/browser/chromeos/file_system_provider/operations/execute_action.h#newcode9 chrome/browser/chromeos/file_system_provider/operations/execute_action.h:9: #include <string> swap these to please the presubmit
5 years, 3 months ago (2015-09-17 16:52:33 UTC) #17
Ilya Sherman
histograms.xml lgtm
5 years, 3 months ago (2015-09-17 21:31:05 UTC) #18
mtomasz
https://codereview.chromium.org/1239043002/diff/90001/chrome/browser/chromeos/file_system_provider/operations/execute_action.h File chrome/browser/chromeos/file_system_provider/operations/execute_action.h (right): https://codereview.chromium.org/1239043002/diff/90001/chrome/browser/chromeos/file_system_provider/operations/execute_action.h#newcode9 chrome/browser/chromeos/file_system_provider/operations/execute_action.h:9: #include <string> On 2015/09/17 16:52:33, Dan Beam wrote: > ...
5 years, 3 months ago (2015-09-18 01:20:33 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1239043002/130001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1239043002/130001
5 years, 3 months ago (2015-09-18 01:21:35 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/101325)
5 years, 3 months ago (2015-09-18 01:33:02 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1239043002/130001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1239043002/130001
5 years, 2 months ago (2015-09-28 12:41:59 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/104237)
5 years, 2 months ago (2015-09-28 12:52:15 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1239043002/130001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1239043002/130001
5 years, 2 months ago (2015-09-28 13:29:16 UTC) #30
commit-bot: I haz the power
Committed patchset #8 (id:130001)
5 years, 2 months ago (2015-09-28 14:08:17 UTC) #31
commit-bot: I haz the power
Patchset 8 (id:??) landed as https://crrev.com/365c582fcb8c3424ad7356c6893d342a490502e2 Cr-Commit-Position: refs/heads/master@{#351065}
5 years, 2 months ago (2015-09-28 14:09:24 UTC) #32
Ilya Sherman
https://codereview.chromium.org/1239043002/diff/130001/extensions/browser/extension_function_histogram_value.h File extensions/browser/extension_function_histogram_value.h (right): https://codereview.chromium.org/1239043002/diff/130001/extensions/browser/extension_function_histogram_value.h#newcode1124 extensions/browser/extension_function_histogram_value.h:1124: FILEMANAGERPRIVATEINTERNAL_EXECUTECUSTOMACTION, Yikes, I did not notice that you added ...
5 years, 2 months ago (2015-10-16 19:49:49 UTC) #33
mtomasz
https://codereview.chromium.org/1239043002/diff/130001/extensions/browser/extension_function_histogram_value.h File extensions/browser/extension_function_histogram_value.h (right): https://codereview.chromium.org/1239043002/diff/130001/extensions/browser/extension_function_histogram_value.h#newcode1124 extensions/browser/extension_function_histogram_value.h:1124: FILEMANAGERPRIVATEINTERNAL_EXECUTECUSTOMACTION, On 2015/10/16 19:49:49, Ilya Sherman wrote: > Yikes, ...
5 years, 2 months ago (2015-10-16 23:40:55 UTC) #34
Alexei Svitkine (slow)
5 years, 2 months ago (2015-10-21 17:10:47 UTC) #36
Message was sent while issue was closed. 
The repercussions of this CL are being discussed here:

https://codereview.chromium.org/1373073003/

Powered by Google App Engine
This is Rietveld 408576698