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

Source/bindings/scripts/generate_global_constructors.py

Index: Source/bindings/scripts/generate_global_constructors.py
diff --git a/Source/bindings/scripts/generate_global_constructors.py b/Source/bindings/scripts/generate_global_constructors.py
index 3889f132077826bf8919a616c33eef62c38aeb03..7504b09d3990c6a41d59d9a269e2b39f916f5459 100755
--- a/Source/bindings/scripts/generate_global_constructors.py
+++ b/Source/bindings/scripts/generate_global_constructors.py
@@ -22,9 +22,11 @@ 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 = """
@@ -49,13 +51,22 @@ def parse_options():
     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):
@@ -73,13 +84,22 @@ def record_global_constructors(idl_filename):
         '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):
@@ -139,23 +159,32 @@ def main():
     # 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)