Support combination of [OriginTrialEnabled] and [SecureContext]

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

 # 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.
 
 # pylint: disable=import-error,print-statement,relative-import
 
 """Plumbing for a Jinja-based code generator, including CodeGeneratorBase, a base class for all generators."""
 
 import os
 import posixpath
@@ skipping 46 matching lines @@
     return generate_indented_conditional(code, 'executionContext && (%s)' % exposed_test)
 
 
 # [SecureContext]
 def secure_context_if(code, secure_context_test):
     if not secure_context_test:
         return code
     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:
         return code
 
     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),
         # Bytecode cache is not concurrency-safe unless pre-cached:
         # if pre-cached this is read-only, but writing creates a race condition.
         bytecode_cache=jinja2.FileSystemBytecodeCache(cache_dir),
         keep_trailing_newline=True,  # newline-terminate generated files
         lstrip_blocks=True,  # so can indent control flow tags
         trim_blocks=True)
     jinja_env.filters.update({
         'blink_capitalize': capitalize,
         'exposed': exposed_if,
         'format_blink_cpp_source_code': format_blink_cpp_source_code,
         'format_remove_duplicates': format_remove_duplicates,
         '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())
     return jinja_env
 
 
 def normalize_and_sort_includes(include_paths):
     normalized_include_paths = []
     for include_path in include_paths:
         match = re.search(r'/gen/blink/(.*)$', posixpath.abspath(include_path))
@@ skipping 81 matching lines @@
 
     # Create a dummy file as output for the build system,
     # since filenames of individual cache files are unpredictable and opaque
     # (they are hashes of the template path, which varies based on environment)
     with open(dummy_filename, 'w') as dummy_file:
         pass  # |open| creates or touches the file
 
 
 if __name__ == '__main__':
     sys.exit(main(sys.argv))