Add UMA to track the extensions (and friends) which have been allowed in

Add UMA to track the extensions (and friends) which have been allowed in
incognito and/or given access to the file:// scheme.

BUG=372047
R=rdevlin.cronin@chromium.org,isherman@chromium.org

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

[Devlin, 2014-05-09 22:45:25]

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

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:30: #include
"extensions/common/extension.h"
extension_set.h

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:265: const ExtensionSet& disabled
= extension_registry_->disabled_extensions();
why store disabled?  Only used on line 268, right?

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:272: int incognito = 0,
not_incognito = 0, file_access = 0, not_file_access = 0;
chrome style: one variable per line.

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:272: int incognito = 0,
not_incognito = 0, file_access = 0, not_file_access = 0;
nit: seems like these are conceptually a size_t.

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:274: for
(ExtensionSet::const_iterator it = enabled.begin();
Do we only care about enabled extensions?

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:289: } else {
nit: no braces around single-line ifs/elses

[not at google - send to devlin, 2014-05-09 22:58:55]

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

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:30: #include
"extensions/common/extension.h"
On 2014/05/09 22:45:25, D Cronin wrote:
> extension_set.h

Done.

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:265: const ExtensionSet& disabled
= extension_registry_->disabled_extensions();
On 2014/05/09 22:45:25, D Cronin wrote:
> why store disabled?  Only used on line 268, right?

harmony. but I can revert.

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:272: int incognito = 0,
not_incognito = 0, file_access = 0, not_file_access = 0;
On 2014/05/09 22:45:25, D Cronin wrote:
> nit: seems like these are conceptually a size_t.

Done, though I'm not totally sure about this one.

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:274: for
(ExtensionSet::const_iterator it = enabled.begin();
On 2014/05/09 22:45:25, D Cronin wrote:
> Do we only care about enabled extensions?

Yeah I was thinking about this. I wasn't entirely certain and it could go either
way (i.e. the data should be fairly similar, just more exaggerated). Ultimately
I'm interested in the number of extensions each startup which the user has opted
into running in incognito mode or file:// URLs, and that means enabled.

https://codereview.chromium.org/273253002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:289: } else {
On 2014/05/09 22:45:25, D Cronin wrote:
> nit: no braces around single-line ifs/elses

haha

If you insist.

[Devlin, 2014-05-09 23:01:12]

lgtm.

https://codereview.chromium.org/273253002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/installed_loader.cc (right):

https://codereview.chromium.org/273253002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:274: size_t not_incognito = 0;
nit of all nits: 0u, not 0, to avoid an implicit cast.

[not at google - send to devlin, 2014-05-09 23:18:57]

https://codereview.chromium.org/273253002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/installed_loader.cc (right):

https://codereview.chromium.org/273253002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:274: size_t not_incognito = 0;
On 2014/05/09 23:01:12, D Cronin wrote:
> nit of all nits: 0u, not 0, to avoid an implicit cast.

Done. sort of. you're right, but right below there is a bunch of counting.
speaking of which, I'm moving it down a bit :)

[Devlin, 2014-05-09 23:29:11]

Not blocked on any comments that aren't in your code, but if you wanted to fix
them while you're there.

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
File chrome/browser/extensions/installed_loader.cc (right):

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:299: if ((*ex)->is_app()) {
nit: I liked your const Extension* e = *iter to save us from writing *ex
everywhere.

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:344: const
extensions::URLOverrides::URLOverrideMap& override_map =
don't need extensions:: prefix.

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:395: if
(Manifest::IsExternalLocation(location)) {
gah.
bool is_external_location = Manifest::IsExternalLocation(location);
%s/Manifest::IsExternalLocation(location)/is_external_location/gc

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:410: if
(!Manifest::IsExternalLocation((*ex)->location()))
location, not ex->location.

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:412: ExtensionActionManager*
extension_action_manager =
move this outside the for-loop, so we don't have to re-get it each extension.

https://codereview.chromium.org/273253002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:428: if ((*ex)->location() !=
Manifest::UNPACKED &&
location, not ex->location.
Also, we already skipped all component extensions and all unpacked extensions on
line 353.

[Ilya Sherman, 2014-05-12 19:25:34]

histograms lgtm

[not at google - send to devlin, 2014-05-12 19:27:02]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-12 19:27:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kalman@chromium.org/273253002/80001

[commit-bot: I haz the power, 2014-05-12 23:03:52]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-13 04:04:25]

Change committed as 270006