Over to benwells@ - this sounds like a problem with search geolocation + LSD.

Oh dear. This isn't an LSD thing, this is to do with the search geolocation consistency project, and is about to go to 100% of stable.

Emily - any thoughts? Personally I think this is a blocker.

For now I've rolled back the Finch config pushing search geolocation consistency to 100% of stable.

Emily - please advise. Do you think this should prevent search geolocation consistency going stable?

I think the best solution would be to leave he behavior as is, but to have some kind of backoff on the Update Permissions prompt.

A short term (i.e. possibly mergeable) fix would be to not grant the geolocation permission by default if the chrome app doesn't have permission. This should be a pretty good solution. If the user does do a location sensitive query google should still ask for geolocation and the prompt will show up then.

I'll test this out and see how simple it is.

I've got the short term fix working, and the code is pretty simple but merging might be tricky. Certainly merging to stable will require a manual merge as it will conflict with later changes.

Some more info on whether this should block or not: the current problem already existed, for users that had allowed geolocation for google.com and then turned off android level permission. Or for any other site they had turned on geolocation for - if they turn off android level permission the update permissions prompt would be shown any time it tried to get location.

The consistency change made it worse by giving geolocation by default to all sites.

Thanks for rolling back immediately, I do think that was the best move while we triage.

Few things to confirm:
* It seems reasonable to mirror the behavior of DSE searches in the omnibox. I believe with the omnibox we suppress the dialog when app permission is off. Can you confirm what the behavior was of the omnibox when app permission off? 

* I don't think we have metrics for this... but... any chance we know how many of the "update permission" app prompts we were showing in 1% stable experiment?

Some options:
(1) Have backoff on the update permissions prompt (emabargo-style)
(2) Not grant HTML if app doesnt have permission
(3) Only show Chrome infobar for local queries
(4) For any of the above, we could detect whether we're M+ and only show under those conditions

IMO the best option is to detect whether we're M+ and if so only show "update permission" on a local query, which could be done on the GWS side (see email).

I checked on a few things:

1) I estimate that this will impact 1-4% of Clank users.
* M+ is 35-40% of devices generally (though in current M57 metrics 65% of devices), app permission is missing for 7-8% of M57, ~50% have device location off. 
* Also, from new location permission state metrics in M57, app permission prompt or denied with domain prompt is 1.5% of Omnibox search traffic, so we might assume that's the same for google.com. 

2) I don't think we can specify M+ in the finch config. However it's not unreasonable to hard code a specific Android version but would require stable merge I think.

Ideally, we could hardcode Phase1 for M+ only.
If that doesn't work, I would like to see if Ben's change in crbug/709219c#5 is mergeable, either to 57 or to 58.
If neither of those 2 methods work, I'd like to compare our 1-4% estimate to the current rate google.com shows the HTML infobar. IIRC it's now around 6% (old 30% reduced 5x). If so, it seems like this is still a win for overall DPM.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/235d2d36366b8eefb2f167de9432f546a7752e47

commit 235d2d36366b8eefb2f167de9432f546a7752e47
Author: benwells <benwells@chromium.org>
Date: Fri Apr 07 23:54:16 2017

Don't use the DSE geolocation setting when chrome doesn't have location.

The DSE geolocation setting controls whether the default search engine
should have geolocation access or not, and is on by default. When the
setting is used and chrome does not have permission at the android
level, Chrome will ask the user to give Chrome location access.

What this all means is that if the DSE location setting is used on
a device where Chrome doesn't have geolocation access, the user will
be prompted to give Chrome location access on every google search
(whether via the search results page or the omnibox).

This change prevents this by preventing the DSE geolocation setting from
being used if Chrome doesn't have geolocation access.

BUG= 709219 

Review-Url: https://codereview.chromium.org/2804913005
Cr-Commit-Position: refs/heads/master@{#463057}

[modify] https://crrev.com/235d2d36366b8eefb2f167de9432f546a7752e47/chrome/browser/android/search_geolocation/search_geolocation_service.cc
[modify] https://crrev.com/235d2d36366b8eefb2f167de9432f546a7752e47/chrome/browser/android/search_geolocation/search_geolocation_service.h
[modify] https://crrev.com/235d2d36366b8eefb2f167de9432f546a7752e47/chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc
[modify] https://crrev.com/235d2d36366b8eefb2f167de9432f546a7752e47/chrome/browser/geolocation/geolocation_permission_context_unittest.cc

I've tested the fix on Canary and it looks good.

This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sorry, we need to prioritize M58 stability over everything else and we should not be taking late changes for non-critical issues.  Rejected for 58.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a6e489b65f5ad9dd262520e455c5e339d480e7f

commit 3a6e489b65f5ad9dd262520e455c5e339d480e7f
Author: benwells <benwells@chromium.org>
Date: Wed Apr 12 02:22:40 2017

Revert of Don't use the DSE geolocation setting when chrome doesn't have location. (patchset #3 id:40001 of https://codereview.chromium.org/2804913005/ )

Reason for revert:
We are going with a server side fix instead for this problem. We should still investigate separately some means of backing off the Update Permissions prompt (e.g. with embargo).

Original issue's description:
> Don't use the DSE geolocation setting when chrome doesn't have location.
>
> The DSE geolocation setting controls whether the default search engine
> should have geolocation access or not, and is on by default. When the
> setting is used and chrome does not have permission at the android
> level, Chrome will ask the user to give Chrome location access.
>
> What this all means is that if the DSE location setting is used on
> a device where Chrome doesn't have geolocation access, the user will
> be prompted to give Chrome location access on every google search
> (whether via the search results page or the omnibox).
>
> This change prevents this by preventing the DSE geolocation setting from
> being used if Chrome doesn't have geolocation access.
>
> BUG= 709219 
>
> Review-Url: https://codereview.chromium.org/2804913005
> Cr-Commit-Position: refs/heads/master@{#463057}
> Committed: https://chromium.googlesource.com/chromium/src/+/235d2d36366b8eefb2f167de9432f546a7752e47

TBR=dominickn@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 709219 

Review-Url: https://codereview.chromium.org/2811073003
Cr-Commit-Position: refs/heads/master@{#463901}

[modify] https://crrev.com/3a6e489b65f5ad9dd262520e455c5e339d480e7f/chrome/browser/android/search_geolocation/search_geolocation_service.cc
[modify] https://crrev.com/3a6e489b65f5ad9dd262520e455c5e339d480e7f/chrome/browser/android/search_geolocation/search_geolocation_service.h
[modify] https://crrev.com/3a6e489b65f5ad9dd262520e455c5e339d480e7f/chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc
[modify] https://crrev.com/3a6e489b65f5ad9dd262520e455c5e339d480e7f/chrome/browser/geolocation/geolocation_permission_context_unittest.cc

The plan is now to abandon the fix and have a server side change prevent this from happening too much (i.e. only ask on M+ for local queries). Once we get to M59 this becomes a non-issue due to the changes to the permissions API for the Location Settings Dialog.

Marking this fixed for now as the experiment has been rolled back.