Add a commandline flag to chrome to validate crx files

Add a commandline flag to chrome to validate crx files

BUG=222215
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191186

[asargent_no_longer_on_chrome, 2013-03-20 17:43:47]

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/startup_helper.cc (right):

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/startup_helper.cc:191: if (success)
Just using LOG(ERROR) here to report both success and failure feels pretty lame,
but I didn't see any convenient way of doing something better. Let me know if
you have any ideas.

[Matt Perry, 2013-03-20 19:52:15]

Code LGTM, but I have some questions.

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/startup_helper.cc (right):

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/startup_helper.cc:191: if (success)
On 2013/03/20 17:43:47, Antony Sargent wrote:
> Just using LOG(ERROR) here to report both success and failure feels pretty
lame,
> but I didn't see any convenient way of doing something better. Let me know if
> you have any ideas. 

This is intended to be used as an automated tool by the webstore, right? It
seems like it'll be ugly for them to parse through log output to find the
success/error code. Maybe they have a better idea of a technique that would be
good for them. Passing in a named pipe filename, for example.

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/startup_helper.h (right):

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/startup_helper.h:33: // it be installed? Returns true
if the crx is valid, or false otherwise.
The code just unpacks it. That doesn't necessarily mean it can be installed,
right? I forget how much error checking we do at unpack time. Do we check for
valid pemissions, etc?

Also, should we report warnings? Like "The bla permission is not available in
the stable channel." ?

[asargent_no_longer_on_chrome, 2013-03-21 21:34:44]

sky - please review:
chrome/browser/ui/startup/startup_browser_creator.cc

FYI, the reason for this feature is to help the webstore team validate that the
.crx files they generate are valid and installable in chrome, since we don't
share validation code (we've discussed ways we might do this in the future, but
for now adding a validation mode in chrome is a small step towards making this
better).

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/startup_helper.cc (right):

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/startup_helper.cc:191: if (success)
As we discussed offline, I've switched this to just printing success/failure to
stdout so they can easily filter out any unrelated LOG(ERROR) cruft that goes to
stderr.

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
File chrome/browser/extensions/startup_helper.h (right):

https://codereview.chromium.org/12660017/diff/1001/chrome/browser/extensions/...
chrome/browser/extensions/startup_helper.h:33: // it be installed? Returns true
if the crx is valid, or false otherwise.
As discussed offline, I think most 'error' checks have been done by this point,
and the idea about optional output of warnings is a great idea for future
enhancement.

For now I'm leaving this as-is since getting this working in the short term is
helpful for the webstore folks.

[sky, 2013-03-26 23:16:10]

printf->LOG and LGTM

https://codereview.chromium.org/12660017/diff/14002/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/12660017/diff/14002/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator.cc:559: printf("%s\n",
message.c_str());
LOG?

[asargent_no_longer_on_chrome, 2013-03-26 23:26:58]

On 2013/03/26 23:16:10, sky wrote:
> printf->LOG and LGTM
> 
>
https://codereview.chromium.org/12660017/diff/14002/chrome/browser/ui/startup...
> File chrome/browser/ui/startup/startup_browser_creator.cc (right):
> 
>
https://codereview.chromium.org/12660017/diff/14002/chrome/browser/ui/startup...
> chrome/browser/ui/startup/startup_browser_creator.cc:559: printf("%s\n",
> message.c_str());
> LOG?

Actually the idea here is that this command line flag can be used to run chrome
via a script that discards any extraneous error log output and just returns
whether the crx file was valid or not. E.g.

RESULT=$(chrome --validate-crx=/path/to/file.crx 2>/dev/null)

So that's why I'm using printf to print to STDOUT instead of LOG'ing to STDERR.

[sky, 2013-03-26 23:59:20]

Does that work on windows?


On Tue, Mar 26, 2013 at 4:26 PM, <asargent@chromium.org> wrote:

> On 2013/03/26 23:16:10, sky wrote:
>
>> printf->LOG and LGTM
>>
>
>
> https://codereview.chromium.**org/12660017/diff/14002/**
>
chrome/browser/ui/startup/**startup_browser_creator.cc<https://codereview.chromium.org/12660017/diff/14002/chrome/browser/ui/startup/startup_browser_creator.cc>
>
>> File chrome/browser/ui/startup/**startup_browser_creator.cc (right):
>>
>
>
> https://codereview.chromium.**org/12660017/diff/14002/**
>
chrome/browser/ui/startup/**startup_browser_creator.cc#**newcode559<https://codereview.chromium.org/12660017/diff/14002/chrome/browser/ui/startup/startup_browser_creator.cc#newcode559>
>
>> chrome/browser/ui/startup/**startup_browser_creator.cc:**559:
>> printf("%s\n",
>> message.c_str());
>> LOG?
>>
>
> Actually the idea here is that this command line flag can be used to run
> chrome
> via a script that discards any extraneous error log output and just returns
> whether the crx file was valid or not. E.g.
>
> RESULT=$(chrome --validate-crx=/path/to/file.**crx 2>/dev/null)
>
> So that's why I'm using printf to print to STDOUT instead of LOG'ing to
> STDERR.
>
>
>
>
>
>
https://codereview.chromium.**org/12660017/<https://codereview.chromium.org/1...
>

[asargent_no_longer_on_chrome, 2013-03-27 19:45:55]

On 2013/03/26 23:59:20, sky wrote:
> Does that work on windows?

Yes, you just have to either run from within a cygwin terminal, or pass the
--enable-logging flag if running from cmd.exe.

[sky, 2013-03-28 15:37:38]

LGTM

[commit-bot: I haz the power, 2013-03-28 16:57:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asargent@chromium.org/12660017/14002

[commit-bot: I haz the power, 2013-03-28 18:17:25]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-03-28 18:43:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asargent@chromium.org/12660017/14002

[commit-bot: I haz the power, 2013-03-28 19:26:39]

Change committed as 191186