Eagerly install Origin Trial features on window

third_party/WebKit/Source/bindings/modules/v8/ConditionalFeaturesForModules.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/modules/v8/ConditionalFeaturesForModules.h"
 
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/V8DedicatedWorkerGlobalScope.h"
 #include "bindings/core/v8/V8Navigator.h"
 #include "bindings/core/v8/V8SharedWorkerGlobalScope.h"
@@ skipping 11 matching lines @@
 #include "core/dom/ExecutionContext.h"
 #include "core/origin_trials/OriginTrials.h"
 
 namespace blink {
 
 namespace {
 InstallConditionalFeaturesFunction
     s_originalInstallConditionalFeaturesFunction = nullptr;
 }
 
 void installConditionalFeaturesForModules(

[Yuki, 2016/11/01 11:37:54]

Could you add a function header comment in *.h?
Especially what each argument should be.

     const WrapperTypeInfo* wrapperTypeInfo,
     const ScriptState* scriptState,
     v8::Local<v8::Object> prototypeObject,
     v8::Local<v8::Function> interfaceObject) {
   // 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_originalInstallConditionalFeaturesFunction)(
       wrapperTypeInfo, scriptState, prototypeObject, interfaceObject);
 
   ExecutionContext* executionContext = scriptState->getExecutionContext();
   if (!executionContext)
     return;
-  v8::Isolate* isolate = scriptState->isolate();
-  const DOMWrapperWorld& world = scriptState->world();
   v8::Local<v8::Object> global = scriptState->context()->Global();
   if (wrapperTypeInfo == &V8Navigator::wrapperTypeInfo) {
     if (OriginTrials::webShareEnabled(executionContext)) {
-      V8NavigatorPartial::installWebShare(isolate, world,
-                                          v8::Local<v8::Object>(),
+      V8NavigatorPartial::installWebShare(scriptState, v8::Local<v8::Object>(),

[Yuki, 2016/11/01 11:37:54]

Given that you've already extracted |global|, why don't you pass in |global|?

[iclelland, 2016/11/01 17:06:27]

That parameter is supposed to be the object instance to install the interface
members on, *if* any of them are configured with
V8DOMConfiguration::OnInterface. This doesn't happen except with constructor
attrtibutes, so the parameter is unused here.

Passing |global| would satisfy the requirements of the function signature, and
it isn't used anyway, but it's the *wrong* object if it were used.

If we improved the code generation, we could tell that the instance parameter is
never used, and remove it from |installWebShare| and similar methods. But at the
moment, all of the install{origin_trial} methods take all three parameters
(instance, prototype, interface), even if one or more of them are unused.

                                           prototypeObject, interfaceObject);
     }
     if (OriginTrials::webUSBEnabled(executionContext)) {
-      V8NavigatorPartial::installWebUSB(isolate, world, v8::Local<v8::Object>(),
+      V8NavigatorPartial::installWebUSB(scriptState, v8::Local<v8::Object>(),
                                         prototypeObject, interfaceObject);
     }
     if (OriginTrials::webVREnabled(executionContext)) {
-      V8NavigatorPartial::installWebVR(isolate, world, global, prototypeObject,
+      V8NavigatorPartial::installWebVR(scriptState, global, prototypeObject,
                                        interfaceObject);
     }
   } else if (wrapperTypeInfo == &V8Window::wrapperTypeInfo) {
     if (OriginTrials::webUSBEnabled(executionContext)) {
-      V8WindowPartial::installWebUSB(isolate, world, global, prototypeObject,
+      V8WindowPartial::installWebUSB(scriptState, global, prototypeObject,
                                      interfaceObject);
     }
     if (OriginTrials::webVREnabled(executionContext)) {
-      V8WindowPartial::installWebVR(isolate, world, global, prototypeObject,
+      V8WindowPartial::installWebVR(scriptState, global, prototypeObject,
                                     interfaceObject);
     }
     if (OriginTrials::gamepadExtensionsEnabled(executionContext)) {
       V8WindowPartial::installGamepadExtensions(
-          isolate, world, global, prototypeObject, interfaceObject);
+          scriptState, global, prototypeObject, interfaceObject);
     }
   } else if (wrapperTypeInfo == &V8ServiceWorkerGlobalScope::wrapperTypeInfo) {
     if (OriginTrials::foreignFetchEnabled(executionContext)) {
       V8ServiceWorkerGlobalScope::installForeignFetch(
-          isolate, world, global, prototypeObject, interfaceObject);
+          scriptState, global, prototypeObject, interfaceObject);
     }
   } else if (wrapperTypeInfo == &V8InstallEvent::wrapperTypeInfo) {
     if (OriginTrials::foreignFetchEnabled(executionContext)) {
-      V8InstallEvent::installForeignFetch(isolate, world,
-                                          v8::Local<v8::Object>(),
+      V8InstallEvent::installForeignFetch(scriptState, v8::Local<v8::Object>(),
                                           prototypeObject, interfaceObject);
     }
   } else if (wrapperTypeInfo == &V8Gamepad::wrapperTypeInfo) {
     if (OriginTrials::gamepadExtensionsEnabled(executionContext)) {
-      V8Gamepad::installGamepadExtensions(isolate, world, global,
-                                          prototypeObject, interfaceObject);
+      V8Gamepad::installGamepadExtensions(scriptState, global, prototypeObject,
+                                          interfaceObject);
     }
   } else if (wrapperTypeInfo == &V8GamepadButton::wrapperTypeInfo) {
     if (OriginTrials::gamepadExtensionsEnabled(executionContext)) {
       V8GamepadButton::installGamepadExtensions(
-          isolate, world, global, prototypeObject, interfaceObject);
+          scriptState, global, prototypeObject, interfaceObject);
     }
   }
 }
 
 void registerInstallConditionalFeaturesForModules() {
   s_originalInstallConditionalFeaturesFunction =
       setInstallConditionalFeaturesFunction(
           &installConditionalFeaturesForModules);
 }
 
 }  // namespace blink