Add a unittest for SearchGeolocationService

Add a unittest for SearchGeolocationService

This adds a unittest for SearchGeolocationService on android which tests each
of the publicly exposed functions.

BUG=674398
Review-Url: https://codereview.chromium.org/2633103002
Cr-Commit-Position: refs/heads/master@{#445308}
Committed: https://chromium.googlesource.com/chromium/src/+/e9802a6eb32809909a06f79abd8a01fdf5dcd22f

[raymes, 2017-01-18 05:19:03]

Description was changed from

==========
Empty shell of a thing. DO NOT LAND

BUG=

patch from issue 2623033004 at patchset 1 (http://crrev.com/2623033004#ps1)
==========

to

==========
Add a unittest for SearchGeolocationService

This adds a unittest for SearchGeolocationService on android which tests each
of the publicly exposed functions.

BUG=674398
==========

[raymes, 2017-01-18 05:20:27]

raymes@chromium.org changed reviewers:
+ dominickn@chromium.org

[raymes, 2017-01-18 05:20:32]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 05:20:54]

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

[raymes, 2017-01-18 05:22:50]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 05:23:01]

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

[dominickn, 2017-01-18 06:14:11]

Mostly includes cleaning.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
File
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc
(right):

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:5:
#include
"chrome/browser/android/search_geolocation/search_geolocation_service.h"
Nit:

#include <memory>
#include <utility>

#include "base/callback.h"
#include "base/memory/ptr_util.h"

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:7:
#include "base/strings/utf_string_conversions.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:10:
#include "chrome/browser/search_engines/template_url_service_factory.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:11:
#include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:13:
#include "chrome/common/pref_names.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:14:
#include "chrome/test/base/search_test_utils.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:17:
#include "components/content_settings/core/common/pref_names.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:19:
#include "components/search_engines/template_url_service.h"
Not used?

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:76:
std::unique_ptr<TestSearchEngineDelegate> test_delegate =
auto would make this shorter.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:86:
test_delegate_ = nullptr;
This test_delegate_ line looks out of place.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:89:
TestSearchEngineDelegate* test_delegate() { return test_delegate_; }
Either format this like profile() or format profile() like this.

[commit-bot: I haz the power, 2017-01-18 07:07:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-18 07:07:15]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-01-18 23:49:36]

Thanks Dom!

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
File
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc
(right):

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:5:
#include
"chrome/browser/android/search_geolocation/search_geolocation_service.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Nit:
> 
> #include <memory>
> #include <utility>
> 
> #include "base/callback.h"
> #include "base/memory/ptr_util.h"

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:7:
#include "base/strings/utf_string_conversions.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Not used?

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:10:
#include "chrome/browser/search_engines/template_url_service_factory.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Not used?

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:11:
#include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Not used?

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:13:
#include "chrome/common/pref_names.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Not used?

I think this one is used for the pref name

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:14:
#include "chrome/test/base/search_test_utils.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Not used?

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:17:
#include "components/content_settings/core/common/pref_names.h"
This one is used for the content setting pref name.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:19:
#include "components/search_engines/template_url_service.h"
On 2017/01/18 06:14:11, dominickn wrote:
> Not used?

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:76:
std::unique_ptr<TestSearchEngineDelegate> test_delegate =
On 2017/01/18 06:14:11, dominickn wrote:
> auto would make this shorter.

Done.

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:86:
test_delegate_ = nullptr;
On 2017/01/18 06:14:11, dominickn wrote:
> This test_delegate_ line looks out of place.

Whoops!

https://codereview.chromium.org/2633103002/diff/50001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:89:
TestSearchEngineDelegate* test_delegate() { return test_delegate_; }
On 2017/01/18 06:14:11, dominickn wrote:
> Either format this like profile() or format profile() like this.

This was just what clang format did. If it fits on 1 line it will format it like
this, otherwise it will format it like that. They look consistent now though
because they both fit on a line.

[raymes, 2017-01-18 23:50:48]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[raymes, 2017-01-18 23:51:06]

raymes@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[raymes, 2017-01-18 23:51:06]

+dfalcantara for OWNERS

[commit-bot: I haz the power, 2017-01-18 23:51:33]

Dry run: 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-01-19 00:56:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 00:56:28]

Dry run: This issue passed the CQ dry run.

[gone, 2017-01-20 20:16:06]

Waiting for dom's review, here.

[dominickn, 2017-01-20 21:52:16]

lgtm

[gone, 2017-01-20 21:57:49]

OWNERS lgtm

[raymes, 2017-01-22 23:24:38]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-01-22 23:24:48]

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-01-23 00:55:47]

CQ is committing da patch.

Bot data: {"patchset_id": 70001, "attempt_start_ts": 1485127478370010,
"parent_rev": "028ee351f7f79f2a27c8aef5f463d7dec9a0f3e8", "commit_rev":
"e9802a6eb32809909a06f79abd8a01fdf5dcd22f"}

[commit-bot: I haz the power, 2017-01-23 00:56:24]

Description was changed from

==========
Add a unittest for SearchGeolocationService

This adds a unittest for SearchGeolocationService on android which tests each
of the publicly exposed functions.

BUG=674398
==========

to

==========
Add a unittest for SearchGeolocationService

This adds a unittest for SearchGeolocationService on android which tests each
of the publicly exposed functions.

BUG=674398

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

[commit-bot: I haz the power, 2017-01-23 00:56:25]

Committed patchset #5 (id:70001) as
https://chromium.googlesource.com/chromium/src/+/e9802a6eb32809909a06f79abd8a...