Additional methods for chrome.logPrivate API

Added additional methods for chrome.logPrivate:

chrome.logPrivate.dumpLogs(function(fileEntry) {...});
  
- dumps all system logs as |fileEntry| (a .tgz file)


chrome.logPrivate.startEventRecorder(type, function(){});
chrome.logPrivate.stopEventRecorder(type, function(){});

- start/stop additional log collection for dumps generated with chrome.logPrivate.dumpLogs(). 

TEST=LogPrivateApiTest.*
BUG=385289
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285065

[Zachary Kuznia, 2014-06-25 00:05:54]

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/system_logs/debug_log_writer.cc (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
nit: no (c)

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:47: LOG(ERROR) <<
I think we currently prefer VLOG(1)

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:73: base::File* file =
new base::File;
Is there a reason not to make this a scoped_ptr?  I think WriteDebugLogToFile
should at least take a scoped_ptr, so that the ownership is clear.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/system_logs/debug_log_writer.h (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
Nit: no (c)

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:3: // found in the
LICENSE file.
nit: empty line before ifndef

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:19: bool succeded)>
StoreDebugLogsCallback;
s/succeded/succeeded/

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api.h (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api.h:33: typedef enum {
Is there a reason to typedef this, instead of using "enum CapturedEventsSink"?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc
(right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:307: if
(params->type == api::log_private::EVENT_TYPE_NETWORK) {
params->type should be generated as an enum, so if you do a switch on it it'll
catch missing cases when new types are added.

https://codereview.chromium.org/329853010/diff/70001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/70001/chrome/renderer/resource...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
nit: no (c)

https://codereview.chromium.org/329853010/diff/70001/chrome/test/data/extensi...
File chrome/test/data/extensions/api_test/log_private/dump_logs/test.js (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/log_private/dump_logs/test.js:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
nit: no (c)

[xiaowenx, 2014-06-25 05:30:44]

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/system_logs/debug_log_writer.cc (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:72: int flags = 
base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE;
Two spaces here after "=", intentional?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/system_logs/debug_log_writer.h (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:15: // Called once
StoreDebugLogs is complete. Takes two parameters:
The function is called "StoreLogs", not "StoreDebugLogs", right?  Do you want to
rename this callback to "StoreLogsCallback"?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc
(right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:180:
LOG(ERROR) << "Could not open file " << file_path.value()
Does zork's comment earlier about VLOG(1) also apply here?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:190: }
Do you want to add in: "default: NOTREACHED()" to future-proof against adding in
another event sink type and forgetting to update this function?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:216:
break;
Same as above, do you want to add a default case?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:310:
LogPrivateAPI::RECORD_EVENTS_TO_FILE);
We always want network events to be saved to a file?

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:326:
scoped_ptr<api::log_private::StartEventRecorder::Params> params(
Should this say "StopEventRecorder::Params" instead of
"StartEventRecorder::Params"?

[Zachary Kuznia, 2014-06-25 18:39:03]

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc
(right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:190: }
On 2014/06/25 05:30:44, xiaowenx wrote:
> Do you want to add in: "default: NOTREACHED()" to future-proof against adding
in
> another event sink type and forgetting to update this function?

Switches with enums generate a compile error if cases are unhandled, so it's
preferred not to have a default.

[xiaowenx, 2014-06-26 20:52:02]

Also need an html file like
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...
(I think this causes the documentation to be generated from the idl)

[zel, 2014-06-27 19:31:01]

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/system_logs/debug_log_writer.cc (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
On 2014/06/25 00:05:53, Zachary Kuznia wrote:
> nit: no (c)

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:47: LOG(ERROR) <<
On 2014/06/25 00:05:53, Zachary Kuznia wrote:
> I think we currently prefer VLOG(1)

VLOG(1) gets recorded nowhere. This is really an error that indicates serious
issues.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.cc:72: int flags = 
base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE;
On 2014/06/25 05:30:44, xiaowenx wrote:
> Two spaces here after "=", intentional?

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/system_logs/debug_log_writer.h (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
On 2014/06/25 00:05:53, Zachary Kuznia wrote:
> Nit: no (c)

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:3: // found in the
LICENSE file.
On 2014/06/25 00:05:53, Zachary Kuznia wrote:
> nit: empty line before ifndef

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:15: // Called once
StoreDebugLogs is complete. Takes two parameters:
On 2014/06/25 05:30:44, xiaowenx wrote:
> The function is called "StoreLogs", not "StoreDebugLogs", right?  Do you want
to
> rename this callback to "StoreLogsCallback"?

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/system_logs/debug_log_writer.h:19: bool succeded)>
StoreDebugLogsCallback;
On 2014/06/25 00:05:53, Zachary Kuznia wrote:
> s/succeded/succeeded/

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api.h (right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api.h:33: typedef enum {
On 2014/06/25 00:05:53, Zachary Kuznia wrote:
> Is there a reason to typedef this, instead of using "enum CapturedEventsSink"?

Done.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc
(right):

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:180:
LOG(ERROR) << "Could not open file " << file_path.value()
On 2014/06/25 05:30:44, xiaowenx wrote:
> Does zork's comment earlier about VLOG(1) also apply here?

same answer

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:190: }
On 2014/06/25 18:39:03, Zachary Kuznia wrote:
> On 2014/06/25 05:30:44, xiaowenx wrote:
> > Do you want to add in: "default: NOTREACHED()" to future-proof against
adding
> in
> > another event sink type and forgetting to update this function?
> 
> Switches with enums generate a compile error if cases are unhandled, so it's
> preferred not to have a default.

Yep. We prefer failing in compile time vs. runtime.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:216:
break;
On 2014/06/25 05:30:44, xiaowenx wrote:
> Same as above, do you want to add a default case?

same as above

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:310:
LogPrivateAPI::RECORD_EVENTS_TO_FILE);
On 2014/06/25 05:30:44, xiaowenx wrote:
> We always want network events to be saved to a file?

For this API call - yes. Those files are aggregated into debug log dump .tgz
file.

https://codereview.chromium.org/329853010/diff/70001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:326:
scoped_ptr<api::log_private::StartEventRecorder::Params> params(
On 2014/06/25 05:30:44, xiaowenx wrote:
> Should this say "StopEventRecorder::Params" instead of
> "StartEventRecorder::Params"?

Good catch. They happen to be the same by accident. Done.

[Zachary Kuznia, 2014-06-27 22:34:08]

LGTM with nits.

https://codereview.chromium.org/329853010/diff/90001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc
(right):

https://codereview.chromium.org/329853010/diff/90001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:307: if
(params->type == api::log_private::EVENT_TYPE_NETWORK) {
Nit: switch on type here, too.

https://codereview.chromium.org/329853010/diff/90001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/90001/chrome/renderer/resource...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
Nit: No (c)

https://codereview.chromium.org/329853010/diff/90001/chrome/test/data/extensi...
File chrome/test/data/extensions/api_test/log_private/dump_logs/test.js (right):

https://codereview.chromium.org/329853010/diff/90001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/log_private/dump_logs/test.js:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
Nit: no (c)

[zel, 2014-07-14 19:14:41]

https://codereview.chromium.org/329853010/diff/90001/chrome/browser/extension...
File chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc
(right):

https://codereview.chromium.org/329853010/diff/90001/chrome/browser/extension...
chrome/browser/extensions/api/log_private/log_private_api_chromeos.cc:307: if
(params->type == api::log_private::EVENT_TYPE_NETWORK) {
On 2014/06/27 22:34:07, Zachary Kuznia wrote:
> Nit: switch on type here, too.

Done.

https://codereview.chromium.org/329853010/diff/90001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/90001/chrome/renderer/resource...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
On 2014/06/27 22:34:08, Zachary Kuznia wrote:
> Nit: No (c)

Done.

https://codereview.chromium.org/329853010/diff/90001/chrome/test/data/extensi...
File chrome/test/data/extensions/api_test/log_private/dump_logs/test.js (right):

https://codereview.chromium.org/329853010/diff/90001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/log_private/dump_logs/test.js:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
On 2014/06/27 22:34:08, Zachary Kuznia wrote:
> Nit: no (c)

Done.

[zel, 2014-07-18 22:32:52]

+kalman

[not at google - send to devlin, 2014-07-18 22:45:09]

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
File chrome/common/extensions/api/_api_features.json (right):

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/_api_features.json:496: "extension_types":
["extension", "platform_app"],
you don't even need this line, it's implied by the dependency

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
File chrome/common/extensions/api/log_private.idl (right):

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:60: // Events will be sent to a log
file.
mention that they are written when dumpLogs is called.

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:70: static void
stopEventRecorder(EventType eventType, CompletionCallback callback);
keep these to < 80 chars.

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:72: static void
dumpLogs(DumpLogsCallback callback);
mention the structure of the tar

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
File chrome/common/extensions/docs/templates/public/apps/logPrivate.html
(right):

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/docs/templates/public/apps/logPrivate.html:1:
{{+partials.standard_apps_api api:apis.apps.logPrivate/}}
add a file for extensions as well?

https://codereview.chromium.org/329853010/diff/330001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/330001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:23: // after
the original window is closed.
how much of this is different from file_system_custom_bindings.js? can that be
factored into a common place?

[zel, 2014-07-22 01:26:36]

+rockot for extensions/browser/api/api_resource_manager.h part

In other to get all my file business on the sequenced task runner, I needed to
improve extension::ApiResourceManager<T> to work seamlessly on the blocking pool
(in addition to named threads).

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
File chrome/common/extensions/api/log_private.idl (right):

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:60: // Events will be sent to a log
file.
On 2014/07/18 22:45:09, kalman wrote:
> mention that they are written when dumpLogs is called.

Done.

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:60: // Events will be sent to a log
file.
On 2014/07/18 22:45:09, kalman wrote:
> mention that they are written when dumpLogs is called.

Done.

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:70: static void
stopEventRecorder(EventType eventType, CompletionCallback callback);
On 2014/07/18 22:45:09, kalman wrote:
> keep these to < 80 chars.

Done.

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/api/log_private.idl:72: static void
dumpLogs(DumpLogsCallback callback);
On 2014/07/18 22:45:09, kalman wrote:
> mention the structure of the tar

Done.

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
File chrome/common/extensions/docs/templates/public/apps/logPrivate.html
(right):

https://codereview.chromium.org/329853010/diff/330001/chrome/common/extension...
chrome/common/extensions/docs/templates/public/apps/logPrivate.html:1:
{{+partials.standard_apps_api api:apis.apps.logPrivate/}}
On 2014/07/18 22:45:09, kalman wrote:
> add a file for extensions as well?

Done.

https://codereview.chromium.org/329853010/diff/330001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/330001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:23: // after
the original window is closed.
On 2014/07/18 22:45:09, kalman wrote:
> how much of this is different from file_system_custom_bindings.js? can that be
> factored into a common place?

file_system_custom_bindings.js does very something similar (most of the code
originated from there) but method signatures are bit different. For example,
fileSystem callbacks pass arrays of file entries while the method here passes a
single file entry back to the caller.

[Ken Rockot(use gerrit already), 2014-07-22 15:34:00]

Nice-  ApiResourceManager stuff lgtm

https://codereview.chromium.org/329853010/diff/370001/extensions/browser/api/...
File extensions/browser/api/api_resource_manager.h (right):

https://codereview.chromium.org/329853010/diff/370001/extensions/browser/api/...
extensions/browser/api/api_resource_manager.h:382: // Instead of kThreadId.
classes used for tracking identifying such resources
nit: -identifying? (or +and)

[not at google - send to devlin, 2014-07-22 17:55:07]

https://codereview.chromium.org/329853010/diff/330001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/330001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:23: // after
the original window is closed.
On 2014/07/22 01:26:35, zel wrote:
> On 2014/07/18 22:45:09, kalman wrote:
> > how much of this is different from file_system_custom_bindings.js? can that
be
> > factored into a common place?
> 
> file_system_custom_bindings.js does very something similar (most of the code
> originated from there) but method signatures are bit different. For example,
> fileSystem callbacks pass arrays of file entries while the method here passes
a
> single file entry back to the caller.
>   

Ok, thanks. I'll make a suggestion on the current version of the file, but in
response to the point about an array of filesystems vs a single filesystem -
they're still very similar, and you could imagine this factored-to-share-code
approach to take that into account.

https://codereview.chromium.org/329853010/diff/370001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/370001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:24: if
(window == backgroundPage) {
How about for now you just factor out lines 24-88 into a file like
file_entry_binding_util which returns this bindFileEntryCallback method.

https://codereview.chromium.org/329853010/diff/370001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:32: var
entry = null;
[this isn't used, but it's a good place to attach a comment]

It looks like with multiple set to false
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/re...)
the code in file_system_custom_bindings.js does what you're doing here?

and given that you're not setting multiple in log_private_api.cc (since it's not
supported) it'll automatically work?

[zel, 2014-07-22 22:45:03]

https://codereview.chromium.org/329853010/diff/370001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/log_private_custom_bindings.js
(right):

https://codereview.chromium.org/329853010/diff/370001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:24: if
(window == backgroundPage) {
On 2014/07/22 17:55:07, kalman wrote:
> How about for now you just factor out lines 24-88 into a file like
> file_entry_binding_util which returns this bindFileEntryCallback method.

Done.

https://codereview.chromium.org/329853010/diff/370001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/log_private_custom_bindings.js:32: var
entry = null;
On 2014/07/22 17:55:07, kalman wrote:
> [this isn't used, but it's a good place to attach a comment]
> 
> It looks like with multiple set to false
>
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/re...)
> the code in file_system_custom_bindings.js does what you're doing here?
> 
> and given that you're not setting multiple in log_private_api.cc (since it's
not
> supported) it'll automatically work?

Done.

https://codereview.chromium.org/329853010/diff/370001/extensions/browser/api/...
File extensions/browser/api/api_resource_manager.h (right):

https://codereview.chromium.org/329853010/diff/370001/extensions/browser/api/...
extensions/browser/api/api_resource_manager.h:382: // Instead of kThreadId.
classes used for tracking identifying such resources
On 2014/07/22 15:33:59, Ken Rockot wrote:
> nit: -identifying? (or +and)

Done.

[not at google - send to devlin, 2014-07-23 01:52:07]

lgtm

https://codereview.chromium.org/329853010/diff/430001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/file_entry_binding_util.js (right):

https://codereview.chromium.org/329853010/diff/430001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/file_entry_binding_util.js:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
2014

https://codereview.chromium.org/329853010/diff/430001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/file_entry_binding_util.js:11: 
comment?

[zel, 2014-07-23 03:12:53]

+sky for stuff under chrome/* and chrome/common/*

https://codereview.chromium.org/329853010/diff/430001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/file_entry_binding_util.js (right):

https://codereview.chromium.org/329853010/diff/430001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/file_entry_binding_util.js:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
On 2014/07/23 01:52:07, kalman wrote:
> 2014

Done.

https://codereview.chromium.org/329853010/diff/430001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/file_entry_binding_util.js:11: 
On 2014/07/23 01:52:07, kalman wrote:
> comment?

Done.

[sky, 2014-07-23 15:44:26]

LGTM

[jam, 2014-07-23 16:49:35]

lgtm

[Alexei Svitkine (slow), 2014-07-23 16:58:00]

lgtm

[zel, 2014-07-23 16:59:03]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-07-23 17:01:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zelidrag@chromium.org/329853010/470001

[commit-bot: I haz the power, 2014-07-23 22:29:54]

Change committed as 285065