Add support for [Global] / [PrimaryGlobal] IDL extended attributes

Source/bindings/scripts/generate_global_constructors.py

 #!/usr/bin/python
 #
 # Copyright 2014 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.
 
 """Generates interface properties on global objects.
 
 Concretely these are implemented as "constructor attributes", meaning
 "attributes whose name ends with Constructor" (special-cased by code generator),
 hence "global constructors" for short.
 
 For reference on global objects, see:
 http://heycam.github.io/webidl/#Global
 http://heycam.github.io/webidl/#Exposed
 
 Design document: http://www.chromium.org/developers/design-documents/idl-build
 """
 
 import optparse
 import os
 import re
 import sys
 
+from collections import defaultdict
 from utilities import get_file_contents, write_file, get_interface_extended_attributes_from_idl, is_callback_interface_from_idl
 
-global_objects = {}
+global_name_to_interface_names = defaultdict(list)
+global_name_to_constructors = defaultdict(list)
 
 
 HEADER_FORMAT = """
 // Stub header file for {{idl_basename}}
 // Required because the IDL compiler assumes that a corresponding header file
 // exists for each IDL file.
 """
 
 def parse_options():
     parser = optparse.OptionParser()
     parser.add_option('--idl-files-list', help='file listing IDL files')
     parser.add_option('--write-file-only-if-changed', type='int', help='if true, do not write an output file if it would be identical to the existing one, which avoids unnecessary rebuilds in ninja')
 
     options, args = parser.parse_args()
 
     if options.idl_files_list is None:
         parser.error('Must specify a file listing IDL files using --idl-files-list.')
     if options.write_file_only_if_changed is None:
         parser.error('Must specify whether output files are only written if changed using --write-file-only-if-changed.')
     options.write_file_only_if_changed = bool(options.write_file_only_if_changed)
 
     return options, args
 
 
-# Global name: http://heycam.github.io/webidl/#dfn-global-name
-# FIXME: We should add support for [Global=xx] extended attribute instead of
-# hard-coding this mapping.
-def global_name_to_interface_name(global_name):
-    if global_name.endswith('Worker'):
-        return global_name + 'GlobalScope'
-    return global_name
+# If the [Global] or [PrimaryGlobal] extended attribute is declared with an

[Nils Barth (inactive), 2014/05/07 01:55:57]

Could you put this explanation in a docstring instead of comment?
(We're not very consistent on this, but this sort of explanation
is perfect for a docstring.)

[Inactive, 2014/05/07 15:45:12]

Done.

+# identifier list argument, then those identifiers are the interface's global
+# names; otherwise, the interface has a single global name, which is the
+# interface's identifier.
+# http://heycam.github.io/webidl/#Global
+def interface_to_global_names(interface_name, extended_attributes):
+    for extended_attribute in ['Global', 'PrimaryGlobal']:

[Nils Barth (inactive), 2014/05/07 01:55:57]

|key| is fine for the name here

[Inactive, 2014/05/07 15:45:12]

Done.

+        global_value = extended_attributes.get(extended_attribute)

[Nils Barth (inactive), 2014/05/07 01:55:57]

Could you flatten this logic?

I generally prefer the following form:
if key not in extended_attributes:
    continue
global_value = extended_attributes[key]
if global_value:
    # FIXME: In spec names are comma-separated, but that makes parsing very
    # difficult (https://www.w3.org/Bugs/Public/show_bug.cgi?id=24959).
    return global_value.split('&')
return [interface_name]


Note official style points:
* Use the "implicit" false if at all possible.
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#True/False_eva...
* Don't use else after return
http://www.chromium.org/developers/coding-style

(Checking key instead of using dict.get() is personal preference, and avoids
test for implicit None.)

[Inactive, 2014/05/07 15:45:12]

Done.

+        if global_value is not None:
+            if len(global_value) > 0:
+                # FIXME: In spec names are comma-separated, but that makes parsing very
+                # difficult (https://www.w3.org/Bugs/Public/show_bug.cgi?id=24959).
+                return global_value.split('&')
+            else:
+                return [interface_name]
+    return []
 
 
 def record_global_constructors(idl_filename):
     interface_name, _ = os.path.splitext(os.path.basename(idl_filename))
     full_path = os.path.realpath(idl_filename)
     idl_file_contents = get_file_contents(full_path)
     extended_attributes = get_interface_extended_attributes_from_idl(idl_file_contents)
 
     # An interface property is produced for every non-callback interface
     # that does not have [NoInterfaceObject].
     # Callback interfaces with constants also have interface properties,
     # but there are none of these in Blink.
     # http://heycam.github.io/webidl/#es-interfaces
     if (is_callback_interface_from_idl(idl_file_contents) or
         'NoInterfaceObject' in extended_attributes):
         return
 
+    # Check if interface has [Global] / [PrimaryGlobal] extended attributes.
+    global_names = interface_to_global_names(interface_name, extended_attributes)
+    for global_name in global_names:
+        global_name_to_interface_names[global_name].append(interface_name)
+
+    # The [Exposed] extended attribute MUST take an identifier list. Each
+    # identifier in the list MUST be a global name. An interface or interface
+    # member the extended attribute applies to will be exposed only on objects
+    # associated with ECMAScript global environments whose global object
+    # implements an interface that has a matching global name.
     # FIXME: In spec names are comma-separated, but that makes parsing very
     # difficult (https://www.w3.org/Bugs/Public/show_bug.cgi?id=24959).
-    global_names = extended_attributes.get('Exposed', 'Window').split('&')
+    exposed_global_names = extended_attributes.get('Exposed', 'Window').split('&')
     new_constructors_list = generate_global_constructors_list(interface_name, extended_attributes)
-    for global_name in global_names:
-        interface_name = global_name_to_interface_name(global_name)
-        global_objects[interface_name]['constructors'].extend(new_constructors_list)
+    for exposed_global_name in exposed_global_names:
+        global_name_to_constructors[exposed_global_name].extend(new_constructors_list)
 
 
 def generate_global_constructors_list(interface_name, extended_attributes):
     extended_attributes_list = [
             name + '=' + extended_attributes[name]
             for name in 'Conditional', 'PerContextEnabled', 'RuntimeEnabled'
             if name in extended_attributes]
     if extended_attributes_list:
         extended_string = '[%s] ' % ', '.join(extended_attributes_list)
     else:
@@ skipping 39 matching lines @@
 
     # Input IDL files are passed in a file, due to OS command line length
     # limits. This is generated at GYP time, which is ok b/c files are static.
     with open(options.idl_files_list) as idl_files_list:
         idl_files = [line.rstrip('\n') for line in idl_files_list]
 
     # Output IDL files (to generate) are passed at the command line, since
     # these are in the build directory, which is determined at build time, not
     # GYP time.
     # These are passed as pairs of GlobalObjectName, GlobalObject.idl
-    interface_name_idl_filename = [(args[i], args[i + 1])
-                                   for i in range(0, len(args), 2)]
-    global_objects.update(
-        (interface_name, {
-            'idl_filename': idl_filename,
-            'constructors': [],
-        })
-        for interface_name, idl_filename in interface_name_idl_filename)
+    interface_name_to_idl_filename = {args[i]: args[i + 1]

[Nils Barth (inactive), 2014/05/07 01:55:57]

Need to use dict(...) instead of {...: ...}:
dictionary comprehensions are Python 2.7, and we're using Python 2.6.
(Mac OS X and Windows)
Also, I'd prefer this to be a list of tuples, since it's just used for one loop:
dict just makes the loop more complicated.
(Detailed below.)

[Inactive, 2014/05/07 15:45:12]

Done.

+                                      for i in range(0, len(args), 2)}
 
     for idl_filename in idl_files:
         record_global_constructors(idl_filename)
 
-    for interface_name, global_object in global_objects.iteritems():
+    # An interface can have multiple global names (e.g.

[Nils Barth (inactive), 2014/05/07 01:55:57]

This is really confusing; I think it's because of a dictionary being in the
backwards order.
I think you want
interface_name_to_global_names instead of
global_name_to_interface_names.

Ultimately we're writing IDL files with lists of constructors, right?
(...and these IDL files have one-to-one correspondence with interface names,
like
WindowConstructors.idl: partial interface Window { ... };
...so can use interface name as key.)

So what we want is a dictionary (aka, map)
interface_name_to_constructors, as you've done.
However, it's easier to compose maps if you don't have to invert,
so clearer is:
interface name -> global names
and
global name -> constructors.
...and then interface_name_to_constructors is very simple to compute.

...and in fact can be computed inline in the next loop, so we don't need a
separate loop.

Does this make sense?

[Inactive, 2014/05/07 15:45:12]

Done.

+    # Global=Worker&SharedWorker) so we construct a
+    # interface_name_to_constructors dict from global_name_to_constructors and
+    # global_name_to_interface_names.
+    interface_name_to_constructors = defaultdict(list)
+    for global_name, constructors in global_name_to_constructors.iteritems():
+        assert global_name in global_name_to_interface_names, \

[Nils Barth (inactive), 2014/05/07 01:55:57]

This assert is not needed, since it'll trigger an error on the next line.
If you'd like to catch the error, just use try:... except KeyError:...

More generally, asserts should *not* be used for bad input,
only for logic errors (incorrect assumptions).
(Among other reasons, b/c asserts can be turned off.)
https://wiki.python.org/moin/UsingAssertionsEffectively

[Inactive, 2014/05/07 15:45:12]

Done.

+               'Unknown global name: "%s", values in [Exposed=xx] must '\
+               'correspond to a [Global] or [PrimaryGlobal] extended '\
+               'attribute.' % global_name
+        for interface_name in global_name_to_interface_names[global_name]:
+            interface_name_to_constructors[interface_name].extend(constructors)
+
+    # Write partial interfaces containing constructor attributes for each
+    # global interface.
+    for interface_name, constructors in interface_name_to_constructors.iteritems():

[Nils Barth (inactive), 2014/05/07 01:55:57]

for interface_name, idl_filename in interface_name_idl_filename:

[Inactive, 2014/05/07 15:45:12]

Done.

         write_global_constructors_partial_interface(
             interface_name,
-            global_object['idl_filename'],
-            global_object['constructors'],
+            interface_name_to_idl_filename[interface_name],

[Nils Barth (inactive), 2014/05/07 01:55:57]

How about:
interface_name,
idl_filename,
interface_name_to_constructors[interface_name],

(Avoids an iteritems)

[Inactive, 2014/05/07 15:45:12]

Done.

+            constructors,
             options.write_file_only_if_changed)
 
 
 if __name__ == '__main__':
     sys.exit(main())