New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 723923 link

Starred by 4 users
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: May 2017
Cc:
█ alokp@chromium.org
iclell...@chromium.org
jbroman@chromium.org
cha...@chromium.org
dcheng@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Support per-context feature filtering on Blink IDL

Project Member Reported by roc...@chromium.org, May 18 2017 
As discussed on https://codereview.chromium.org/2400563002/ (in particular see comment #67), there is a need to enable certain global interfaces only within specific execution contexts.

For some concrete use cases in Chrome, we'd like to enable native Mojo bindings (currently behind a test-only RuntimeEnabled feature) as well as a new lightweight ServiceManager API selectively in a few scenarios:

  - For all contexts in layout tests
  - Globally for all contexts, only when a special command line flag is set
  - For all WebUI contexts in Chrome

RuntimeEnabled feature flags are insufficient particularly for the third case.

I would propose that we add something like a ContextEnabled=*, which causes an interface to be omitted from Windows's data attributes, but still generates the necessary code to install the API manually; this can then be plumbed up through some ContextEnabledFeatures/WebContextFeatures static helpers that e.g. content can use like:

  WebContextFeatures::EnableServiceManager(context, true);

Finally, I think we'd need somewhere for content to hook into the creation of every new V8 Context while being aware of the context's SecurityOrigin. The latter data is not because we trust the renderer to make an actual security decision (these interfaces are already exposed natively to the renderer anyway) but so we can avoid exposing non-standard APIs to every open web context when only some will need it, as determined by the embedder.

For WebUI for now, we could continue to get away with using RuntimeEnabled assuming WebUI render processes are never shared with or reused for non-WebUI contexts, but we will have other use cases in the future which require more granular exposure.

Comment 1 by dcheng@chromium.org, May 18 2017 

Sorry, to clarify, is the proposed ContextEnabled= an IDL attribute or a Mojo attribute? It sounds like it would be an IDL attribute, from reading the rest of this.

Comment 2 by roc...@chromium.org, May 18 2017 

IDL attribute

Comment 3 by dcheng@chromium.org, May 18 2017 

If we're calling a method to inject attributes after the global has already been created, can we just do what the layout test runner does for window.internals [1]?

[1] https://cs.chromium.org/chromium/src/content/shell/renderer/layout_test/blink_test_runner.cc?rcl=c4f60b915d897050dac3d7532ff8c1e0139c1821&l=805

Comment 4 by roc...@chromium.org, May 18 2017 

Possibly. I was looking at something like that (actually RenderFrameImpl::DidCreateScriptContext) but I'm not sure it's sufficient. Both of these things seem to be limited to main-frame contexts. That's fine for WebUI and at least most layout test use cases, but I don't think it's enough.

We'll want access to such features in worker contexts for example.

Comment 5 by dcheng@chromium.org, May 18 2017

Components: Blink>Bindings
We have https://cs.chromium.org/chromium/src/content/shell/renderer/shell_content_renderer_client.cc?rcl=02582585906bf67195c50330ffef797fb2f7ce59&l=144 for workers. I don't think there's a universal "listen for all v8 context creation and hook it" type callback today. I would feel a bit wary of adding such a thing, since we already have so many hooks (as you pointed out, there's also DidCreateScriptContext)

Comment 6 by roc...@chromium.org, May 18 2017 

Sounds reasonable enough then.

Comment 7 by jbroman@chromium.org, May 18 2017

Cc: cha...@chromium.org iclell...@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, May 26 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2296029e3eb2343421c90354582aa111ecba43d6

commit 2296029e3eb2343421c90354582aa111ecba43d6
Author: Ken Rockot <rockot@chromium.org>
Date: Fri May 26 09:04:28 2017

Enable Blink native Mojo bindings in WebUI

Adds the concept of ContextEnabled features as an extended attribute in
Web IDL, exposing a way for embedders to turn specific API features on
or off at context creation time.

Affixes this extended attribute to Mojo, MojoHandle, and MojoWatcher
IDLs to allow their exposure to be controlled per-context separately
from the MojoJS RuntimeEnabled state.

Finally, this also flips on the MojoJS ContextEnabled feature for all
RenderFrame contexts in which WebUI bindings are enabled.

BUG= 699569 , 723923 

Change-Id: Ie0fd6383ae86c133685b3e7b31eeca769bf83e06
Reviewed-on: https://chromium-review.googlesource.com/509390
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474973}
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/content/browser/webui/web_ui_mojo_browsertest.cc
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/content/renderer/mojo_bindings_controller.cc
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/content/renderer/mojo_bindings_controller.h
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/content/test/data/web_ui_mojo_native.html
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/content/test/data/web_ui_mojo_native.js
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/IDLExtendedAttributes.txt
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/core/v8/ConditionalFeaturesForCore.cpp
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/scripts/generate_global_constructors.py
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/BUILD.gn
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/context_features/BUILD.gn
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/context_features/ContextFeatureSettings.cpp
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/context_features/ContextFeatureSettings.h
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/mojo/Mojo.idl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/mojo/MojoHandle.idl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/core/mojo/MojoWatcher.idl
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/Source/web/WebContextFeatures.cpp
[modify] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/2296029e3eb2343421c90354582aa111ecba43d6/third_party/WebKit/public/web/WebContextFeatures.h

Comment 9 by roc...@chromium.org, May 26 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment