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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 22674006: Delete the Windows 7 Geolocation provider. (Closed)

Created:
7 years, 4 months ago by Michael van Ouwerkerk
Modified:
7 years, 4 months ago
Reviewers:
bulach, jam, joth
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, Miguel Garcia
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Delete the Windows 7 Geolocation provider. This experimental feature is hidden behind the experimental-location-features flag and was never fully developed and has very little usage. Please refer to the bug for more details. BUG=134075 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216980

Patch Set 1 #

Total comments: 2

Patch Set 2 : Define default NewSystemLocationProvider. Remove dependency on locationapi.lib. #

Total comments: 2

Patch Set 3 : Move the ifdef. #

Patch Set 4 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -891 lines) Patch
M chrome/browser/about_flags.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/geolocation/location_arbitrator_impl.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
D content/browser/geolocation/win7_location_api_unittest_win.cc View 1 chunk +0 lines, -339 lines 0 comments Download
D content/browser/geolocation/win7_location_api_win.h View 1 chunk +0 lines, -66 lines 0 comments Download
D content/browser/geolocation/win7_location_api_win.cc View 1 chunk +0 lines, -169 lines 0 comments Download
D content/browser/geolocation/win7_location_provider_unittest_win.cc View 1 chunk +0 lines, -147 lines 0 comments Download
D content/browser/geolocation/win7_location_provider_win.h View 1 chunk +0 lines, -50 lines 0 comments Download
D content/browser/geolocation/win7_location_provider_win.cc View 1 chunk +0 lines, -110 lines 0 comments Download
M content/content_browser.gypi View 1 2 3 2 chunks +0 lines, -5 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M content/public/common/content_switches.cc View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Michael van Ouwerkerk
To Marcus and Jonathan: please take a look because you are or were geolocation owners. ...
7 years, 4 months ago (2013-08-09 11:45:29 UTC) #1
bulach
https://chromiumcodereview.appspot.com/22674006/diff/1/content/browser/geolocation/win7_location_provider_win.cc File content/browser/geolocation/win7_location_provider_win.cc (left): https://chromiumcodereview.appspot.com/22674006/diff/1/content/browser/geolocation/win7_location_provider_win.cc#oldcode103 content/browser/geolocation/win7_location_provider_win.cc:103: LocationProvider* NewSystemLocationProvider() { bots are complaining about this, needs ...
7 years, 4 months ago (2013-08-09 13:04:13 UTC) #2
Michael van Ouwerkerk
https://chromiumcodereview.appspot.com/22674006/diff/1/content/browser/geolocation/win7_location_provider_win.cc File content/browser/geolocation/win7_location_provider_win.cc (left): https://chromiumcodereview.appspot.com/22674006/diff/1/content/browser/geolocation/win7_location_provider_win.cc#oldcode103 content/browser/geolocation/win7_location_provider_win.cc:103: LocationProvider* NewSystemLocationProvider() { On 2013/08/09 13:04:13, bulach wrote: > ...
7 years, 4 months ago (2013-08-09 13:05:24 UTC) #3
bulach
crossed the streams. :) lgtm, one suggestion below: https://chromiumcodereview.appspot.com/22674006/diff/10001/content/browser/geolocation/location_arbitrator_impl.cc File content/browser/geolocation/location_arbitrator_impl.cc (right): https://chromiumcodereview.appspot.com/22674006/diff/10001/content/browser/geolocation/location_arbitrator_impl.cc#newcode163 content/browser/geolocation/location_arbitrator_impl.cc:163: return ...
7 years, 4 months ago (2013-08-09 13:06:18 UTC) #4
Michael van Ouwerkerk
https://chromiumcodereview.appspot.com/22674006/diff/10001/content/browser/geolocation/location_arbitrator_impl.cc File content/browser/geolocation/location_arbitrator_impl.cc (right): https://chromiumcodereview.appspot.com/22674006/diff/10001/content/browser/geolocation/location_arbitrator_impl.cc#newcode163 content/browser/geolocation/location_arbitrator_impl.cc:163: return content::NewSystemLocationProvider(); On 2013/08/09 13:06:18, bulach wrote: > perhaps ...
7 years, 4 months ago (2013-08-09 13:27:02 UTC) #5
jam
lgtm
7 years, 4 months ago (2013-08-09 15:53:46 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mvanouwerkerk@chromium.org/22674006/29001
7 years, 4 months ago (2013-08-12 09:41:25 UTC) #7
commit-bot: I haz the power
7 years, 4 months ago (2013-08-12 13:12:43 UTC) #8
Message was sent while issue was closed. 
Change committed as 216980

Powered by Google App Engine
This is Rietveld 408576698