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

Issue 2791623004: [sensors][permission] Add new permission types in permission module.

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 3 weeks ago by riju_
Modified:
1 month, 2 weeks ago
CC:
Aaron Boodman, abarth-chromium, android-webview-reviews_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, dglazkov+blink, haraken, jam, Mikhail, mlamouri+watch-permissions_chromium.org, mlamouri+watch-blink_chromium.org, qsr+mojo_chromium.org, raymes+watch_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, shalamov, Reilly Grant (use Gerrit)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[sensors][permission] Add new permission types in permission module. This is the first part of adding permission guard for sensors based on Generic Sensor Framework. There was consensus to add granular permissions for sensors here https://github.com/w3c/sensors/issues/22 This CL adds permission name for the different sensors in the permission module. Spec discussion : https://github.com/w3c/permissions/pull/142 BUG=606766

Patch Set 1 #

Total comments: 1

Patch Set 2 : Add Layout tests for the newly added permission types #

Patch Set 3 : Add runtime flag check for sensor permissions #

Patch Set 4 : Remove orientation-sensor permission string. #

Total comments: 1

Patch Set 5 : Fix mounir's comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -1 line) Patch
M android_webview/browser/aw_permission_manager.cc View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/permissions/permission_manager.cc View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
M content/browser/permissions/permission_service_impl.cc View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M content/public/browser/permission_type.h View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/permissions/resources/test-query.js View 1 2 3 1 chunk +36 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/permissions/PermissionDescriptor.idl View 1 2 3 1 chunk +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/permissions/Permissions.cpp View 1 2 3 4 2 chunks +18 lines, -0 lines 0 comments Download
M third_party/WebKit/public/platform/modules/permissions/permission.mojom View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 76 (50 generated)
riju_
Mounir : For some context, this is the WIP CL (https://codereview.chromium.org/2458453002/) for some overall picture. ...
2 months, 2 weeks ago (2017-04-03 06:37:38 UTC) #9
timvolodine
A comment/question below. Also, do we need all the permission types? I understood from my ...
2 months, 2 weeks ago (2017-04-03 16:57:17 UTC) #10
riju_
On 2017/04/03 16:57:17, timvolodine wrote: > A comment/question below. Also, do we need all the ...
2 months, 2 weeks ago (2017-04-03 18:01:59 UTC) #11
riju_
Adding @raymes, @benwells from the chrome privacy team, to make sure we are all in ...
2 months, 2 weeks ago (2017-04-03 18:04:32 UTC) #13
benwells
A couple of comments: - I think the description of this CL is misleading. Can ...
2 months, 2 weeks ago (2017-04-04 00:49:37 UTC) #14
riju_
Thanks for the comments ! On 2017/04/04 00:49:37, benwells wrote: > A couple of comments: ...
2 months, 2 weeks ago (2017-04-04 10:52:14 UTC) #16
mlamouri (slow)
Should LayoutTests be added?
2 months, 2 weeks ago (2017-04-04 12:54:26 UTC) #17
riju_
On 2017/04/04 12:54:26, mlamouri wrote: > Should LayoutTests be added? I added some basic tests ...
2 months, 2 weeks ago (2017-04-04 15:12:02 UTC) #20
mlamouri (slow)
sgtm but isn't this exposing the feature to the web?
2 months, 2 weeks ago (2017-04-05 13:17:22 UTC) #23
riju_
On 2017/04/05 13:17:22, mlamouri wrote: > sgtm but isn't this exposing the feature to the ...
2 months, 2 weeks ago (2017-04-05 13:33:01 UTC) #24
mlamouri (slow)
On 2017/04/05 at 13:33:01, rijubrata.bhaumik wrote: > On 2017/04/05 13:17:22, mlamouri wrote: > > sgtm ...
2 months, 2 weeks ago (2017-04-05 16:11:31 UTC) #25
riju_
On 2017/04/05 16:11:31, mlamouri wrote: > On 2017/04/05 at 13:33:01, rijubrata.bhaumik wrote: > > On ...
2 months, 2 weeks ago (2017-04-05 18:01:37 UTC) #26
riju_
On 2017/04/05 18:01:37, riju_ wrote: > On 2017/04/05 16:11:31, mlamouri wrote: > > On 2017/04/05 ...
2 months, 2 weeks ago (2017-04-05 18:03:59 UTC) #27
riju_
tsepez@ : for permission.mojom file.
2 months, 2 weeks ago (2017-04-05 18:06:53 UTC) #29
Tom Sepez
lgtm
2 months, 2 weeks ago (2017-04-05 18:22:38 UTC) #30
riju_
@raymes: Can you take a look please.
2 months, 2 weeks ago (2017-04-05 21:16:04 UTC) #31
raymes
Hey riju, I'm not sure if benwells/timvolodine questions have been addressed sufficiently. Could you add ...
2 months, 2 weeks ago (2017-04-06 05:32:55 UTC) #32
shalamov
On 2017/04/06 05:32:55, raymes wrote: > Hey riju, I'm not sure if benwells/timvolodine questions have ...
2 months, 2 weeks ago (2017-04-07 07:37:30 UTC) #33
mlamouri (slow)
On 2017/04/07 at 07:37:30, alexander.shalamov wrote: > @mlamouri > > Do you have any ideas ...
2 months, 2 weeks ago (2017-04-07 11:28:26 UTC) #34
Mikhail
raymes@, we have updated the design doc with the permissions considerations (https://docs.google.com/document/d/1Ml65ZdW5AgIsZTszk4mD_ohr40pcrdVFOIf0ZtWxDv0). After we got ...
2 months, 1 week ago (2017-04-13 11:59:30 UTC) #60
riju_
@raymes: PR for the permission spec for this corresponding change has now landed. (https://github.com/w3c/permissions/commit/e9623d75dad47a992597848e86607fa39c1e32d7) We ...
2 months ago (2017-04-19 10:20:22 UTC) #66
raymes
+owencm who is PM of this feature for Chrome On 2017/04/19 10:20:22, riju_ wrote: > ...
2 months ago (2017-04-20 03:20:52 UTC) #68
blink-reviews
Expanding a little, there's a reasonable chance Chrome may want to explicitly differentiate between a ...
2 months ago (2017-04-20 03:53:10 UTC) #69
chromium-reviews
Expanding a little, there's a reasonable chance Chrome may want to explicitly differentiate between a ...
2 months ago (2017-04-20 03:53:10 UTC) #70
mlamouri (slow)
lgtm assuming raymes@ is happy with it https://codereview.chromium.org/2791623004/diff/160001/third_party/WebKit/Source/modules/permissions/Permissions.cpp File third_party/WebKit/Source/modules/permissions/Permissions.cpp (right): https://codereview.chromium.org/2791623004/diff/160001/third_party/WebKit/Source/modules/permissions/Permissions.cpp#newcode106 third_party/WebKit/Source/modules/permissions/Permissions.cpp:106: IsSensorRuntimeFlagEnabled(exception_state)) Instead ...
2 months ago (2017-04-20 13:26:32 UTC) #71
riju_
1 month, 2 weeks ago (2017-05-03 13:43:22 UTC) #76
@raymes, @owen: After getting to know the concerns from Privacy team, regarding
low and high resolution modes, we created a Github issue
(https://github.com/w3c/sensors/issues/183) summarizing what it might mean.
The opinion of our team is listed here:
https://github.com/w3c/sensors/issues/183#issuecomment-297341213
We would like some input from the Chrome privacy team regarding this, in the GH
issue.
Sign in to reply to this message.

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