Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(12)

Issue 2811073003: Revert of Don't use the DSE geolocation setting when chrome doesn't have location. (Closed)

Created:
3 years, 8 months ago by benwells
Modified:
3 years, 8 months ago
Reviewers:
dominickn
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/chromium/src/+/3a6e489b65f5ad9dd262520e455c5e339d480e7f

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -26 lines) Patch
M chrome/browser/android/search_geolocation/search_geolocation_service.h View 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/android/search_geolocation/search_geolocation_service.cc View 4 chunks +1 line, -11 lines 0 comments Download
M chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc View 3 chunks +1 line, -9 lines 0 comments Download
M chrome/browser/geolocation/geolocation_permission_context_unittest.cc View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
benwells
Created Revert of Don't use the DSE geolocation setting when chrome doesn't have location.
3 years, 8 months ago (2017-04-12 01:08:14 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2811073003/1
3 years, 8 months ago (2017-04-12 01:08:47 UTC) #3
dominickn
lgtm
3 years, 8 months ago (2017-04-12 01:14:41 UTC) #4
commit-bot: I haz the power
3 years, 8 months ago (2017-04-12 02:23:37 UTC) #7
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/3a6e489b65f5ad9dd262520e455c...

Powered by Google App Engine
This is Rietveld 408576698