Bug 1443317: Update WebIDL hook to allow ChromeOnly WebIDL changes without DOM peer review. r=gps,mystor
authorKris Maglione <maglione.k@gmail.com>
Mon, 05 Mar 2018 14:09:59 -0800
changeset 5827 7d8dc89dee25
parent 5826 5fd802067309
child 5828 83f1cda27356
push id2690
push usermaglione.k@gmail.com
push dateThu, 08 Mar 2018 18:48:05 +0000
treeherderversion-control-tools@7d8dc89dee25 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps, mystor
bugs1443317
Bug 1443317: Update WebIDL hook to allow ChromeOnly WebIDL changes without DOM peer review. r=gps,mystor MozReview-Commit-ID: 8pe4phuI9dY
hghooks/mozhghooks/prevent_webidl_changes.py
hghooks/tests/test-prevent-webidl.t
--- a/hghooks/mozhghooks/prevent_webidl_changes.py
+++ b/hghooks/mozhghooks/prevent_webidl_changes.py
@@ -87,16 +87,20 @@ def hook(ui, repo, hooktype, node, sourc
         'kchen@mozilla.com',       # Kan-Ru Chen
         'kanru@kanru.info',        # Kan-Ru Chen
         'bkelly@mozilla.com',      # Ben Kelly
         'ben@wanderview.com',      # Ben Kelly
         'nfroyd@mozilla.com',      # Nathan Froyd
         'continuation@gmail.com',  # Andrew McCreight
     ]
 
+    # The root directory for WebIDL files which contain only ChromeOnly
+    # interfaces, and do not require DOM peer review.
+    chrome_webidl_root = 'dom/chrome-webidl/'
+
     error = ""
     note = ""
     changesets = list(repo.changelog.revs(repo[node].rev()))
     if 'a=release' in repo.changectx(changesets[-1]).description().lower():
         # Accept the entire push for code uplifts.
         return 0
     # Loop through each changeset being added to the repository
     for i in reversed(changesets):
@@ -133,21 +137,28 @@ def hook(ui, repo, hooktype, node, sourc
               # they're committing.
               if any(peer == email for peer in authors):
                   return True
 
               return False
 
             # Only check WebIDL files here.
             if file.endswith('.webidl'):
-                if webidlReviewed is None:
-                    webidlReviewed = search(DOM_authors, DOM_peers)
-                if not webidlReviewed:
-                    error += "WebIDL file %s altered in changeset %s without DOM peer review\n" % (file, short(c.node()))
-                    note = "\nChanges to WebIDL files in this repo require review from a DOM peer in the form of r=...\nThis is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..\n"
+                if file.startswith(chrome_webidl_root):
+                    print ("Not enforcing DOM peer review for WebIDL file %s "
+                           "in changeset %s since it is in the chrome WebIDL "
+                           "root. Please make sure that it does not contain "
+                           "any web-visible binding definitions."
+                           % (file, short(c.node())))
+                else:
+                    if webidlReviewed is None:
+                        webidlReviewed = search(DOM_authors, DOM_peers)
+                    if not webidlReviewed:
+                        error += "WebIDL file %s altered in changeset %s without DOM peer review\n" % (file, short(c.node()))
+                        note = "\nChanges to WebIDL files in this repo require review from a DOM peer in the form of r=...\nThis is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..\n"
             # Only check the IPDL sync-messages.ini here.
             elif file.endswith('ipc/ipdl/sync-messages.ini'):
                 if syncIPCReviewed is None:
                     syncIPCReviewed = search(IPC_authors, IPC_peers)
                 if not syncIPCReviewed:
                     error += "sync-messages.ini altered in changeset %s without IPC peer review\n" % (short(c.node()))
                     note = "\nChanges to sync-messages.ini in this repo require review from a IPC peer in the form of r=...\nThis is to ensure that we behave responsibly by not adding sync IPC messages that cause performance issues needlessly. We appreciate your understanding..\n"
 
--- a/hghooks/tests/test-prevent-webidl.t
+++ b/hghooks/tests/test-prevent-webidl.t
@@ -332,16 +332,31 @@ Hook should not run when stripping
   pushing to $TESTTMP/server
   searching for changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
   (79921ab00c00 modifies servo/interface.webidl from Servo; not enforcing peer review)
 
+Editing a .webidl file that isn't in a web root should pass
+
+  $ mkdir -p dom/chrome-webidl
+  $ echo "[ChromeOnly] interface MozFoo{};" > dom/chrome-webidl/MozFoo.webidl
+  $ hg add dom/chrome-webidl/MozFoo.webidl
+  $ hg commit -m 'Bug 123 - Add MozFoo'
+  $ hg push
+  pushing to $TESTTMP/server
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  Not enforcing DOM peer review for WebIDL file dom/chrome-webidl/MozFoo.webidl in changeset e4bbe117b4fe since it is in the chrome WebIDL root. Please make sure that it does not contain any web-visible binding definitions.
+
 Editing the sync-messages.ini file without any review should fail
 
   $ mkdir -p ipc/ipdl
   $ echo "foo" > ipc/ipdl/sync-messages.ini
   $ hg add ipc/ipdl/sync-messages.ini
   $ hg commit -m 'Bug 123 - Add sync-messages.ini'
   $ hg push
   pushing to $TESTTMP/server
@@ -349,17 +364,17 @@ Editing the sync-messages.ini file witho
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
   
   
   ************************** ERROR ****************************
   
-  sync-messages.ini altered in changeset 89b6b37b743f without IPC peer review
+  sync-messages.ini altered in changeset d6b9948481a3 without IPC peer review
   
   
   Changes to sync-messages.ini in this repo require review from a IPC peer in the form of r=...
   This is to ensure that we behave responsibly by not adding sync IPC messages that cause performance issues needlessly. We appreciate your understanding..
   
   *************************************************************
   
   
@@ -377,17 +392,17 @@ Editing the sync-messages.ini file witho
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
   
   
   ************************** ERROR ****************************
   
-  sync-messages.ini altered in changeset da03659ab1db without IPC peer review
+  sync-messages.ini altered in changeset e5a374f9bf9b without IPC peer review
   
   
   Changes to sync-messages.ini in this repo require review from a IPC peer in the form of r=...
   This is to ensure that we behave responsibly by not adding sync IPC messages that cause performance issues needlessly. We appreciate your understanding..
   
   *************************************************************
   
   
@@ -401,9 +416,9 @@ Editing the sync-messages.ini file with 
   $ hg -q commit --amend -m 'Bug 123 - Add Bar; r=billm'
   $ hg push
   pushing to $TESTTMP/server
   searching for changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
-  You've received proper review from an IPC peer on the sync-messages.ini change(s) in commit b1e0bcac4341, thanks for paying enough attention.
+  You've received proper review from an IPC peer on the sync-messages.ini change(s) in commit b571eaaf6a18, thanks for paying enough attention.