Bug 1442931: Part 1 - Forbid web-visible interfaces outside of WebIDL root. r=mystor
☠☠ backed out by 5c7eb716937a ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Mon, 05 Mar 2018 14:21:38 -0800
changeset 406837 608e21fcd1674dae3f8b685cedab85c43ffdb485
parent 406836 44b331a2a53d9d12106c1711814ab80a2cd8829e
child 406838 9f46e7d52b9b2e30bf0ccf64bb5805168dd79c29
push id100524
push usermaglione.k@gmail.com
push dateWed, 07 Mar 2018 05:48:09 +0000
treeherdermozilla-inbound@9f46e7d52b9b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1442931: Part 1 - Forbid web-visible interfaces outside of WebIDL root. r=mystor Web-visible WebIDL interfaces require DOM peer review with every change, which is enforced by a commit hook. ChromeOnly interfaces are not exposed to the web, and therefore don't require the same strictures. The current commit hook enforces the review requirement for changes to any (non-Servo) WebIDL file, and is not smart enough to determine if the change is web-visible. In order to loosen that restriction, we need the build system to enforce the requirement that only WebIDL files in certain locations may contain web-visible interfaces, so that the commit hook can restrict itself to checking only those directories. This change restricts the location of web-visible WebIDL interfaces to the dom/webidl/ and dom/bindings/ roots (along with the corresponding objdir root for generated interfaces). A follow-up will change the commit hook to only enforce review requirements on these directories. MozReview-Commit-ID: CiDxXxN4oO4
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -13,29 +13,38 @@ class DescriptorProvider:
     A way of getting descriptors for interface names.  Subclasses must
     have a getDescriptor method callable with the interface name only.
     def __init__(self):
+def isChildPath(path, basePath):
+    path = os.path.normpath(path)
+    return os.path.commonprefix((path, basePath)) == basePath
 class Configuration(DescriptorProvider):
     Represents global configuration state based on IDL parse data and
     the configuration file.
-    def __init__(self, filename, parseData, generatedEvents=[]):
+    def __init__(self, filename, webRoots, parseData, generatedEvents=[]):
         # Read the configuration file.
         glbl = {}
         execfile(filename, glbl)
         config = glbl['DOMInterfaces']
+        webRoots = tuple(map(os.path.normpath, webRoots))
+        def isInWebIDLRoot(path):
+            return any(isChildPath(path, root) for root in webRoots)
         # Build descriptors for all the interfaces we have in the parse data.
         # This allows callers to specify a subset of interfaces by filtering
         # |parseData|.
         self.descriptors = []
         self.interfaces = {}
         self.descriptorsByName = {}
         self.optimizedOutDescriptorNames = set()
         self.generatedEvents = generatedEvents
@@ -89,16 +98,27 @@ class Configuration(DescriptorProvider):
                         raise TypeError(
                             "The binding build system doesn't really support "
                             "partial interfaces which don't appear in the "
                             "file in which the interface they are extending is "
                             "defined.  Don't do this.\n"
                             "%s" %
                             (partialIface.location, iface.location))
+                if not (iface.getExtendedAttribute("ChromeOnly") or
+                        not (iface.hasInterfaceObject() or
+                             iface.isNavigatorProperty()) or
+                        isInWebIDLRoot(iface.filename())):
+                    raise TypeError(
+                        "Interfaces which are exposed to the web may only be "
+                        "defined in a DOM WebIDL root %r. Consider marking "
+                        "the interface [ChromeOnly] if you do not want it "
+                        "exposed to the web.\n"
+                        "%s" %
+                        (webRoots, iface.location))
             self.interfaces[iface.identifier.name] = iface
             if iface.identifier.name not in config:
                 # Completely skip consequential interfaces with no descriptor
                 # if they have no interface object because chances are we
                 # don't need to do anything interesting with them.
                 if iface.isConsequential() and not iface.hasInterfaceObject():
--- a/dom/bindings/mozwebidlcodegen/__init__.py
+++ b/dom/bindings/mozwebidlcodegen/__init__.py
@@ -145,17 +145,17 @@ class WebIDLCodegenManager(LoggingMixin)
-    def __init__(self, config_path, inputs, exported_header_dir,
+    def __init__(self, config_path, webidl_root, inputs, exported_header_dir,
                  codegen_dir, state_path, cache_dir=None, make_deps_path=None,
         """Create an instance that manages WebIDLs in the build system.
         config_path refers to a WebIDL config file (e.g. Bindings.conf).
         inputs is a 4-tuple describing the input .webidl files and how to
         process them. Members are:
             (set(.webidl files), set(basenames of exported files),
@@ -171,16 +171,17 @@ class WebIDLCodegenManager(LoggingMixin)
         make_deps_target is the target that receives the make dependencies. It
         must be defined if using make_deps_path.
         input_paths, exported_stems, generated_events_stems, example_interfaces = inputs
         self._config_path = config_path
+        self._webidl_root = webidl_root
         self._input_paths = set(input_paths)
         self._exported_stems = set(exported_stems)
         self._generated_events_stems = set(generated_events_stems)
         self._generated_events_stems_as_array = generated_events_stems
         self._example_interfaces = set(example_interfaces)
         self._exported_header_dir = exported_header_dir
         self._codegen_dir = codegen_dir
         self._state_path = state_path
@@ -327,18 +328,36 @@ class WebIDLCodegenManager(LoggingMixin)
         parser = WebIDL.Parser(self._cache_dir)
         for path in sorted(self._input_paths):
             with open(path, 'rb') as fh:
                 data = fh.read()
                 hashes[path] = hashlib.sha1(data).hexdigest()
                 parser.parse(data, path)
+        # Only these directories may contain WebIDL files with interfaces
+        # which are exposed to the web. WebIDL files in these roots may not
+        # be changed without DOM peer review.
+        #
+        # Other directories may contain WebIDL files as long as they only
+        # contain ChromeOnly interfaces. These are not subject to mandatory
+        # DOM peer review.
+        web_roots = (
+            # The main WebIDL root.
+            self._webidl_root,
+            # The binding config root, which contains some test-only
+            # interfaces.
+            os.path.dirname(self._config_path),
+            # The objdir sub-directory which contains generated WebIDL files.
+            self._codegen_dir,
+        )
         self._parser_results = parser.finish()
-        self._config = Configuration(self._config_path, self._parser_results,
+        self._config = Configuration(self._config_path, web_roots,
+                                     self._parser_results,
         self._input_hashes = hashes
     def _write_global_derived(self):
         from Codegen import GlobalGenRoots
         things = [('declare', f) for f in self.GLOBAL_DECLARE_FILES]
         things.extend(('define', f) for f in self.GLOBAL_DEFINE_FILES)
@@ -541,32 +560,34 @@ class WebIDLCodegenManager(LoggingMixin)
 def create_build_system_manager(topsrcdir, topobjdir, dist_dir):
     """Create a WebIDLCodegenManager for use by the build system."""
     src_dir = os.path.join(topsrcdir, 'dom', 'bindings')
     obj_dir = os.path.join(topobjdir, 'dom', 'bindings')
+    webidl_root = os.path.join(topsrcdir, 'dom', 'webidl')
     with open(os.path.join(obj_dir, 'file-lists.json'), 'rb') as fh:
         files = json.load(fh)
     inputs = (files['webidls'], files['exported_stems'],
               files['generated_events_stems'], files['example_interfaces'])
     cache_dir = os.path.join(obj_dir, '_cache')
     except OSError as e:
         if e.errno != errno.EEXIST:
     return WebIDLCodegenManager(
         os.path.join(src_dir, 'Bindings.conf'),
+        webidl_root,
         os.path.join(dist_dir, 'include', 'mozilla', 'dom'),
         os.path.join(obj_dir, 'codegen.json'),
         # The make rules include a codegen.pp file containing dependencies.
         make_deps_path=os.path.join(obj_dir, 'codegen.pp'),
--- a/dom/bindings/mozwebidlcodegen/test/test_mozwebidlcodegen.py
+++ b/dom/bindings/mozwebidlcodegen/test/test_mozwebidlcodegen.py
@@ -66,16 +66,17 @@ class TestWebIDLCodegenManager(unittest.
             {mozpath.splitext(mozpath.basename(p))[0] for p in ip},
         return dict(
+            webidl_root='/',
             exported_header_dir=mozpath.join(tmp, 'exports'),
             codegen_dir=mozpath.join(tmp, 'codegen'),
             state_path=mozpath.join(tmp, 'state.json'),
             make_deps_path=mozpath.join(tmp, 'codegen.pp'),