Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
360673004:
Demonstrate the functionality of the File System Provider API for archives. (Closed)
DescriptionDemonstrate the functionality of the File System Provider API for archives.
Extension for mounting fake archives on launch event.
Multiple archives can be mounted and handled at the same time.
There is support for listening directories, reading files, etc.
As an important feature we have the state saving on suspending the
extension. When the extension is loaded again we can restore the
state at the previous point before suspending. Also there is
support for restarts and crashes by using the onStartup event.
BUG=388080
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=280773
Patch Set 1 #
Total comments: 31
Patch Set 2 : Split readMetadataFromFile function in order to make the code more readable and correct some commen… #
Total comments: 20
Patch Set 3 : Resolve a bug with the entry.name used as mounted fileSystemId as entry.name can be duplicated. Now… #
Total comments: 4
Patch Set 4 : Resolve format issues for files stored in JSON format. #Patch Set 5 : Add fileSystemProvider API extension example. #
Messages
Total messages: 16 (0 generated)
PTAL
https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:152: var entryId = chrome.fileSystem.retainEntry(volumes[volumeId].entry_); As a mention: retainEntry executes quite often. I haven't find in the API a method that tests if an Entry is already saved. I saw the isRestorable method, but it's using the entryId not an Entry object. As for the program, based on the retainEntry specification, there shouldn't be any problems with the current code as only the most 500 recent entries are stored.
Nice! Just minor style comments. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:7: // For file examples with metadata inside, see example1.fake and example2.fake. ... -> Metadata is stored in files as serialized to JSON maps. See contents of example1.fake and example2.fake. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:10: // fileSystemId, which is the same as the file entry.name. The value file -> file's https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:16: function Volume(entry, mount, metadata, openedFiles) { openedFiles -> opt_openedFiles https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:17: // Used for restoring file entry. ... -> Used for restoring the opened file entry after resuming the event page. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:20: // The volume metadata. Let's add a comment: Date object is serialized in JSON as string. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:21: for (var item in metadata) { I think it is better not to modify input arguments, especially if they are a reference. How about: this.metadata_ = []; for (...) { this.metadata_[path] = metadata[path]; this.metadata_[path].modificationTime = new Date(...); } Also "item" is not a helpful naming. How about "path"? https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:31: // Mount device and save device information on local storage in order to be Mount device -> Mount the volume and save its information in local .. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:135: var contents = volumes[options.fileSystemId].metadata_[filePath].contents; nit: metadata_ is private here. Let's make it public, so we don't access a private member (metadata_ -> metadata). https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:148: // Save state in case of restart, extension suspend, crashes, etc. nit: restart -> restarts nit: extension suspend -> event page suspends https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:181: function readMetadataFromFile(entry, mount, onSuccess, onError, openedFiles) { readMetadataFromFile shouldn't need |openedFiles|. How about not passing it here, and instead creating the volume in the onSuccess callback? Currently readMetadataFromFile does not only read the metadata, but also mounts the volume. We could split it, and make the code easier to read. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:199: // in the manifest file. Not only file extension, but also if mime type is matching. Note, that currently this doesn't work, but I'm working on it, and it will soon be fixed. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:208: // Event called on chromeos startup. ... -> Event called on a profile startup. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:215: // Remove opened files from state. The file entry restore I think we could clarify it a little bit, eg.: ... -> Remove files opened before the profile shutdown from the state. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:224: // Event called when the extension is idle for about 10+ seconds and it will I think we can remove this comment. The idling period before suspending may change in the future. Instead we could add a comment like: Save the state before suspending the event page, so we can resume it once new events arrive. WDYT? https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/manifest.json (right): https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/manifest.json:5: "description": "Demonstrate File System Provider API usage for archives", nit: period at the end of the description.
https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:152: var entryId = chrome.fileSystem.retainEntry(volumes[volumeId].entry_); On 2014/06/30 08:01:44, cmihail wrote: > As a mention: retainEntry executes quite often. I haven't find in the API a > method that tests if an Entry is already saved. I saw the isRestorable method, > but it's using the entryId not an Entry object. > > As for the program, based on the retainEntry specification, there shouldn't be > any problems with the current code as only the most 500 recent entries are > stored. That's a good point. I took a look at the C++ code and it seems that a file once remembered, will not use more quota than 1. So we can keep it as it is.
https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:7: // For file examples with metadata inside, see example1.fake and example2.fake. On 2014/06/30 08:30:30, mtomasz wrote: > ... -> Metadata is stored in files as serialized to JSON maps. See contents of > example1.fake and example2.fake. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:10: // fileSystemId, which is the same as the file entry.name. The value On 2014/06/30 08:30:31, mtomasz wrote: > file -> file's Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:16: function Volume(entry, mount, metadata, openedFiles) { On 2014/06/30 08:30:31, mtomasz wrote: > openedFiles -> opt_openedFiles Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:17: // Used for restoring file entry. On 2014/06/30 08:30:30, mtomasz wrote: > ... -> Used for restoring the opened file entry after resuming the event page. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:20: // The volume metadata. On 2014/06/30 08:30:31, mtomasz wrote: > Let's add a comment: > Date object is serialized in JSON as string. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:21: for (var item in metadata) { On 2014/06/30 08:30:31, mtomasz wrote: > I think it is better not to modify input arguments, especially if they are a > reference. How about: > > this.metadata_ = []; > for (...) { > this.metadata_[path] = metadata[path]; > this.metadata_[path].modificationTime = new Date(...); > } > > Also "item" is not a helpful naming. How about "path"? Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:31: // Mount device and save device information on local storage in order to be On 2014/06/30 08:30:31, mtomasz wrote: > Mount device -> Mount the volume and save its information in local .. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:135: var contents = volumes[options.fileSystemId].metadata_[filePath].contents; On 2014/06/30 08:30:31, mtomasz wrote: > nit: metadata_ is private here. Let's make it public, so we don't access a > private member (metadata_ -> metadata). Done. Also modified the other variables. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:148: // Save state in case of restart, extension suspend, crashes, etc. On 2014/06/30 08:30:30, mtomasz wrote: > nit: restart -> restarts > nit: extension suspend -> event page suspends Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:181: function readMetadataFromFile(entry, mount, onSuccess, onError, openedFiles) { On 2014/06/30 08:30:30, mtomasz wrote: > readMetadataFromFile shouldn't need |openedFiles|. How about not passing it > here, and instead creating the volume in the onSuccess callback? > > Currently readMetadataFromFile does not only read the metadata, but also mounts > the volume. We could split it, and make the code easier to read. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:199: // in the manifest file. On 2014/06/30 08:30:30, mtomasz wrote: > Not only file extension, but also if mime type is matching. > > Note, that currently this doesn't work, but I'm working on it, and it will soon > be fixed. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:208: // Event called on chromeos startup. On 2014/06/30 08:30:31, mtomasz wrote: > ... -> Event called on a profile startup. Done. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:215: // Remove opened files from state. The file entry restore On 2014/06/30 08:30:31, mtomasz wrote: > I think we could clarify it a little bit, eg.: > ... -> Remove files opened before the profile shutdown from the state. Done. I added local storage state. It seems a bit more clear to me. https://codereview.chromium.org/360673004/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:224: // Event called when the extension is idle for about 10+ seconds and it will On 2014/06/30 08:30:31, mtomasz wrote: > I think we can remove this comment. The idling period before suspending may > change in the future. > > Instead we could add a comment like: Save the state before suspending the event > page, so we can resume it once new events arrive. WDYT? That sounds good to me.
https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:15: // Defines a volume object that contains information about mounted devices. nit: about mounted devices -> about a mounted file system. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:20: // The volume metadata. Date object is serialized in JSON as string. nit: I think the second comment is confusing in this place, since metadata has already Date object inside. Let's move the second sentence to #24. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:28: // A map with currently opened files. As key it has requestId of nit: As key it has requestId... -> The key is a requestId value from the openFileRequested event, and the value is the file path. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:139: // Save state in case of restarts, event page suspend, crashes, etc. nit: Save -> Saves https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:152: // Restore state. In this case the file system is already mounted and nit: Restore -> Restores. Comments for methods should be rather written in 3-rd person form. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:153: // we only need to obtain the metadata, which is done lazily. The comment about remounting is confusing. How about: Restores metadata for the passed file system ID. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:175: // onSuccess has as parameter the metadata read in JSON format. The returned metadata is already deserialized, so not in JSON format. How about: ... -> Reads metadata from a file and returns it with the onSuccess callback. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:192: // mentioned in the manifest file. nit: clicking on the file -> opening the file nit: mentioned -> declared https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:202: {fileSystemId: item.entry.name, displayName: item.entry.name}, This may fail when two files with the same name are in different directories. How about using: https://developer.chrome.com/apps/fileSystem#method-getDisplayPath for the fileSystemmId? https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:220: // storage state. nit: local storage state -> local storage.
https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:15: // Defines a volume object that contains information about mounted devices. On 2014/07/01 00:40:33, mtomasz wrote: > nit: about mounted devices -> about a mounted file system. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:20: // The volume metadata. Date object is serialized in JSON as string. On 2014/07/01 00:40:33, mtomasz wrote: > nit: I think the second comment is confusing in this place, since metadata has > already Date object inside. Let's move the second sentence to #24. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:28: // A map with currently opened files. As key it has requestId of On 2014/07/01 00:40:32, mtomasz wrote: > nit: As key it has requestId... -> The key is a requestId value from the > openFileRequested event, and the value is the file path. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:139: // Save state in case of restarts, event page suspend, crashes, etc. On 2014/07/01 00:40:32, mtomasz wrote: > nit: Save -> Saves Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:152: // Restore state. In this case the file system is already mounted and On 2014/07/01 00:40:32, mtomasz wrote: > nit: Restore -> Restores. Comments for methods should be rather written in 3-rd > person form. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:153: // we only need to obtain the metadata, which is done lazily. On 2014/07/01 00:40:33, mtomasz wrote: > The comment about remounting is confusing. How about: > > Restores metadata for the passed file system ID. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:175: // onSuccess has as parameter the metadata read in JSON format. On 2014/07/01 00:40:33, mtomasz wrote: > The returned metadata is already deserialized, so not in JSON format. > > How about: > ... -> Reads metadata from a file and returns it with the onSuccess callback. Yes. I wasn't clear. I was referring to the fact that the metadata is stored as JSON format in the file. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:192: // mentioned in the manifest file. On 2014/07/01 00:40:32, mtomasz wrote: > nit: clicking on the file -> opening the file > nit: mentioned -> declared Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:202: {fileSystemId: item.entry.name, displayName: item.entry.name}, On 2014/07/01 00:40:33, mtomasz wrote: > This may fail when two files with the same name are in different directories. > > How about using: > https://developer.chrome.com/apps/fileSystem#method-getDisplayPath > > for the fileSystemmId? Done. For displayName I have kept item.entry.name as it looks better. If necessary I can do something like adding (2), (3), etc at the end of each name. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:220: // storage state. On 2014/07/01 00:40:32, mtomasz wrote: > nit: local storage state -> local storage. Done.
https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js (right): https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:15: // Defines a volume object that contains information about mounted devices. On 2014/07/01 00:40:33, mtomasz wrote: > nit: about mounted devices -> about a mounted file system. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:20: // The volume metadata. Date object is serialized in JSON as string. On 2014/07/01 00:40:33, mtomasz wrote: > nit: I think the second comment is confusing in this place, since metadata has > already Date object inside. Let's move the second sentence to #24. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:28: // A map with currently opened files. As key it has requestId of On 2014/07/01 00:40:32, mtomasz wrote: > nit: As key it has requestId... -> The key is a requestId value from the > openFileRequested event, and the value is the file path. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:139: // Save state in case of restarts, event page suspend, crashes, etc. On 2014/07/01 00:40:32, mtomasz wrote: > nit: Save -> Saves Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:152: // Restore state. In this case the file system is already mounted and On 2014/07/01 00:40:32, mtomasz wrote: > nit: Restore -> Restores. Comments for methods should be rather written in 3-rd > person form. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:153: // we only need to obtain the metadata, which is done lazily. On 2014/07/01 00:40:33, mtomasz wrote: > The comment about remounting is confusing. How about: > > Restores metadata for the passed file system ID. Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:175: // onSuccess has as parameter the metadata read in JSON format. On 2014/07/01 00:40:33, mtomasz wrote: > The returned metadata is already deserialized, so not in JSON format. > > How about: > ... -> Reads metadata from a file and returns it with the onSuccess callback. Yes. I wasn't clear. I was referring to the fact that the metadata is stored as JSON format in the file. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:192: // mentioned in the manifest file. On 2014/07/01 00:40:32, mtomasz wrote: > nit: clicking on the file -> opening the file > nit: mentioned -> declared Done. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:202: {fileSystemId: item.entry.name, displayName: item.entry.name}, On 2014/07/01 00:40:33, mtomasz wrote: > This may fail when two files with the same name are in different directories. > > How about using: > https://developer.chrome.com/apps/fileSystem#method-getDisplayPath > > for the fileSystemmId? Done. For displayName I have kept item.entry.name as it looks better. If necessary I can do something like adding (2), (3), etc at the end of each name. https://codereview.chromium.org/360673004/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/background.js:220: // storage state. On 2014/07/01 00:40:32, mtomasz wrote: > nit: local storage state -> local storage. Done.
lgtm with 2 nits https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/example1.fake (right): https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/example1.fake:2: "/":{ nit: Please add a space after each : https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/manifest.json (right): https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/manifest.json:5: "description": "Demonstrate File System Provider API usage for archives", nit: the period (.) is still missing in the description field.
+@kalman for OWNERS review.
https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/example1.fake (right): https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/example1.fake:2: "/":{ On 2014/07/01 01:31:29, mtomasz wrote: > nit: Please add a space after each : Done. https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... File chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/manifest.json (right): https://codereview.chromium.org/360673004/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/examples/api/fileSystemProvider/archive/manifest.json:5: "description": "Demonstrate File System Provider API usage for archives", On 2014/07/01 01:31:29, mtomasz wrote: > nit: the period (.) is still missing in the description field. Done.
lgtm
The CQ bit was checked by mtomasz@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cmihail@chromium.org/360673004/60001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
Message was sent while issue was closed.
Change committed as 280773 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
