Bug 1442931: Part 1 - Forbid web-visible interfaces outside of WebIDL root. r=mystor
authorKris Maglione <maglione.k@gmail.com>
Mon, 05 Mar 2018 14:21:38 -0800
changeset 461905 6cb20ada1a0aa1f6d621ba3c85ce9946a6f9841f
parent 461904 aa22c7964b317297f10ca8d58b64202d1c80b6b9
child 461906 195cbf3d34334978e5a9d101d4b79f899598159c
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmystor
bugs1442931
milestone60.0a1
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
dom/bindings/Configuration.py
dom/bindings/mozwebidlcodegen/__init__.py
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -13,29 +13,36 @@ 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):
         pass
 
 
+def isChildPath(path, basePath):
+    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=[]):
         DescriptorProvider.__init__(self)
 
         # Read the configuration file.
         glbl = {}
         execfile(filename, glbl)
         config = glbl['DOMInterfaces']
 
+        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 +96,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\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():
                     self.optimizedOutDescriptorNames.add(iface.identifier.name)
                     continue
--- a/dom/bindings/mozwebidlcodegen/__init__.py
+++ b/dom/bindings/mozwebidlcodegen/__init__.py
@@ -145,17 +145,17 @@ class WebIDLCodegenManager(LoggingMixin)
         'RegisterWorkerBindings.cpp',
         'RegisterWorkerDebuggerBindings.cpp',
         'RegisterWorkletBindings.cpp',
         'ResolveSystemBinding.cpp',
         'UnionTypes.cpp',
         'PrototypeList.cpp',
     }
 
-    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,
                  make_deps_target=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.
         """
         self.populate_logger()
 
         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._generated_events_stems_as_array)
         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)
         else:
             result[2].add(path)
 
 
 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')
     try:
         os.makedirs(cache_dir)
     except OSError as e:
         if e.errno != errno.EEXIST:
             raise
 
     return WebIDLCodegenManager(
         os.path.join(src_dir, 'Bindings.conf'),
+        webidl_root,
         inputs,
         os.path.join(dist_dir, 'include', 'mozilla', 'dom'),
         obj_dir,
         os.path.join(obj_dir, 'codegen.json'),
         cache_dir=cache_dir,
         # The make rules include a codegen.pp file containing dependencies.
         make_deps_path=os.path.join(obj_dir, 'codegen.pp'),
         make_deps_target='codegen.pp',