Allow origin trials to be enabled by script

third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.h

Index: third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.h b/third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.h
index 0c9a6040f5926955d921af4e7dd5f5d96e57caba..69bcca76d087b82fc489786020e7e587492ad368 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.h
+++ b/third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.h
@@ -5,21 +5,40 @@
 #ifndef ConditionalFeatures_h
 #define ConditionalFeatures_h
 
-#include "bindings/core/v8/DOMWrapperWorld.h"
 #include "core/CoreExport.h"
 #include "platform/feature_policy/FeaturePolicy.h"
+#include "wtf/text/WTFString.h"
 #include <v8.h>
 
 namespace blink {
 
-class ScriptState;
 class LocalFrame;
+class ScriptState;
+struct WrapperTypeInfo;
 
 using InstallConditionalFeaturesFunction = void (*)(const WrapperTypeInfo*,
                                                     const ScriptState*,
                                                     v8::Local<v8::Object>,
                                                     v8::Local<v8::Function>);
 
+using InstallPendingConditionalFeatureFunction = void (*)(const String&,
+                                                          const ScriptState*);
+
+// Sets the function to be called by |installConditionalFeatures|. The function
+// is initially set to the private |installConditionalFeaturesCore| 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 InstallConditionalFeaturesFunction
+    setInstallConditionalFeaturesFunction(InstallConditionalFeaturesFunction);
+
+// Sets the function to be called by |installPendingConditionalFeature|. This
+// Initially set to the private |installPendingConditionalFeatureCore| 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 InstallPendingConditionalFeatureFunction
+    setInstallPendingConditionalFeatureFunction(
+        InstallPendingConditionalFeatureFunction);
+
 // Installs all of the conditionally enabled V8 bindings for the given type, in
 // a specific context. This is called in V8PerContextData, after the constructor
 // and prototype for the type have been created. It indirectly calls the
@@ -29,19 +48,20 @@ CORE_EXPORT void installConditionalFeatures(const WrapperTypeInfo*,
                                             v8::Local<v8::Object>,
                                             v8::Local<v8::Function>);
 
-// Sets the function to be called by |installConditionalFeatures|. The function
-// is initially set to the private |installConditionalFeaturesCore| 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 InstallConditionalFeaturesFunction
-    setInstallConditionalFeaturesFunction(InstallConditionalFeaturesFunction);
-
 // Installs all of the conditionally enabled V8 bindings on the Window object.
 // This is called separately from other objects so that attributes and
 // interfaces which need to be visible on the global object are installed even
 // when the V8 context is reused (i.e., after navigation)

[haraken, 2017/01/28 02:25:20]

Not related to this CL, would you again help me understand why we need a special
logic for window? I think you're worrying about a case where a window object is
reused when we navigate from an initial empty document to a same-origin
document, but I've forgotten why we need a special handling for that case.

[chasej, 2017/01/30 19:15:56]

This method is called in LocalWindowProxy::initialize(). My understand is, in
that case, the trials are enabled before the context is initialized, so this
method is needed to install.

iclelland, please correct if I have that wrong.

[iclelland, 2017/01/30 20:37:39]

As I recall, when navigating between documents (not just from the initial empty
document), we clear out the per-context-data, but the context object itself is
reused. At that point, we needed to install any new origin trial attributes on
Window, so that the constructor interfaces are exposed to scripts.

[haraken, 2017/01/31 08:19:35]

Hmm, I'm a bit confused.

When we navigate from one document to another document, a global proxy is
reused. A global object is recreated. A per-context data is recreated. Are you
install anything on the global proxy? That shouldn't happen because it will
"leak" the origin trials on previous windows to future windows...

What's more confusing is:

- installConditionalFeatures calls s_installConditionalFeaturesFunction.
- install"Pending"ConditionalFeaturesOnWindow calls
s_installConditionalFeaturesFunction
- install"Pending"ConditionalFeature calls
s_install"Pending"ConditionalFeatureFunction

Why is install"Pending"ConditionalFeaturesOnWindow not calling
s_install"Pending"ConditionalFeatureFunction?

[iclelland, 2017/01/31 15:29:17]

The original issue that prompted that code was https://crbug.com/653671, which
was addressed in https://codereview.chromium.org/2458183002. What we were seeing
was exactly the opposite of that. Navigating from a page where the trial was not
installed to one where it was would not actually install it. (Navigating from a
page with the trial to one without wouldn't leak the trial; it was not present
on the new page).

If that behaviour is unexpected, we should look into why it's happening that
way.
chasej, is that just bad naming at this point?
installPendingConditionalFeaturesOnWindow pre-dates the new
installPendingConditionalFeatures functions. Perhaps it should just be
"installConditionalFeaturesOnWindow" now, for consistency.

 CORE_EXPORT void installPendingConditionalFeaturesOnWindow(const ScriptState*);
 
+// Installs all of the conditionally enabled V8 bindings for a feature, if
+// needed. This is called to install a newly-enabled feature on any existing
+// objects. If the target object hasn't been created, nothing is installed. The
+// enabled feature will be instead be installed when the object is created
+// (avoids forcing the creation of objects prematurely).
+CORE_EXPORT void installPendingConditionalFeature(const String&,
+                                                  const ScriptState*);
+
 CORE_EXPORT bool isFeatureEnabledInFrame(const FeaturePolicy::Feature&,
                                          const LocalFrame*);