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

Issue 2823883004: [Extensions Bindings] Add a native type for ContentSettings (Closed)

Created:
3 years, 8 months ago by Devlin
Modified:
3 years, 8 months ago
Reviewers:
lazyboy, jbroman
CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Extensions Bindings] Add a native type for ContentSettings The ContentSettings allows extensions to modify the content settings (e.g. javascript execution, microphone access, etc) for different sites. It is implemented by exposing "ContentSetting" objects on the API. Implement a native type for these, and hook it into the bindings system. Add tests for the same. BUG=653596 Review-Url: https://codereview.chromium.org/2823883004 Cr-Commit-Position: refs/heads/master@{#466744} Committed: https://chromium.googlesource.com/chromium/src/+/c15ac888ad840b483c5abf18863fb48f090386e4

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Patch Set 4 : . #

Total comments: 23

Patch Set 5 : jbroman, lazyboy #

Total comments: 11

Patch Set 6 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+349 lines, -1 line) Patch
M chrome/common/extensions/api/content_settings.json View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/data/extensions/api_test/native_bindings/extension/background.js View 1 2 3 4 1 chunk +68 lines, -0 lines 0 comments Download
M chrome/test/data/extensions/api_test/native_bindings/extension/manifest.json View 1 chunk +1 line, -1 line 0 comments Download
M extensions/renderer/BUILD.gn View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A extensions/renderer/content_setting.h View 1 2 3 4 1 chunk +85 lines, -0 lines 0 comments Download
A extensions/renderer/content_setting.cc View 1 2 3 4 5 1 chunk +188 lines, -0 lines 0 comments Download
M extensions/renderer/native_extension_bindings_system.cc View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 29 (19 generated)
Devlin
Hey folks, mind taking a quick look?
3 years, 8 months ago (2017-04-20 18:21:15 UTC) #11
lazyboy
https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/BUILD.gn File extensions/renderer/BUILD.gn (right): https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/BUILD.gn#newcode59 extensions/renderer/BUILD.gn:59: "content_setting.cc", optional: maybe consider naming all custom type files ...
3 years, 8 months ago (2017-04-20 19:29:50 UTC) #13
jbroman
https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/content_setting.cc File extensions/renderer/content_setting.cc (right): https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/content_setting.cc#newcode23 extensions/renderer/content_setting.cc:23: const char* kDeprecatedTypes[] = { nit: const char* const. ...
3 years, 8 months ago (2017-04-20 19:38:38 UTC) #14
Devlin
https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/BUILD.gn File extensions/renderer/BUILD.gn (right): https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/BUILD.gn#newcode59 extensions/renderer/BUILD.gn:59: "content_setting.cc", On 2017/04/20 19:29:49, lazyboy wrote: > optional: maybe ...
3 years, 8 months ago (2017-04-21 23:33:02 UTC) #15
jbroman
lgtm https://codereview.chromium.org/2823883004/diff/80001/extensions/renderer/content_setting.cc File extensions/renderer/content_setting.cc (right): https://codereview.chromium.org/2823883004/diff/80001/extensions/renderer/content_setting.cc#newcode38 extensions/renderer/content_setting.cc:38: v8::Local<v8::Context> context, nit: If you only want the ...
3 years, 8 months ago (2017-04-24 15:44:42 UTC) #16
lazyboy
lgtm https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/BUILD.gn File extensions/renderer/BUILD.gn (right): https://codereview.chromium.org/2823883004/diff/60001/extensions/renderer/BUILD.gn#newcode59 extensions/renderer/BUILD.gn:59: "content_setting.cc", On 2017/04/21 23:33:02, Devlin wrote: > On ...
3 years, 8 months ago (2017-04-24 18:10:54 UTC) #17
Devlin
https://codereview.chromium.org/2823883004/diff/80001/extensions/renderer/content_setting.cc File extensions/renderer/content_setting.cc (right): https://codereview.chromium.org/2823883004/diff/80001/extensions/renderer/content_setting.cc#newcode38 extensions/renderer/content_setting.cc:38: v8::Local<v8::Context> context, On 2017/04/24 15:44:42, jbroman wrote: > nit: ...
3 years, 8 months ago (2017-04-24 20:09:28 UTC) #20
jbroman
https://codereview.chromium.org/2823883004/diff/80001/extensions/renderer/content_setting.cc File extensions/renderer/content_setting.cc (right): https://codereview.chromium.org/2823883004/diff/80001/extensions/renderer/content_setting.cc#newcode38 extensions/renderer/content_setting.cc:38: v8::Local<v8::Context> context, On 2017/04/24 at 20:09:28, Devlin wrote: > ...
3 years, 8 months ago (2017-04-24 20:16:23 UTC) #21
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/2823883004/100001
3 years, 8 months ago (2017-04-24 20:23:48 UTC) #26
commit-bot: I haz the power
3 years, 8 months ago (2017-04-24 20:30:38 UTC) #29
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/c15ac888ad840b483c5abf18863f...

Powered by Google App Engine
This is Rietveld 408576698