Skip to content

Commit

Permalink
[Origin Trials] Install origin trial bindings on V8 context condition…
Browse files Browse the repository at this point in the history
…ally

This patch removes the call-time check for origin-trial-enabled features,
replacing it with code that installs features into the V8 context as trial
tokens are encountered. This means that experimental features which were previously polluting the global namespaces are now completely unavailable except when enabled by origin trials. (The specific case of pollution caused by Durable Storage is removed from test expectations in this patch)

Origin trials are now initialized after the V8 context, by calling initializeOriginTrials. This calls, through a series of function pointers, the bindings initialization code for core, modules, and optionally internals (for tests).

This patch implements the design at https://docs.google.com/document/d/1vwKD8sUrr5mtktvEwvO_f2KUsNhpcgLMixYXhD8K4KY

BUG=584367

Review-Url: https://codereview.chromium.org/2005433002
Cr-Commit-Position: refs/heads/master@{#396860}
(cherry picked from commit e3f3c54)

Review URL: https://codereview.chromium.org/2032613005 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#185}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
clelland committed Jun 2, 2016
1 parent 79f002e commit c31f015
Show file tree
Hide file tree
Showing 27 changed files with 338 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,6 @@ interface webkitIDBTransaction : EventTarget
setter oncomplete
setter onerror
global object
attribute StorageManager
attribute console
attribute internals
getter caches
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
CONSOLE ERROR: line 94: The 'DurableStorage' feature is currently enabled in limited trials. Please see https://bit.ly/OriginTrials for information on enabling a trial for your site.
CONSOLE WARNING: 'webkitIDBTransaction' is deprecated. Please use 'IDBTransaction' instead.
CONSOLE WARNING: 'webkitIDBRequest' is deprecated. Please use 'IDBRequest' instead.
CONSOLE WARNING: 'webkitIDBObjectStore' is deprecated. Please use 'IDBObjectStore' instead.
Expand Down Expand Up @@ -854,7 +853,6 @@ Starting worker: resources/global-interface-listing.js
[Worker] setter oncomplete
[Worker] setter onerror
[Worker] [GLOBAL OBJECT]
[Worker] attribute StorageManager
[Worker] attribute console
[Worker] attribute internals
[Worker] getter caches
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ CONSOLE WARNING: line 94: 'webkitIDBFactory' is deprecated. Please use 'IDBFacto
CONSOLE WARNING: line 94: 'webkitIDBDatabase' is deprecated. Please use 'IDBDatabase' instead.
CONSOLE WARNING: line 94: 'webkitIDBCursor' is deprecated. Please use 'IDBCursor' instead.
CONSOLE WARNING: line 94: 'webkitIndexedDB' is deprecated. Please use 'indexedDB' instead.
CONSOLE ERROR: line 94: The 'DurableStorage' feature is currently enabled in limited trials. Please see https://bit.ly/OriginTrials for information on enabling a trial for your site.
CONSOLE WARNING: line 94: 'webkitURL' is deprecated. Please use 'URL' instead.
This test documents all interface attributes and methods on the global window object and element instances.

Expand Down Expand Up @@ -3373,7 +3372,6 @@ interface Navigator
getter product
getter productSub
getter serviceWorker
getter storage
getter userAgent
getter vendor
getter vendorSub
Expand Down Expand Up @@ -6458,7 +6456,6 @@ interface webkitURL
setter username
[GLOBAL OBJECT]
attribute GCController
attribute StorageManager
attribute accessibilityController
attribute applicationCache
attribute caches
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,6 @@ Starting worker: resources/global-interface-listing.js
[Worker] setter oncomplete
[Worker] setter onerror
[Worker] [GLOBAL OBJECT]
[Worker] attribute StorageManager
[Worker] attribute console
[Worker] attribute internals
[Worker] getter caches
Expand Down
26 changes: 26 additions & 0 deletions third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,32 @@ v8::Local<v8::Context> toV8ContextEvenIfDetached(Frame* frame, DOMWrapperWorld&
return frame->windowProxy(world)->contextIfInitialized();
}

void installOriginTrialsCore(ScriptState* scriptState)
{
// TODO(iclelland): Generate all of this logic at compile-time, based on the
// configuration of origin trial enabled attibutes and interfaces in IDL
// files. (crbug.com/615060)

// Initialization code for origin trials for core bindings, if necessary,
// should go here.
}

namespace {
InstallOriginTrialsFunction s_installOriginTrialsFunction = &installOriginTrialsCore;
}

void installOriginTrials(ScriptState* scriptState)
{
(*s_installOriginTrialsFunction)(scriptState);
}

InstallOriginTrialsFunction setInstallOriginTrialsFunction(InstallOriginTrialsFunction newInstallOriginTrialsFunction)
{
InstallOriginTrialsFunction originalFunction = s_installOriginTrialsFunction;
s_installOriginTrialsFunction = newInstallOriginTrialsFunction;
return originalFunction;
}

void crashIfIsolateIsDead(v8::Isolate* isolate)
{
if (isolate->IsDead()) {
Expand Down
16 changes: 16 additions & 0 deletions third_party/WebKit/Source/bindings/core/v8/V8Binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ class WorkerGlobalScope;
class WorkerOrWorkletGlobalScope;
class XPathNSResolver;

using InstallOriginTrialsFunction = void (*)(ScriptState*);

template <typename T>
struct V8TypeOf {
STATIC_ONLY(V8TypeOf);
Expand Down Expand Up @@ -861,6 +863,20 @@ CORE_EXPORT EventTarget* toEventTarget(v8::Isolate*, v8::Local<v8::Value>);
// to allocate it using alloca() in the callers stack frame.
CORE_EXPORT void toFlexibleArrayBufferView(v8::Isolate*, v8::Local<v8::Value>, FlexibleArrayBufferView&, void* storage = nullptr);

// Installs all of the origin-trial-enabled V8 bindings for the given context
// and world, based on the trial tokens which have been added to the
// ExecutionContext. This should be called after the V8 context has been
// installed, but may be called multiple times, as trial tokens are
// encountered. It indirectly calls the function set by
// |setInstallOriginTrialsFunction|.
CORE_EXPORT void installOriginTrials(ScriptState*);

// Sets the function to be called by |installOriginTrials|. The function is
// initially set to the private |installOriginTrialsForCore| function, but
// can be overridden by this function. A pointer to the previously set function
// is returned, so that functions can be chained.
CORE_EXPORT InstallOriginTrialsFunction setInstallOriginTrialsFunction(InstallOriginTrialsFunction);

// If the current context causes out of memory, JavaScript setting
// is disabled and it returns true.
bool handleOutOfMemory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "bindings/core/v8/V8DOMConfiguration.h"

#include "bindings/core/v8/V8ObjectConstructor.h"
#include "bindings/core/v8/V8PerContextData.h"
#include "platform/TraceEvent.h"

namespace blink {
Expand Down Expand Up @@ -68,7 +69,16 @@ void installAttributeInternal(v8::Isolate* isolate, v8::Local<v8::Object> instan
return;

v8::Local<v8::Name> name = v8AtomicString(isolate, attribute.name);
v8::Local<v8::Value> data = v8::External::New(isolate, const_cast<WrapperTypeInfo*>(attribute.data));

// This method is only being used for installing interfaces which are
// enabled through origin trials. Assert here that it is being called with
// an attribute configuration for a constructor.
// TODO(iclelland): Relax this constraint and allow arbitrary data-type
// properties to be added here.
DCHECK_EQ(&v8ConstructorAttributeGetter, attribute.getter);

V8PerContextData* perContextData = V8PerContextData::from(isolate->GetCurrentContext());
v8::Local<v8::Function> data = perContextData->constructorForType(attribute.data);

DCHECK(attribute.propertyLocationConfiguration);
if (attribute.propertyLocationConfiguration & V8DOMConfiguration::OnInstance)
Expand Down
9 changes: 9 additions & 0 deletions third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "core/loader/DocumentLoader.h"
#include "core/loader/FrameLoader.h"
#include "core/loader/FrameLoaderClient.h"
#include "core/origin_trials/OriginTrialContext.h"
#include "platform/Histogram.h"
#include "platform/RuntimeEnabledFeatures.h"
#include "platform/ScriptForbiddenScope.h"
Expand Down Expand Up @@ -269,6 +270,14 @@ bool WindowProxy::initialize()
MainThreadDebugger::instance()->contextCreated(m_scriptState.get(), frame, origin);
frame->loader().client()->didCreateScriptContext(context, m_world->extensionGroup(), m_world->worldId());
}
// If Origin Trials have been registered before the V8 context was ready,
// then inject them into the context now
ExecutionContext* executionContext = m_scriptState->getExecutionContext();
if (executionContext) {
OriginTrialContext* originTrialContext = OriginTrialContext::from(executionContext);
if (originTrialContext)
originTrialContext->initializePendingFeatures();
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ void initPartialInterfacesInModules();
void ModuleBindingsInitializer::init()
{
registerToExecutionContextForModules(toExecutionContextForModules);
registerInstallOriginTrialsForModules();
initPartialInterfacesInModules();
SerializedScriptValueFactory::initialize(new SerializedScriptValueForModulesFactory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,22 @@
#include "bindings/core/v8/V8Uint8Array.h"
#include "bindings/core/v8/WorkerOrWorkletScriptController.h"
#include "bindings/modules/v8/ToV8ForModules.h"
#include "bindings/modules/v8/V8DedicatedWorkerGlobalScopePartial.h"
#include "bindings/modules/v8/V8IDBCursor.h"
#include "bindings/modules/v8/V8IDBCursorWithValue.h"
#include "bindings/modules/v8/V8IDBDatabase.h"
#include "bindings/modules/v8/V8IDBIndex.h"
#include "bindings/modules/v8/V8IDBKeyRange.h"
#include "bindings/modules/v8/V8IDBObjectStore.h"
#include "bindings/modules/v8/V8NavigatorPartial.h"
#include "bindings/modules/v8/V8ServiceWorkerGlobalScope.h"
#include "bindings/modules/v8/V8SharedWorkerGlobalScopePartial.h"
#include "bindings/modules/v8/V8WindowPartial.h"
#include "bindings/modules/v8/V8WorkletGlobalScope.h"
#include "core/dom/DOMArrayBuffer.h"
#include "core/dom/DOMArrayBufferView.h"
#include "core/dom/ExecutionContext.h"
#include "core/origin_trials/OriginTrialContext.h"
#include "modules/indexeddb/IDBKey.h"
#include "modules/indexeddb/IDBKeyPath.h"
#include "modules/indexeddb/IDBKeyRange.h"
Expand Down Expand Up @@ -532,4 +538,46 @@ ExecutionContext* toExecutionContextForModules(v8::Local<v8::Context> context)
return nullptr;
}

namespace {
InstallOriginTrialsFunction s_originalInstallOriginTrialsFunction = nullptr;
}

void installOriginTrialsForModules(ScriptState* scriptState)
{
// TODO(iclelland): Generate all of this logic at compile-time, based on the
// configuration of origin trial enabled attibutes and interfaces in IDL
// files. (crbug.com/615060)
(*s_originalInstallOriginTrialsFunction)(scriptState);

v8::Local<v8::Context> context = scriptState->context();
ExecutionContext* executionContext = toExecutionContext(context);
OriginTrialContext* originTrialContext = OriginTrialContext::from(executionContext, OriginTrialContext::DontCreateIfNotExists);
if (!originTrialContext)
return;

ScriptState::Scope scope(scriptState);
v8::Local<v8::Object> global = context->Global();
v8::Isolate* isolate = scriptState->isolate();

if (!originTrialContext->featureBindingsInstalled("DurableStorage") && (RuntimeEnabledFeatures::durableStorageEnabled() || originTrialContext->isFeatureEnabled("DurableStorage", nullptr))) {
if (executionContext->isDocument()) {
v8::Local<v8::String> navigatorName = v8::String::NewFromOneByte(isolate, reinterpret_cast<const uint8_t*>("navigator"), v8::NewStringType::kNormal).ToLocalChecked();
v8::Local<v8::Object> navigator = global->Get(context, navigatorName).ToLocalChecked()->ToObject();
V8WindowPartial::installDurableStorage(scriptState, global);
V8NavigatorPartial::installDurableStorage(scriptState, navigator);
} else if (executionContext->isSharedWorkerGlobalScope()) {
V8SharedWorkerGlobalScopePartial::installDurableStorage(scriptState, global);
} else if (executionContext->isDedicatedWorkerGlobalScope()) {
V8DedicatedWorkerGlobalScopePartial::installDurableStorage(scriptState, global);
} else if (executionContext->isServiceWorkerGlobalScope()) {
V8ServiceWorkerGlobalScope::installDurableStorage(scriptState, global);
}
originTrialContext->setFeatureBindingsInstalled("DurableStorage");
}
}

void registerInstallOriginTrialsForModules()
{
s_originalInstallOriginTrialsFunction = setInstallOriginTrialsFunction(&installOriginTrialsForModules);
}
} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ struct NativeValueTraits<IDBKeyRange*> {

ExecutionContext* toExecutionContextForModules(v8::Local<v8::Context>);

void registerInstallOriginTrialsForModules();

} // namespace blink

#endif // V8BindingForModules_h
48 changes: 47 additions & 1 deletion third_party/WebKit/Source/bindings/scripts/v8_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ def attribute_context(interface, attribute):
'on_interface': v8_utilities.on_interface(interface, attribute),
'on_prototype': v8_utilities.on_prototype(interface, attribute),
'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(attribute, interface), # [OriginTrialEnabled]
'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(attribute), # [OriginTrialEnabled]
'use_output_parameter_for_result': idl_type.use_output_parameter_for_result,
'measure_as': v8_utilities.measure_as(attribute, interface), # [MeasureAs]
'name': attribute.name,
Expand All @@ -169,9 +170,54 @@ def attribute_context(interface, attribute):
if not has_custom_setter(attribute) and has_setter(interface, attribute):
setter_context(interface, attribute, context)

# [OriginTrialEnabled]
# TODO(iclelland): Allow origin trials on static interfaces
# (crbug.com/614352)
if context['origin_trial_feature_name'] and context['on_interface']:
raise Exception('[OriginTrialEnabled] cannot be specified on static '
'attributes: %s.%s' % (interface.name, attribute.name))

return context


def filter_has_accessor_configuration(attributes):
return [attribute for attribute in attributes if
not (attribute['exposed_test'] or
attribute['origin_trial_enabled_function'] or
attribute['runtime_enabled_function']) and
not attribute['is_data_type_property'] and
attribute['should_be_exposed_to_script']]


def filter_has_attribute_configuration(attributes):
return [attribute for attribute in attributes if
not (attribute['exposed_test'] or
attribute['origin_trial_enabled_function'] or
attribute['runtime_enabled_function']) and
attribute['is_data_type_property'] and
attribute['should_be_exposed_to_script']]


def filter_origin_trial_enabled(attributes):
return [attribute for attribute in attributes if
attribute['origin_trial_feature_name'] and
not attribute['exposed_test']]


def filter_runtime_enabled(attributes):
return [attribute for attribute in attributes if
attribute['runtime_feature_name'] and
not attribute['exposed_test']]


def attribute_filters():
return {'has_accessor_configuration': filter_has_accessor_configuration,
'has_attribute_configuration': filter_has_attribute_configuration,
'origin_trial_enabled_attributes': filter_origin_trial_enabled,
'runtime_enabled_attributes': filter_runtime_enabled,
}


################################################################################
# Getter
################################################################################
Expand Down Expand Up @@ -520,4 +566,4 @@ def is_constructor_attribute(attribute):


def update_constructor_attribute_context(interface, attribute, context):
context['needs_constructor_getter_callback'] = context['measure_as'] or context['deprecate_as'] or context['origin_trial_enabled_function'] # TODO(chasej): Should/can this be true when OriginTrialEnabled is inherited from containing interface?
context['needs_constructor_getter_callback'] = context['measure_as'] or context['deprecate_as']
9 changes: 8 additions & 1 deletion third_party/WebKit/Source/bindings/scripts/v8_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ def origin_trial_enabled_function_name(definition_or_member, interface):
'%s.%s' % (definition_or_member.idl_name, definition_or_member.name))

if is_origin_trial_enabled:
includes.add('core/inspector/ConsoleMessage.h')
includes.add('bindings/core/v8/ScriptState.h')
includes.add('core/origin_trials/OriginTrials.h')

trial_name = extended_attributes['OriginTrialEnabled']
Expand All @@ -391,6 +391,13 @@ def origin_trial_enabled_function_name(definition_or_member, interface):
return None


def origin_trial_feature_name(definition_or_member):
extended_attributes = definition_or_member.extended_attributes
if 'OriginTrialEnabled' not in extended_attributes:
return None
return extended_attributes['OriginTrialEnabled']


def runtime_feature_name(definition_or_member):
extended_attributes = definition_or_member.extended_attributes
if 'RuntimeEnabled' not in extended_attributes:
Expand Down
14 changes: 1 addition & 13 deletions third_party/WebKit/Source/bindings/templates/attributes.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{% from 'utilities.cpp' import declare_enum_validation_variable, v8_value_to_local_cpp_value, check_origin_trial %}
{% from 'utilities.cpp' import declare_enum_validation_variable, v8_value_to_local_cpp_value %}

{##############################################################################}
{% macro attribute_getter(attribute, world_suffix) %}
Expand All @@ -9,9 +9,6 @@ const v8::PropertyCallbackInfo<v8::Value>& info
const v8::FunctionCallbackInfo<v8::Value>& info
{%- endif %})
{
{% if attribute.origin_trial_enabled_function %}
{{check_origin_trial(attribute) | indent}}
{% endif %}
{% if attribute.is_reflect and not attribute.is_url
and attribute.idl_type == 'DOMString' and is_node
and not attribute.is_implemented_in_private_script %}
Expand Down Expand Up @@ -189,9 +186,6 @@ const v8::FunctionCallbackInfo<v8::Value>& info
{% if attribute.measure_as %}
UseCounter::countIfNotPrivateScript(info.GetIsolate(), currentExecutionContext(info.GetIsolate()), UseCounter::{{attribute.measure_as('AttributeGetter')}});
{% endif %}
{% if attribute.origin_trial_enabled_function %}
{{check_origin_trial(attribute) | indent}}
{% endif %}
{% if world_suffix in attribute.activity_logging_world_list_for_getter %}
ScriptState* scriptState = ScriptState::from(info.GetIsolate()->GetCurrentContext());
V8PerContextData* contextData = scriptState->perContextData();
Expand Down Expand Up @@ -221,9 +215,6 @@ static void {{attribute.name}}ConstructorGetterCallback{{world_suffix}}(v8::Loca
{% if attribute.measure_as %}
UseCounter::countIfNotPrivateScript(info.GetIsolate(), currentExecutionContext(info.GetIsolate()), UseCounter::{{attribute.measure_as('ConstructorGetter')}});
{% endif %}
{% if attribute.origin_trial_enabled_function %}
{{check_origin_trial(attribute) | indent}}
{% endif %}
v8ConstructorAttributeGetter(property, info);
}
{% endmacro %}
Expand Down Expand Up @@ -423,9 +414,6 @@ bool {{v8_class}}::PrivateScript::{{attribute.name}}AttributeGetter(LocalFrame*
if (holder.IsEmpty())
return false;

{% if attribute.origin_trial_enabled_function %}
{{check_origin_trial(attribute, "scriptState->isolate()") | indent}}
{% endif %}

ExceptionState exceptionState(ExceptionState::GetterContext, "{{attribute.name}}", "{{cpp_class}}", scriptState->context()->Global(), scriptState->isolate());
v8::Local<v8::Value> v8Value = PrivateScriptRunner::runDOMAttributeGetter(scriptState, scriptStateInUserScript, "{{cpp_class}}", "{{attribute.name}}", holder);
Expand Down
Loading

0 comments on commit c31f015

Please sign in to comment.