Add back DefaultSearchProviderType UMA metric.

Add back DefaultSearchProviderType UMA metric.

This gets logged once per profile load/open, and records the type of the default search provider at that time.

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

[Cait (Slow), 2014-05-21 19:03:49]

Peter: PTAL. This CL adds back an UMA stat which got deleted during the great
refactoring.

Thanks!

[Mark P, 2014-05-21 19:12:46]

Is the changelist description accurate?  If so, then this is not equivalent
to the old histogram, which gets logged on profile load/open, not merely
once per startup.


On Wed, May 21, 2014 at 12:03 PM, <caitkp@chromium.org> wrote:

> Reviewers: Peter Kasting,
>
> Message:
> Peter: PTAL. This CL adds back an UMA stat which got deleted during the
> great
> refactoring.
>
> Thanks!
>
> Description:
> Add back DefaultSearchProviderType UMA metric.
>
> This gets logged once per chrome run, at load time, and records the type of
> the default search provider at that time.
>
> Please review this at https://codereview.chromium.org/295093007/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+6, -0 lines):
>   M chrome/browser/search_engines/template_url_service.cc
>
>
> Index: chrome/browser/search_engines/template_url_service.cc
> diff --git a/chrome/browser/search_engines/template_url_service.cc
> b/chrome/browser/search_engines/template_url_service.cc
> index bc44c0207813b8c465ddf306f65d08640da25647..
> 1f4a64827f8d058e6948fc51c97c8575582c0a76 100644
> --- a/chrome/browser/search_engines/template_url_service.cc
> +++ b/chrome/browser/search_engines/template_url_service.cc
> @@ -959,6 +959,12 @@ void TemplateURLService::OnWebDataServiceRequestDone(
>
>    if (new_resource_keyword_version)
>      service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
> +
> +  if (default_search_provider_) {
> +    UMA_HISTOGRAM_ENUMERATION("Search.DefaultSearchProviderType",
> +        TemplateURLPrepopulateData::GetEngineType(*default_search_
> provider_),
> +        SEARCH_ENGINE_MAX);
> +  }
>  }
>
>  base::string16 TemplateURLService::GetKeywordShortName(
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Cait (Slow), 2014-05-21 19:19:38]

On 2014/05/21 19:12:46, Mark P wrote:
> Is the changelist description accurate?  If so, then this is not equivalent
> to the old histogram, which gets logged on profile load/open, not merely
> once per startup.
> 

Sorry for the confusion -- you're correct. This gets logged once per
TemplateULRLService load, which happens once per profile load.

I've updated the CL description.

> 
> On Wed, May 21, 2014 at 12:03 PM, <mailto:caitkp@chromium.org> wrote:
> 
> > Reviewers: Peter Kasting,
> >
> > Message:
> > Peter: PTAL. This CL adds back an UMA stat which got deleted during the
> > great
> > refactoring.
> >
> > Thanks!
> >
> > Description:
> > Add back DefaultSearchProviderType UMA metric.
> >
> > This gets logged once per chrome run, at load time, and records the type of
> > the default search provider at that time.
> >
> > Please review this at https://codereview.chromium.org/295093007/
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src
> >
> > Affected files (+6, -0 lines):
> >   M chrome/browser/search_engines/template_url_service.cc
> >
> >
> > Index: chrome/browser/search_engines/template_url_service.cc
> > diff --git a/chrome/browser/search_engines/template_url_service.cc
> > b/chrome/browser/search_engines/template_url_service.cc
> > index bc44c0207813b8c465ddf306f65d08640da25647..
> > 1f4a64827f8d058e6948fc51c97c8575582c0a76 100644
> > --- a/chrome/browser/search_engines/template_url_service.cc
> > +++ b/chrome/browser/search_engines/template_url_service.cc
> > @@ -959,6 +959,12 @@ void TemplateURLService::OnWebDataServiceRequestDone(
> >
> >    if (new_resource_keyword_version)
> >      service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
> > +
> > +  if (default_search_provider_) {
> > +    UMA_HISTOGRAM_ENUMERATION("Search.DefaultSearchProviderType",
> > +        TemplateURLPrepopulateData::GetEngineType(*default_search_
> > provider_),
> > +        SEARCH_ENGINE_MAX);
> > +  }
> >  }
> >
> >  base::string16 TemplateURLService::GetKeywordShortName(
> >
> >
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Mark P, 2014-05-21 19:22:24]

Can you please add histograms.xml to this changelist and edit the
description of the histogram to note that there was a period between
(roughly) version XXX and YYY where this histogram was not logged?

thanks,
mark



On Wed, May 21, 2014 at 12:19 PM, <caitkp@chromium.org> wrote:

> On 2014/05/21 19:12:46, Mark P wrote:
>
>> Is the changelist description accurate?  If so, then this is not
>> equivalent
>> to the old histogram, which gets logged on profile load/open, not merely
>> once per startup.
>>
>
>
> Sorry for the confusion -- you're correct. This gets logged once per
> TemplateULRLService load, which happens once per profile load.
>
> I've updated the CL description.
>
>
>
>  On Wed, May 21, 2014 at 12:03 PM, <mailto:caitkp@chromium.org> wrote:
>>
>
>  > Reviewers: Peter Kasting,
>> >
>> > Message:
>> > Peter: PTAL. This CL adds back an UMA stat which got deleted during the
>> > great
>> > refactoring.
>> >
>> > Thanks!
>> >
>> > Description:
>> > Add back DefaultSearchProviderType UMA metric.
>> >
>> > This gets logged once per chrome run, at load time, and records the
>> type of
>> > the default search provider at that time.
>> >
>> > Please review this at https://codereview.chromium.org/295093007/
>> >
>> > SVN Base: svn://svn.chromium.org/chrome/trunk/src
>> >
>> > Affected files (+6, -0 lines):
>> >   M chrome/browser/search_engines/template_url_service.cc
>> >
>> >
>> > Index: chrome/browser/search_engines/template_url_service.cc
>> > diff --git a/chrome/browser/search_engines/template_url_service.cc
>> > b/chrome/browser/search_engines/template_url_service.cc
>> > index bc44c0207813b8c465ddf306f65d08640da25647..
>> > 1f4a64827f8d058e6948fc51c97c8575582c0a76 100644
>> > --- a/chrome/browser/search_engines/template_url_service.cc
>> > +++ b/chrome/browser/search_engines/template_url_service.cc
>> > @@ -959,6 +959,12 @@ void TemplateURLService::
>> OnWebDataServiceRequestDone(
>> >
>> >    if (new_resource_keyword_version)
>> >      service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
>> > +
>> > +  if (default_search_provider_) {
>> > +    UMA_HISTOGRAM_ENUMERATION("Search.DefaultSearchProviderType",
>> > +        TemplateURLPrepopulateData::GetEngineType(*default_search_
>> > provider_),
>> > +        SEARCH_ENGINE_MAX);
>> > +  }
>> >  }
>> >
>> >  base::string16 TemplateURLService::GetKeywordShortName(
>> >
>> >
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
>
>
>
>
> https://codereview.chromium.org/295093007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Peter Kasting, 2014-05-21 19:25:35]

RSLGTM once Mark is OK with the change

[Cait (Slow), 2014-05-21 19:39:34]

Mark: PTAL. I'll try to get this merged into M36, too.

[Mark P, 2014-05-21 19:43:59]

lgtm

[Cait (Slow), 2014-05-21 19:46:46]

The CQ bit was checked by caitkp@chromium.org

[commit-bot: I haz the power, 2014-05-21 19:58:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/caitkp@chromium.org/295093007/20001

[commit-bot: I haz the power, 2014-05-22 08:07:48]

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:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/10896)

[commit-bot: I haz the power, 2014-05-22 12:34:16]

Change committed as 272169