Support combination of [OriginTrialEnabled] and [SecureContext]

third_party/WebKit/Source/bindings/scripts/code_generator.py

Index: third_party/WebKit/Source/bindings/scripts/code_generator.py
diff --git a/third_party/WebKit/Source/bindings/scripts/code_generator.py b/third_party/WebKit/Source/bindings/scripts/code_generator.py
index e6e1764ed86e5f643d938b01051ccde0f2e80b93..157688761a4b92b2a128679ad065ab192af04a78 100644
--- a/third_party/WebKit/Source/bindings/scripts/code_generator.py
+++ b/third_party/WebKit/Source/bindings/scripts/code_generator.py
@@ -64,6 +64,11 @@ def secure_context_if(code, secure_context_test):
     return generate_indented_conditional(code, 'executionContext && (%s)' % secure_context_test)
 
 
+def secure_context_cached_if(code, secure_context_test):

[Yuki, 2017/05/12 09:10:05]

Hmm, I think *cached* in the name is not easy to understand.  I took some time
to understand that *cached* means that we already have the result in
|isSecureContext|.  Also, it's not so good to have both of secure_context_if and
secure_context_cached_if.

What about to define the following?

  def secure_context_if(code, secure_context_test, test_result=None):
    if not secure_context_test:
      return code
    elif test_result:
      return generate_indented_conditional(code, test_result)
    else:
      return generate_indented_conditional(code, 'executionContext && (%s)' %
secure_context_test)

and call it as:

  secure_context(attribute.secure_context_test)

or

  secure_context(attribute.secure_context_test, 'isSecureContext')


Or, it would be good to replace the old use cases of |secure_context()| with the
new version so that we always use the |isSecureContext| version.

By the way, I am personally not a big fan to hard-code the variable names here
and there.  If we're going to pass |isSecureContext| in an argument, then we
don't need to hard-code it.

[chasej, 2017/05/16 02:21:39]

I agree on many points here. In an interim version, I had made similar changes
to the existing secure_context_if() function to handle both scenarios. I also
didn't like the hard-coding of variable names, but it seemed to be the common
approach in existing code. The function I came up with didn't seem like enough
of an improvement (especially since it still had |executionContext| hard-coded).

Yuki's suggestion for secure_context_if() is better, so I've implemented that
and
removed secure_context_cached_if(). I have not replaced existing usages of the
secure_context_if() function, as that would require larger changes (perhaps in
a later CL).

+    if not secure_context_test:
+        return code
+    return generate_indented_conditional(code, 'isSecureContext')

[Yuki, 2017/05/12 09:10:05]

nit: s/isSecureContext/is_secure_context/

[iclelland, 2017/05/12 15:11:49]

This looks like it matches surrounding code -- especially the 'executionContext'
variable declared in many places in the generated interfaces.

Should all of the bindings templates be updated to the new Blink style in a
single CL? That seems better than making a mix of styles in the templates for
now.

[Yuki, 2017/05/12 15:14:11]

Makes sense.  I'm fine with either way.

+
 # [RuntimeEnabled]
 def runtime_enabled_if(code, name):
     if not name:
@@ -72,7 +77,6 @@ def runtime_enabled_if(code, name):
     function = v8_utilities.runtime_enabled_function(name)
     return generate_indented_conditional(code, function)
 
-
 def initialize_jinja_env(cache_dir):
     jinja_env = jinja2.Environment(
         loader=jinja2.FileSystemLoader(TEMPLATES_DIR),
@@ -90,6 +94,7 @@ def initialize_jinja_env(cache_dir):
         'runtime_enabled': runtime_enabled_if,
         'runtime_enabled_function': v8_utilities.runtime_enabled_function,
         'secure_context': secure_context_if,
+        'secure_context_cached': secure_context_cached_if,
         'unique_by': unique_by})
     jinja_env.filters.update(constant_filters())
     jinja_env.filters.update(method_filters())