Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(4)

Issue 1530403002: Removal of geolocation APIs on insecure origins (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by jww
Modified:
1 year, 4 months ago
CC:
android-webview-reviews_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, jam, kcarattini+watch_chromium.org, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-geolocation_chromium.org, mlamouri+watch-permissions_chromium.org, mlamouri+watch-blink_chromium.org, mlamouri+watch-content_chromium.org, Michael van Ouwerkerk, mvanouwerkerk+watch_chromium.org, nasko+codewatch_chromium.org, timvolodine
Base URL:
https://chromium.googlesource.com/chromium/src@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. Note that this is a re-land of https://codereview.chromium.org/1485973002/. See that CL for full discussion. BUG=520765, 561641 TBR=thestig@chromium.org,sgurun@chromium.org,philipj@opera.com,mlamouri@chromium.org Committed: https://crrev.com/9d4ca2d9838b5f33bdb3f8fcfb8ef381d449b2a1 Cr-Commit-Position: refs/heads/master@{#370185}

Patch Set 1 #

Patch Set 2 : Actual WebView fixes #

Patch Set 3 : Rebase on ToT (and minor cleanup) #

Patch Set 4 : Fix unit test #

Total comments: 2

Patch Set 5 : Make message more ambiguous #

Patch Set 6 : Rebase on ToT #

Total comments: 4

Patch Set 7 : Grammar fix #

Total comments: 2

Patch Set 8 : Spelling #

Patch Set 9 : Rebase on ToT #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -84 lines) Patch
M android_webview/javatests/src/org/chromium/android_webview/test/GeolocationTest.java View 6 chunks +12 lines, -12 lines 0 comments Download
M android_webview/native/aw_settings.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/geolocation/geolocation_permission_context.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/geolocation/geolocation_permission_context_unittest.cc View 1 2 3 4 5 13 chunks +32 lines, -20 lines 0 comments Download
M content/public/common/common_param_traits_macros.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -2 lines 0 comments Download
M content/public/common/web_preferences.h View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -2 lines 0 comments Download
M content/public/common/web_preferences.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html View 1 2 3 4 5 6 7 2 chunks +17 lines, -33 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin-expected.txt View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/frame/Settings.in View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/UseCounter.cpp View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/geolocation/Geolocation.cpp View 1 2 3 4 5 3 chunks +5 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/web/WebSettingsImpl.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebSettingsImpl.cpp View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebSettings.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 40 (14 generated)
jww
torne@, I believe this implements the API we need for this to work with WebKit. ...
1 year, 5 months ago (2015-12-17 02:37:50 UTC) #3
jww
On 2015/12/17 02:37:50, jww wrote: > torne@, I believe this implements the API we need ...
1 year, 5 months ago (2015-12-17 03:11:01 UTC) #4
jww
torne@, this is ready to review now. Sorry about the false start yesterday. Let me ...
1 year, 5 months ago (2015-12-17 19:09:02 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530403002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530403002/40001
1 year, 5 months ago (2015-12-17 21:29:41 UTC) #7
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/103639)
1 year, 5 months ago (2015-12-17 22:12:35 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530403002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530403002/60001
1 year, 5 months ago (2015-12-17 23:37:52 UTC) #11
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/138141)
1 year, 5 months ago (2015-12-18 00:11:39 UTC) #13
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530403002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530403002/60001
1 year, 5 months ago (2015-12-18 01:29:06 UTC) #15
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/138213)
1 year, 5 months ago (2015-12-18 01:57:41 UTC) #17
Torne
Sorry, didn't get to this before the holidays. Approach looks okay, but one question: https://codereview.chromium.org/1530403002/diff/60001/third_party/WebKit/Source/core/frame/UseCounter.cpp ...
1 year, 4 months ago (2016-01-05 11:21:23 UTC) #18
jww
https://codereview.chromium.org/1530403002/diff/60001/third_party/WebKit/Source/core/frame/UseCounter.cpp File third_party/WebKit/Source/core/frame/UseCounter.cpp (right): https://codereview.chromium.org/1530403002/diff/60001/third_party/WebKit/Source/core/frame/UseCounter.cpp#newcode880 third_party/WebKit/Source/core/frame/UseCounter.cpp:880: return "getCurrentPosition() and watchPosition() no longer work on insecure ...
1 year, 4 months ago (2016-01-06 02:40:59 UTC) #19
Torne
LGTM. Once this lands we can talk about when we can try unsetting the flag ...
1 year, 4 months ago (2016-01-06 11:59:11 UTC) #20
jww
Hi everyone. I need a few OWNER reviews here. tsepez@chromium.org, would you mind checking out ...
1 year, 4 months ago (2016-01-06 18:57:29 UTC) #22
Tom Sepez
Param traits LGTM.
1 year, 4 months ago (2016-01-06 19:59:51 UTC) #23
Charlie Reis (OOO until 5-30)
content/ LGTM. https://codereview.chromium.org/1530403002/diff/100001/content/public/common/content_switches.cc File content/public/common/content_switches.cc (right): https://codereview.chromium.org/1530403002/diff/100001/content/public/common/content_switches.cc#newcode459 content/public/common/content_switches.cc:459: // Blocks insecure usage of number of ...
1 year, 4 months ago (2016-01-06 21:39:41 UTC) #24
jww
https://codereview.chromium.org/1530403002/diff/100001/content/public/common/content_switches.cc File content/public/common/content_switches.cc (right): https://codereview.chromium.org/1530403002/diff/100001/content/public/common/content_switches.cc#newcode459 content/public/common/content_switches.cc:459: // Blocks insecure usage of number of powerful features ...
1 year, 4 months ago (2016-01-06 23:18:56 UTC) #25
dcheng
https://codereview.chromium.org/1530403002/diff/100001/third_party/WebKit/public/web/WebSettings.h File third_party/WebKit/public/web/WebSettings.h (right): https://codereview.chromium.org/1530403002/diff/100001/third_party/WebKit/public/web/WebSettings.h#newcode113 third_party/WebKit/public/web/WebSettings.h:113: virtual void setAllowGeolocationOnInsecureOrigins(bool) = 0; Are these mainly intended ...
1 year, 4 months ago (2016-01-06 23:42:37 UTC) #26
jww
On 2016/01/06 23:42:37, dcheng wrote: > https://codereview.chromium.org/1530403002/diff/100001/third_party/WebKit/public/web/WebSettings.h > File third_party/WebKit/public/web/WebSettings.h (right): > > https://codereview.chromium.org/1530403002/diff/100001/third_party/WebKit/public/web/WebSettings.h#newcode113 > ...
1 year, 4 months ago (2016-01-07 00:34:53 UTC) #27
dcheng
On 2016/01/07 at 00:34:53, jww wrote: > On 2016/01/06 23:42:37, dcheng wrote: > > https://codereview.chromium.org/1530403002/diff/100001/third_party/WebKit/public/web/WebSettings.h ...
1 year, 4 months ago (2016-01-07 00:39:08 UTC) #28
jww
On 2016/01/07 00:39:08, dcheng wrote: > On 2016/01/07 at 00:34:53, jww wrote: > > On ...
1 year, 4 months ago (2016-01-07 00:45:19 UTC) #29
dcheng
LGTM with previous nit addressed.
1 year, 4 months ago (2016-01-07 00:46:50 UTC) #30
jww
This should address all the comments. However, I'm going to wait to commit this until ...
1 year, 4 months ago (2016-01-07 02:25:40 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530403002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530403002/160001
1 year, 4 months ago (2016-01-19 19:48:06 UTC) #35
jww
On 2016/01/07 02:25:40, jww wrote: > This should address all the comments. However, I'm going ...
1 year, 4 months ago (2016-01-19 19:48:19 UTC) #36
commit-bot: I haz the power
Committed patchset #9 (id:160001)
1 year, 4 months ago (2016-01-19 20:59:08 UTC) #38
commit-bot: I haz the power
1 year, 4 months ago (2016-01-19 21:00:14 UTC) #40
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/9d4ca2d9838b5f33bdb3f8fcfb8ef381d449b2a1
Cr-Commit-Position: refs/heads/master@{#370185}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06