jwir3 / checkiid Goto Github PK
View Code? Open in Web Editor NEWScript for checking interfaces for corresponding IID changes in mozilla code
Script for checking interfaces for corresponding IID changes in mozilla code
Right now, checkiid.py operates on an input (from stdin) of a diff file. It should, in addition, construct this diff file and run on it. This way, we can use it as a standalone tool, as well as an hg hook.
This should probably be used in combination with a set of revisions switches, so that we can use it incrementally.
The function IDLDescriptor.areDescriptorsInLineAffectingBinaryCompat() checks:
What it should be checking is:
Currently, all the python classes are in a single file, checkiid.py. We should pull separate classes out into separate modules, to better modularize the code.
checkiid is triggering an error on comment changes. For example, on this changeset:
http://hg.mozilla.org/mozilla-central/rev/feded4c79308
Maybe it should ignore line starting with:
^+[:space:]*
The following incorrectly flags nsIDOMDeviceProximityEvent as needing an IID change:
diff --git a/dom/interfaces/events/nsIDOMDeviceProximityEvent.idl b/dom/interfaces/events/nsIDOMDeviceProximityEvent.idl
--- a/dom/interfaces/events/nsIDOMDeviceProximityEvent.idl
+++ b/dom/interfaces/events/nsIDOMDeviceProximityEvent.idl
@@ -16,12 +16,12 @@ interface nsIDOMDeviceProximityEvent : n
readonly attribute double value;
readonly attribute double min;
readonly attribute double max;
};
dictionary DeviceProximityEventInit : EventInit
{
- double value;
- double min;
- double max;
+ double value = Infinity;
+ double min = -Infinity;
+ double max = Infinity;
};
We currently have a readme file, generated by github, in the root of the project directory, but it would be nice if we could add some documentation to this script, both in the readme file, and on the mozilla wiki, that details how to use the script.
It would be nice if the output of the script was easier to read by adding colors.
If a file is not found the script will crash inside of findAllSpecialBlocksForFile. We should add an extra try block so that this error condition fails gracefully instead of crashing.
From the README.md file:
$ cat /tmp/firefox.diff | python /path/to/checkiid.py .
usage: checkiid.py [-h] [-V] [-d] [-t ] [-n]
repo [inputfile]
checkiid.py: error: too few arguments
but
$ python /path/to/checkiid.py repo /tmp/firefox.diff
seems to work
If you run one unit test, then another one, and they don't have the same output, then the /tmp/ file is not overwritten. We should probably use an actual temp file, rather than a manually generated one (that's the same every time), and even probably delete it after we're done using it.
When a diff of the following nature is found:
diff --git a/content/base/public/nsIMessageManager.idl b/content/base/public/nsIMessageManager.idl
--- a/content/base/public/nsIMessageManager.idl
+++ b/content/base/public/nsIMessageManager.idl
@@ -331,18 +331,18 @@ interface nsIFrameScriptLoader : nsISupp
void loadFrameScript(in AString aURL, in boolean aAllowDelayedLoad);
/**
* Removes aURL from the list of scripts which support delayed load.
*/
void removeDelayedFrameScript(in AString aURL);
};
-[scriptable, builtinclass, uuid(5f552699-01a2-4f17-833b-ddb3fa0d98b2)]
-interface nsIPermissionChecker : nsISupports
+[scriptable, builtinclass, uuid(134ccbf0-5c08-11e2-bcfd-0800200c9a66)]
+interface nsIProcessChecker : nsISupports
{
/**
* Return true iff the "remote" process has |aPermission|. This is
* intended to be used by JS implementations of cross-process DOM
* APIs, like so
*
* recvFooRequest: function(message) {
@@ -356,9 +356,32 @@ interface nsIPermissionChecker : nsISupp
* then applying this permission check doesn't add any security,
* though it doesn't hurt anything either.
*
* Note: If the remote content process does *not* have |aPermission|,
* it will be killed as a precaution.
*/
boolean assertPermission(in DOMString aPermission);
+ /**
+ * Return true iff the "remote" process has |aManifestURL|. This is
+ * intended to be used by JS implementations of cross-process DOM
+ * APIs, like so
+ *
+ * recvFooRequest: function(message) {
+ * if (!message.target.assertContainApp("foo")) {
+ * return false;
+ * }
+ * // service foo request
+ *
+ * This interface only returns meaningful data when our content is
+ * in a separate process. If it shares the same OS process as us,
+ * then applying this manifest URL check doesn't add any security,
+ * though it doesn't hurt anything either.
+ *
+ * Note: If the remote content process does *not* contain |aManifestURL|,
+ * it will be killed as a precaution.
+ */
+ boolean assertContainApp(in DOMString aManifestURL);
+
+ boolean assertAppHasPermission(in DOMString aPermission);
+
};
The script thinks that 'nsIPermissionChecker' needs a new IID, because of the context line:
@@ -356,9 +356,32 @@ interface nsIPermissionChecker : nsISupp
We should cache a renamed interface so this doesn't confuse the script.
Running checkiid on the new beta (31), I am getting plenty of warnings:
WARNING: 'nsIDocumentEncoder.idl' was not found in local repository. Are you sure your repository is at the correct revision?
For this file, its path hasn't change for a while (at least 2008).
Path + patch look good:
$ find . -iname nsIDocumentEncoder.idl
./content/base/public/nsIDocumentEncoder.idl
$ grep nsIDocumentEncoder.idl /tmp/firefox.diff
diff --git a/content/base/public/nsIDocumentEncoder.idl b/content/base/public/nsIDocumentEncoder.idl
--- a/content/base/public/nsIDocumentEncoder.idl
+++ b/content/base/public/nsIDocumentEncoder.idl
If we have input like the following (i.e. in the IDL file, not the diff):
If we have something like the following:
attribute wstring plexName; /* name of plex mode (like "simplex", "duplex",
* "tumble" and various custom values) */
checkiid.py will fail to construct a correct block comment for this, because the first line doesn't //begin// with a comment block. We're going to have to figure out a way to get it to recognize characters, but only in specific cases.
The checkiid module should be better encapsulated into a class. This will enable us to better represent it as a state machine (which it already is), but will allow us to keep "context" where the state is more understandable.
When the diff file has input like the following:
diff --git a/dom/interfaces/canvas/nsIDOMCanvasRenderingContext2D.idl b/dom/interfaces/canvas/nsIDOMCanvasRenderingContext2D.idl
--- a/dom/interfaces/canvas/nsIDOMCanvasRenderingContext2D.idl
+++ b/dom/interfaces/canvas/nsIDOMCanvasRenderingContext2D.idl
@@ -1,32 +1,15 @@
/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "nsISupports.idl"
-[scriptable, uuid(bbb20a59-524e-4662-981e-5e142814b20c)]
-interface nsIDOMCanvasGradient : nsISupports
-{
- void addColorStop(in float offset, in DOMString color);
-};
-
-[scriptable, uuid(21dea65c-5c08-4eb1-ac82-81fe95be77b8)]
-interface nsIDOMCanvasPattern : nsISupports
-{
-};
-
-[scriptable, uuid(2d01715c-ec7d-424a-ab85-e0fd70c8665c)]
-interface nsIDOMTextMetrics : nsISupports
-{
- readonly attribute float width;
-};
-
The script incorrectly reports these interfaces as needing IID changes. Any time an interface has been completely removed from a file, the script should identify this as not needing an IID change.
During installation, the setup.py script should be installing idlutils and prettyprinter to a system python directory (site-packages), but, at least on mac, it's not doing this properly.
There are inflexibilities with the current test suite. Namely, each test is based off of a specific revision range (as well as tree), and it's likely that, at the rate these are changing, the tests will become invalid very quickly.
We should setup some method of checking the commits on the given hg repo (and probably that the tree is the same tree), and aborting the test if we don't have the correct commits.
If a .ref file has comments (e.g. lines prefixed by '#'), these should be ignored when testing for equivalence in the file and the output from the script.
One of the issues with the current system is that the script expects that the hg repository given is at the same revision as the specified end revision. We should make sure that this is the case, since this can be done programmatically, and then re-checkout the revision that the user had prior.
This will likely eliminate some potential future confusion.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.