Change default mode of extension install verification

Change default mode of extension install verification

Right now the default mode is to make requests against the server unless
the experiment group says not to, and that may be contributing to some
unexpected load we're seeing. So this CL changes it to not do that.

It also adds some histograms to help measure the success rate and
sources of requests to the server.

BUG=335379
R=finnur@chromium.org

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

[Finnur, 2014-01-29 10:35:41]

Working into the night, are we? :)

Anyway... LGTM, with a couple of nits.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_service.cc (right):

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service.cc:548: VerifyAllExtensions(true /*
bootstrap */);
nit: I thought we generally shied away from multi-line comments? 
Can you change this to:
VerifyAllExtensions(true);  // bootstrap=true.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service.cc:585:
VERIFY_ALL_BOOTSTRAP_SUCCESS,
Nit: We're not consistent on this throughout Chrome, but I tend to explicitly
give the first enum the value of 0. I read somewhere that it is up to the
compiler what values it uses if the developer doesn't specify it and that could
be bad for the histograms if it picked different values on different
platforms/used negative numbers. Anyway, up to you.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service.cc:2217: }
nit: I would probably prefer to group these utility functions along with the
rest (in the anonymous namespace at the top), instead of interspersed between
member functions. Same goes for LogVerifyAllSuccessHistogram and the enum above
it.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/install_signer.cc (right):

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/install_signer.cc:273: }
nit: Single line, no braces.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/install_signer.cc:360: response = std::string();
nit: response.clear() ?

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:742:
extension_service_->VerifyAllExtensions(false /*bootstrap*/);
nit: Ditto.

[asargent_no_longer_on_chrome, 2014-01-29 19:52:51]

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_service.cc (right):

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service.cc:548: VerifyAllExtensions(true /*
bootstrap */);
On 2014/01/29 10:35:42, Finnur wrote:
> nit: I thought we generally shied away from multi-line comments? 
> Can you change this to:
> VerifyAllExtensions(true);  // bootstrap=true.

Done.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service.cc:585:
VERIFY_ALL_BOOTSTRAP_SUCCESS,
On 2014/01/29 10:35:42, Finnur wrote:
> Nit: We're not consistent on this throughout Chrome, but I tend to explicitly
> give the first enum the value of 0. I read somewhere that it is up to the
> compiler what values it uses if the developer doesn't specify it and that
could
> be bad for the histograms if it picked different values on different
> platforms/used negative numbers. Anyway, up to you.

Good advice, done.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service.cc:2217: }
On 2014/01/29 10:35:42, Finnur wrote:
> nit: I would probably prefer to group these utility functions along with the
> rest (in the anonymous namespace at the top), instead of interspersed between
> member functions. Same goes for LogVerifyAllSuccessHistogram and the enum
above
> it. 

Done.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/install_signer.cc (right):

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/install_signer.cc:273: }
On 2014/01/29 10:35:42, Finnur wrote:
> nit: Single line, no braces.

Done.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/install_signer.cc:360: response = std::string();
On 2014/01/29 10:35:42, Finnur wrote:
> nit: response.clear() ?

Ah, that's better, thanks.

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://codereview.chromium.org/149353002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:742:
extension_service_->VerifyAllExtensions(false /*bootstrap*/);
On 2014/01/29 10:35:42, Finnur wrote:
> nit: Ditto.

Done.

[commit-bot: I haz the power, 2014-01-29 19:54:00]

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

[commit-bot: I haz the power, 2014-01-29 20:55:05]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2014-01-29 22:51:46]

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

[commit-bot: I haz the power, 2014-01-29 23:53:54]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[asargent_no_longer_on_chrome, 2014-01-30 00:57:32]

Committed patchset #4 manually as r247790 (presubmit successful).