Move logic to clear ExtensionAction values to ExtensionActionAPI

Move logic to clear ExtensionAction values to ExtensionActionAPI

Since we are changing where page actions will show up, it makes sense to not
rely on PageActionController (which is attached to the location bar) to clear
the page action values. Since we also have to clear browser action values, it
makes sense to combine these in the ExtensionActionAPI.

Cherries on top:
- Remove a duplicate histogram
- Make ExtensionActionManager::Get() use a BrowserContext

TBR=avi@chromium.org (cocoa - add profile.h to use a profile as a browser context)

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

[Devlin, 2014-08-20 22:43:37]

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

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:548:
UMA_HISTOGRAM_COUNTS_100("Extensions.LoadPageAction", page_action_count);
I'm fairly certain that this and PageActionController.ExtensionsWithPageActions
measure the same thing.  Once per profile, they iterate over enabled_extensions
and count up the number of extensions that have a page action.

The advantage to having this here is that we only ever try once (instead of
every navigation).  Plus, it's a lot like the rest of these.  Unfortunately,
there doesn't seem to be any data for it on histograms, which I suspect is
because LoadPageAction didn't have an owner.  To keep our existing data, keep
the other name.

[Finnur, 2014-08-21 11:04:21]

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

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:548:
UMA_HISTOGRAM_COUNTS_100("Extensions.LoadPageAction", page_action_count);
I suspect they don't measure the same thing. We wanted to know how common it is
to have more than one page action show up at the same time for any given
navigation, and if I recall correctly ExtensionsWithPageActions is the one
measuring that...

Logging it here would defeat the purpose of that metric.

[Devlin, 2014-08-21 14:05:44]

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

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:548:
UMA_HISTOGRAM_COUNTS_100("Extensions.LoadPageAction", page_action_count);
On 2014/08/21 11:04:21, Finnur wrote:
> I suspect they don't measure the same thing. We wanted to know how common it
is
> to have more than one page action show up at the same time for any given
> navigation, and if I recall correctly ExtensionsWithPageActions is the one
> measuring that...
> 
> Logging it here would defeat the purpose of that metric. 

That *would* be a good metric, but that's not what we're logging with
PageActionController.ExtensionsWithPageActions.  More comments in the other
file. :)

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/page_action_controller.cc (left):

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/page_action_controller.cc:86: if
(!g_reported_for_profiles.Get().count(profile)) {
From Other Comment:
"We wanted to know how common it is to have more than one page action show up at
the same time for any given navigation"

So, here we only log once per profile - which already means that we will only
log for a single page navigation (probably the new tab page, statistically). 
But, more importantly, we count an extension if we can GetActionForExtension(),
but that just forwards to ExtensionActionManager->GetPageAction(), which is
exactly what we use to check for LoadPageActions in InstalledLoader (and is
simply indicative of if an extension has a page action declared).

So...we just log the number of extensions with page actions for the given
profile.

That said, the metric you proposed sounds much better (hence the TODO I added
here awhile back ;))  I could probably make that metric and take this one out in
this patch set - SGTY?

[Finnur, 2014-08-21 14:19:19]

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

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:548:
UMA_HISTOGRAM_COUNTS_100("Extensions.LoadPageAction", page_action_count);
Yeah, after digging some more I realize this isn't the metric I was referring
to. I suspect what happened was that we saw how uncommon page actions were in
the grand scheme of things and stopped worrying about many page actions showing
up at the same time.

But that leaves the question: Why not keep Extensions.LoadPageAction instead of
PageActionController.ExtensionsWithPageActions?

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/page_action_controller.cc (left):

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/page_action_controller.cc:86: if
(!g_reported_for_profiles.Get().count(profile)) {
If that means killing this one and leaving the other, then yeah -- go for it.

[Devlin, 2014-08-21 15:23:02]

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

https://codereview.chromium.org/482993003/diff/20001/chrome/browser/extension...
chrome/browser/extensions/installed_loader.cc:548:
UMA_HISTOGRAM_COUNTS_100("Extensions.LoadPageAction", page_action_count);
On 2014/08/21 14:19:18, Finnur wrote:
> Yeah, after digging some more I realize this isn't the metric I was referring
> to. I suspect what happened was that we saw how uncommon page actions were in
> the grand scheme of things and stopped worrying about many page actions
showing
> up at the same time.
> 
> But that leaves the question: Why not keep Extensions.LoadPageAction instead
of
> PageActionController.ExtensionsWithPageActions?

We don't seem to have data for Extensions.LoadPageAction on the histograms site.
 I suspect this may be because it has no owners listed, and the metrics folks
have been threatening to do away with unowned ones for... forever.  So I figured
keep the one that we have data for.  If we don't really care about the existing
data, we can add an owner to LoadPageAction, but it'll mean starting from
scratch.

[Finnur, 2014-08-21 15:39:29]

OK, fine. LGTM.

[Devlin, 2014-08-21 20:52:51]

Mark, mind taking a quick look at histograms.xml?  (Just removing an unneeded
entry.)

[Mark P, 2014-08-21 20:55:19]

https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:7669: -<histogram
name="Extensions.LoadPageAction">
Was this histogram ever recorded?  If so, please mark it as obsolete.

[Devlin, 2014-08-21 21:12:29]

https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:7669: -<histogram
name="Extensions.LoadPageAction">
On 2014/08/21 20:55:18, Mark P wrote:
> Was this histogram ever recorded?  If so, please mark it as obsolete.

I believe it may have been recorded at one point, but it seems there is no
longer data available for it on the histogram site.  I originally chalked this
up to not having an owner - is there another reason the data wouldn't show up? 
(Do you know if the site's down?)

Reason being, if there really is no data, no reason to mark it as obsolete,
right?

[Mark P, 2014-08-21 21:19:38]

lgtm

https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:7669: -<histogram
name="Extensions.LoadPageAction">
On 2014/08/21 21:12:29, Devlin wrote:
> On 2014/08/21 20:55:18, Mark P wrote:
> > Was this histogram ever recorded?  If so, please mark it as obsolete.
> 
> I believe it may have been recorded at one point, but it seems there is no
> longer data available for it on the histogram site.  I originally chalked this
> up to not having an owner - is there another reason the data wouldn't show up?

> (Do you know if the site's down?)
> 
> Reason being, if there really is no data, no reason to mark it as obsolete,
> right?

Right.  It appears to have never been recorded, ever.

[Devlin, 2014-08-21 21:21:33]

On 2014/08/21 21:19:38, Mark P wrote:
> lgtm
> 
>
https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
> File tools/metrics/histograms/histograms.xml (left):
> 
>
https://codereview.chromium.org/482993003/diff/40001/tools/metrics/histograms...
> tools/metrics/histograms/histograms.xml:7669: -<histogram
> name="Extensions.LoadPageAction">
> On 2014/08/21 21:12:29, Devlin wrote:
> > On 2014/08/21 20:55:18, Mark P wrote:
> > > Was this histogram ever recorded?  If so, please mark it as obsolete.
> > 
> > I believe it may have been recorded at one point, but it seems there is no
> > longer data available for it on the histogram site.  I originally chalked
this
> > up to not having an owner - is there another reason the data wouldn't show
up?
> 
> > (Do you know if the site's down?)
> > 
> > Reason being, if there really is no data, no reason to mark it as obsolete,
> > right?
> 
> Right.  It appears to have never been recorded, ever.

Okay, thanks for checking. :)

[Devlin, 2014-08-22 19:12:17]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-08-22 19:14:30]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/482993003/...

[Devlin, 2014-08-22 19:57:46]

The CQ bit was unchecked by rdevlin.cronin@chromium.org

[Devlin, 2014-08-22 19:59:01]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-08-22 20:00:54]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/482993003/...

[Avi (use Gerrit), 2014-08-22 20:23:47]

I assume you added me for chrome/browser/ui/cocoa. LGTM

[commit-bot: I haz the power, 2014-08-22 20:27:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Devlin, 2014-08-22 20:27:49]

On 2014/08/22 20:23:47, Avi wrote:
> I assume you added me for chrome/browser/ui/cocoa. LGTM

Yep - tbr'd in the description since it was a one-liner include-what-you-use,
but thanks for the explicit lg :)

[commit-bot: I haz the power, 2014-08-22 21:38:27]

Committed patchset #4 (80001) as 291508