[Extensions] Add an access check before executing native code in the renderer

[Extensions] Add an access check before executing native code in the renderer

Sometimes things can intercept bindings. Check access before doing anything.
This is a first step that just adds it for the test API.

BUG=598165
Committed: https://crrev.com/cbad917fdd9651eb9afd723a81c9d2ab437ce03d
Cr-Commit-Position: refs/heads/master@{#384614}

[Devlin, 2016-03-30 21:58:08]

Patchset #1 (id:1) has been deleted

[Devlin, 2016-03-30 21:59:48]

Description was changed from

==========
testing

BUG=
==========

to

==========
[Extensions] Add an access check before executing native code in the renderer

Sometimes things can intercept bindings. Check access before doing anything.
This is a first step that just adds it for the test API.

BUG=598165
==========

[Devlin, 2016-03-30 22:02:27]

rdevlin.cronin@chromium.org changed reviewers:
+ jochen@chromium.org

[Devlin, 2016-03-30 22:02:28]

See the bug for more context, but I'm curious what you think.

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/obj...
File extensions/renderer/object_backed_native_handler.cc (right):

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/obj...
extensions/renderer/object_backed_native_handler.cc:72:
!script_context->GetAvailability(feature_name).is_available()) {
This check is kind of annoying - ideally we wouldn't have to worry about it. 
But it's gotten to the point where I'm thinking it might be better to just
default to doing it.

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/scr...
File extensions/renderer/script_context.cc (right):

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/scr...
extensions/renderer/script_context.cc:213: if (api_name == "test") {
test api is special; the rest of them should pretty much just fall into the
normal code below.

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/use...
File extensions/renderer/user_gestures_native_handler.cc (right):

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/use...
extensions/renderer/user_gestures_native_handler.cc:17: "test",
Ideally, we'd be able to attribute all the native handlers to a particular API
in order to perform this check.  We can go through and update them piecemeal if
this lgty.

[jochen (gone - plz use gerrit), 2016-03-31 15:13:50]

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/obj...
File extensions/renderer/object_backed_native_handler.cc (right):

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/obj...
extensions/renderer/object_backed_native_handler.cc:64: if
(content::WorkerThread::GetCurrentId() == 0) {
why is this check needed?

[Devlin, 2016-03-31 16:38:08]

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/obj...
File extensions/renderer/object_backed_native_handler.cc (right):

https://codereview.chromium.org/1843803002/diff/20001/extensions/renderer/obj...
extensions/renderer/object_backed_native_handler.cc:64: if
(content::WorkerThread::GetCurrentId() == 0) {
On 2016/03/31 15:13:50, jochen wrote:
> why is this check needed?

There probably should have been a TODO here to figure out a way around this. 
Reason we need it right now is because we can't access ScriptContextSet() on the
worker thread.  Luckily, we also don't inject many bindings on the worker
thread, so this isn't as big of a deal.

I've added a comment/todo to that effect.

[jochen (gone - plz use gerrit), 2016-04-01 08:44:22]

lgtm

[Devlin, 2016-04-01 16:24:40]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2016-04-01 16:24:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1843803002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1843803002/40001

[Devlin, 2016-04-01 16:25:06]

+asargent@, lazyboy@ fyi

[commit-bot: I haz the power, 2016-04-01 17:14:58]

Description was changed from

==========
[Extensions] Add an access check before executing native code in the renderer

Sometimes things can intercept bindings. Check access before doing anything.
This is a first step that just adds it for the test API.

BUG=598165
==========

to

==========
[Extensions] Add an access check before executing native code in the renderer

Sometimes things can intercept bindings. Check access before doing anything.
This is a first step that just adds it for the test API.

BUG=598165
==========

[commit-bot: I haz the power, 2016-04-01 17:15:00]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-04-01 17:15:59]

Description was changed from

==========
[Extensions] Add an access check before executing native code in the renderer

Sometimes things can intercept bindings. Check access before doing anything.
This is a first step that just adds it for the test API.

BUG=598165
==========

to

==========
[Extensions] Add an access check before executing native code in the renderer

Sometimes things can intercept bindings. Check access before doing anything.
This is a first step that just adds it for the test API.

BUG=598165

Committed: https://crrev.com/cbad917fdd9651eb9afd723a81c9d2ab437ce03d
Cr-Commit-Position: refs/heads/master@{#384614}
==========

[commit-bot: I haz the power, 2016-04-01 17:16:00]

Patchset 2 (id:??) landed as
https://crrev.com/cbad917fdd9651eb9afd723a81c9d2ab437ce03d
Cr-Commit-Position: refs/heads/master@{#384614}