Only show search geolocation disclosure on omnibox searches or API use.

Only show search geolocation disclosure on omnibox searches or API use.

Previously the disclosure was shown on all navigations to the search
engine origin, which can show disclosures when geolocation is not being
used. Now it is only shown for omnibox searches, which send the X-Geo
header, or for the geolocation API use.

BUG=719904
Review-Url: https://codereview.chromium.org/2885763002
Cr-Commit-Position: refs/heads/master@{#472707}
Committed: https://chromium.googlesource.com/chromium/src/+/44b1737f716e05cdf18fd566d011ce9e915416d0

[benwells, 2017-05-16 11:17:12]

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

[commit-bot: I haz the power, 2017-05-16 11:17:35]

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-05-16 12:50:53]

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

[commit-bot: I haz the power, 2017-05-16 12:50:53]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[benwells, 2017-05-17 04:56:43]

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

[commit-bot: I haz the power, 2017-05-17 04:57:38]

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-05-17 05:41:27]

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

[commit-bot: I haz the power, 2017-05-17 05:41:28]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-05-17 07:19:34]

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

[raymes, 2017-05-17 23:56:07]

lg. Is it hard to add a test for this case in
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/SearchGeolocationDisclosureInfoBarTest.java
? 

Thanks!

https://codereview.chromium.org/2885763002/diff/20001/chrome/browser/android/...
File
chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.h
(right):

https://codereview.chromium.org/2885763002/diff/20001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.h:50:
bool ShouldShowDisclosureForAPIAccess(const GURL& gurl);
nit: could you add some comments describing the 2 different cases a little bit
more?

[benwells, 2017-05-18 04:50:05]

I looked at adding the test. To do what I was planning would add significant
plumbing, which would add overhead to non-test chrome. So I think I'll leave it
as is.

https://codereview.chromium.org/2885763002/diff/20001/chrome/browser/android/...
File
chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.h
(right):

https://codereview.chromium.org/2885763002/diff/20001/chrome/browser/android/...
chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.h:50:
bool ShouldShowDisclosureForAPIAccess(const GURL& gurl);
On 2017/05/17 23:56:07, raymes wrote:
> nit: could you add some comments describing the 2 different cases a little bit
> more? 

Done.

[raymes, 2017-05-18 05:22:43]

lgtm

[benwells, 2017-05-18 05:45:03]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2017-05-18 05:46:04]

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-05-18 06:31:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495086303932050,
"parent_rev": "d898eb9cfe0c0d78d3cd0d080894586b034eb1af", "commit_rev":
"44b1737f716e05cdf18fd566d011ce9e915416d0"}

[commit-bot: I haz the power, 2017-05-18 06:31:34]

Description was changed from

==========
Only show search geolocation disclosure on omnibox searches or API use.

Previously the disclosure was shown on all navigations to the search
engine origin, which can show disclosures when geolocation is not being
used. Now it is only shown for omnibox searches, which send the X-Geo
header, or for the geolocation API use.

BUG=719904
==========

to

==========
Only show search geolocation disclosure on omnibox searches or API use.

Previously the disclosure was shown on all navigations to the search
engine origin, which can show disclosures when geolocation is not being
used. Now it is only shown for omnibox searches, which send the X-Geo
header, or for the geolocation API use.

BUG=719904

Review-Url: https://codereview.chromium.org/2885763002
Cr-Commit-Position: refs/heads/master@{#472707}
Committed:
https://chromium.googlesource.com/chromium/src/+/44b1737f716e05cdf18fd566d011...
==========

[commit-bot: I haz the power, 2017-05-18 06:31:36]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/44b1737f716e05cdf18fd566d011...