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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 258783006: [fsp] Add the getMetadata operation. (Closed)

Created:
6 years, 7 months ago by mtomasz
Modified:
6 years, 7 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, nkostylev+watch_chromium.org, tzik, nhiroki, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, kinuko+watch, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

[fsp] Add the getMetadata operation. This patch adds the getMetadata() operation, which basically returns basic metadata of a file or a directory. It is tied to async file util's getFileInfo() method. TBR=isherman@chromium.org # New histograms. TEST=unit_tests, browser_test: *FileSystemProvider*GetMetadata BUG=248427 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267538

Patch Set 1 : Fixed a lot. #

Total comments: 33

Patch Set 2 : Addressed comments. #

Total comments: 2

Patch Set 3 : Added missing jsdoc. #

Total comments: 2

Patch Set 4 : Rebased. #

Patch Set 5 : Fixed. #

Patch Set 6 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+899 lines, -94 lines) Patch
M chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.h View 1 2 3 4 5 1 chunk +24 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc View 1 2 3 4 5 4 chunks +90 lines, -1 line 0 comments Download
M chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_apitest.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc View 1 chunk +26 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc View 3 chunks +34 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc View 8 chunks +43 lines, -49 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/mount_path_util.h View 1 chunk +3 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/mount_path_util.cc View 1 3 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/chromeos/file_system_provider/mount_path_util_unittest.cc View 1 4 chunks +7 lines, -4 lines 0 comments Download
A + chrome/browser/chromeos/file_system_provider/operations/get_metadata.h View 3 chunks +17 lines, -16 lines 0 comments Download
A chrome/browser/chromeos/file_system_provider/operations/get_metadata.cc View 1 1 chunk +85 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/file_system_provider/operations/get_metadata_unittest.cc View 1 chunk +256 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system.cc View 2 chunks +12 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/request_value.h View 2 chunks +15 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/request_value.cc View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/fileapi/file_system_backend.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/chrome_browser_chromeos.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/api/file_system_provider.idl View 1 2 3 4 3 chunks +33 lines, -5 lines 0 comments Download
M chrome/common/extensions/api/file_system_provider_internal.idl View 1 chunk +14 lines, -0 lines 0 comments Download
M chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js View 1 chunk +19 lines, -0 lines 0 comments Download
A + chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json View 1 chunk +9 lines, -7 lines 0 comments Download
A chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js View 1 2 1 chunk +166 lines, -0 lines 0 comments Download
M extensions/browser/extension_function_histogram_value.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 28 (0 generated)
mtomasz
@kinaba: PTAL at C++ @hirono: PTAL at JS. Thanks!
6 years, 7 months ago (2014-04-30 06:14:11 UTC) #1
mtomasz
On 2014/04/30 06:14:11, mtomasz wrote: > @kinaba: PTAL at C++ > @hirono: PTAL at JS. ...
6 years, 7 months ago (2014-04-30 06:46:40 UTC) #2
hirono
https://codereview.chromium.org/258783006/diff/10001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json File chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json (right): https://codereview.chromium.org/258783006/diff/10001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json#newcode11 chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json:11: "fileBrowserHandler" fileBrowserHandler is needed? https://codereview.chromium.org/258783006/diff/10001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js File chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js (right): https://codereview.chromium.org/258783006/diff/10001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js#newcode4 ...
6 years, 7 months ago (2014-04-30 06:57:38 UTC) #3
benwells
https://codereview.chromium.org/258783006/diff/10001/chrome/common/extensions/api/file_system_provider.idl File chrome/common/extensions/api/file_system_provider.idl (right): https://codereview.chromium.org/258783006/diff/10001/chrome/common/extensions/api/file_system_provider.idl#newcode38 chrome/common/extensions/api/file_system_provider.idl:38: DOMString name; Maybe this should this be called baseName? ...
6 years, 7 months ago (2014-04-30 07:04:34 UTC) #4
kinaba
https://codereview.chromium.org/258783006/diff/10001/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc File chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc (right): https://codereview.chromium.org/258783006/diff/10001/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc#newcode105 chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc:105: base::Bind(&OnGetFileInfo, callback))); nit: Post-back-to-caller-thread callback can be created by ...
6 years, 7 months ago (2014-04-30 07:55:02 UTC) #5
mtomasz
https://codereview.chromium.org/258783006/diff/10001/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc File chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc (right): https://codereview.chromium.org/258783006/diff/10001/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc#newcode105 chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc:105: base::Bind(&OnGetFileInfo, callback))); On 2014/04/30 07:55:03, kinaba wrote: > nit: ...
6 years, 7 months ago (2014-04-30 09:18:23 UTC) #6
kinaba
lgtm
6 years, 7 months ago (2014-04-30 23:50:00 UTC) #7
hirono
lgtm with a nit. https://codereview.chromium.org/258783006/diff/30001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js File chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js (right): https://codereview.chromium.org/258783006/diff/30001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js#newcode24 chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js:24: function onGetMetadataRequested( nit: JSDoc for ...
6 years, 7 months ago (2014-05-01 02:40:39 UTC) #8
mtomasz
https://codereview.chromium.org/258783006/diff/30001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js File chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js (right): https://codereview.chromium.org/258783006/diff/30001/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js#newcode24 chrome/test/data/extensions/api_test/file_system_provider/get_metadata/test.js:24: function onGetMetadataRequested( On 2014/05/01 02:40:40, hirono wrote: > nit: ...
6 years, 7 months ago (2014-05-01 02:50:47 UTC) #9
benwells
idl lgtm https://codereview.chromium.org/258783006/diff/50001/chrome/common/extensions/api/file_system_provider.idl File chrome/common/extensions/api/file_system_provider.idl (right): https://codereview.chromium.org/258783006/diff/50001/chrome/common/extensions/api/file_system_provider.idl#newcode105 chrome/common/extensions/api/file_system_provider.idl:105: // Raised, when metada of a file ...
6 years, 7 months ago (2014-05-01 04:16:24 UTC) #10
mtomasz
https://codereview.chromium.org/258783006/diff/50001/chrome/common/extensions/api/file_system_provider.idl File chrome/common/extensions/api/file_system_provider.idl (right): https://codereview.chromium.org/258783006/diff/50001/chrome/common/extensions/api/file_system_provider.idl#newcode105 chrome/common/extensions/api/file_system_provider.idl:105: // Raised, when metada of a file or a ...
6 years, 7 months ago (2014-05-01 05:09:07 UTC) #11
mtomasz
@isherman: PTAL at extension_function_histogram_value.h and histograms.xml. Thanks.
6 years, 7 months ago (2014-05-01 05:13:44 UTC) #12
mtomasz
The CQ bit was checked by mtomasz@chromium.org
6 years, 7 months ago (2014-05-01 05:13:53 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtomasz@chromium.org/258783006/70002
6 years, 7 months ago (2014-05-01 05:14:28 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-01 05:26:49 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium
6 years, 7 months ago (2014-05-01 05:26:49 UTC) #16
mtomasz
The CQ bit was checked by mtomasz@chromium.org
6 years, 7 months ago (2014-05-01 05:41:30 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtomasz@chromium.org/258783006/70002
6 years, 7 months ago (2014-05-01 05:41:52 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-01 06:10:37 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_clang_dbg on tryserver.chromium
6 years, 7 months ago (2014-05-01 06:10:37 UTC) #20
mtomasz
The CQ bit was checked by mtomasz@chromium.org
6 years, 7 months ago (2014-05-01 06:41:10 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtomasz@chromium.org/258783006/100001
6 years, 7 months ago (2014-05-01 06:41:44 UTC) #22
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-01 07:13:53 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel on tryserver.chromium
6 years, 7 months ago (2014-05-01 07:13:53 UTC) #24
mtomasz
The CQ bit was checked by mtomasz@chromium.org
6 years, 7 months ago (2014-05-01 07:24:19 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtomasz@chromium.org/258783006/100001
6 years, 7 months ago (2014-05-01 07:24:50 UTC) #26
commit-bot: I haz the power
Change committed as 267538
6 years, 7 months ago (2014-05-01 16:50:59 UTC) #27
Ilya Sherman
6 years, 7 months ago (2014-05-01 18:13:39 UTC) #28
Message was sent while issue was closed. 
histograms lgtm

Powered by Google App Engine
This is Rietveld 408576698