Fixed minor bug with Search.DefaultSearchChangeOrigin histogram

Fixed minor bug with Search.DefaultSearchChangeOrigin histogram

Looking at TemplateURLService code I discovered that
ApplyDefaultSearchChangeNoMetrics function has minor bug in comparison
of search engines.

BUG=711608
R=pkasting@chromium.org,vasilii@chromium.org

Review-Url: https://codereview.chromium.org/2810233005
Cr-Commit-Position: refs/heads/master@{#464706}
Committed: https://chromium.googlesource.com/chromium/src/+/ef5c7c7902dc7cabb27c1c0149bf725821ce6dce

[Alexander Yashkin, 2017-04-14 07:51:50]

I have created fix for this bug, but I wonder if
Search.DefaultSearchChangeOrigin must be sent at all until TemplateURLService is
loaded?

[Peter Kasting, 2017-04-14 08:02:00]

LGTM

This is worth doing for the dsp_change_callback_ issue I note below.

https://codereview.chromium.org/2810233005/diff/1/chrome/browser/search_engin...
File chrome/browser/search_engines/template_url_service_unittest.cc (right):

https://codereview.chromium.org/2810233005/diff/1/chrome/browser/search_engin...
chrome/browser/search_engines/template_url_service_unittest.cc:1950: // emitted
when TemplateURLService is not yet loaded.
Another way of testing would be to verify that
TemplateURLService::dsp_change_callback_ is run in this case.  This is probably
more important to get right than the histogram, though testing it might be
annoying.

[Alexander Yashkin, 2017-04-14 08:10:31]

On 2017/04/14 at 08:02:00, pkasting wrote:
> LGTM
> 
> This is worth doing for the dsp_change_callback_ issue I note below.
> 
>
https://codereview.chromium.org/2810233005/diff/1/chrome/browser/search_engin...
> File chrome/browser/search_engines/template_url_service_unittest.cc (right):
> 
>
https://codereview.chromium.org/2810233005/diff/1/chrome/browser/search_engin...
> chrome/browser/search_engines/template_url_service_unittest.cc:1950: //
emitted when TemplateURLService is not yet loaded.
> Another way of testing would be to verify that
TemplateURLService::dsp_change_callback_ is run in this case.  This is probably
more important to get right than the histogram, though testing it might be
annoying.

I had same thoughts first, yet decided that testing through histogram is much
easier :)
dsp_change_callback_ is also used for statistics only, as far as looked at.
https://cs.chromium.org/chromium/src/chrome/browser/search_engines/template_u...

[Alexander Yashkin, 2017-04-14 08:11:43]

The CQ bit was checked by a-v-y@yandex-team.ru

[commit-bot: I haz the power, 2017-04-14 08:11:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 09:06:43]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1492157503920110, "parent_rev":
"fc7300a1028fff4c528fa32f1529a3e8cbb1998f", "commit_rev":
"ef5c7c7902dc7cabb27c1c0149bf725821ce6dce"}

[commit-bot: I haz the power, 2017-04-14 09:07:31]

Description was changed from

==========
Fixed minor bug with Search.DefaultSearchChangeOrigin histogram

Looking at TemplateURLService code I discovered that
ApplyDefaultSearchChangeNoMetrics function has minor bug in comparison
of search engines.

BUG=711608
R=pkasting@chromium.org,vasilii@chromium.org
==========

to

==========
Fixed minor bug with Search.DefaultSearchChangeOrigin histogram

Looking at TemplateURLService code I discovered that
ApplyDefaultSearchChangeNoMetrics function has minor bug in comparison
of search engines.

BUG=711608
R=pkasting@chromium.org,vasilii@chromium.org

Review-Url: https://codereview.chromium.org/2810233005
Cr-Commit-Position: refs/heads/master@{#464706}
Committed:
https://chromium.googlesource.com/chromium/src/+/ef5c7c7902dc7cabb27c1c0149bf...
==========

[commit-bot: I haz the power, 2017-04-14 09:07:32]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/ef5c7c7902dc7cabb27c1c0149bf...