Bug 1166405 - Consolidate classes into a general web manifest module. r=ehsan.

Bound EXPORTED_SYMBOLS to `this` in WebManifest.jsm
Reduced number of iterations on random tests

---
 dom/ipc/manifestMessages.js                        |  17 +---
 ...ObjectProcessor.jsm => ImageObjectProcessor.js} |  35 ++++---
 .../{ManifestObtainer.jsm => ManifestObtainer.js}  |   4 +-
 ...{ManifestProcessor.jsm => ManifestProcessor.js} | 109 ++++++++++-----------
 ...anifestValueExtractor.jsm => ValueExtractor.js} |  25 +++--
 dom/manifest/WebManifest.jsm                       |  19 ++++
 dom/manifest/moz.build                             |   9 +-
 .../test/browser_ManifestObtainer_obtain.js        |   9 +-
 dom/manifest/test/common.js                        |  32 +++---
 9 files changed, 135 insertions(+), 124 deletions(-)
 rename dom/manifest/{ManifestImageObjectProcessor.jsm => ImageObjectProcessor.js} (81%)
 rename dom/manifest/{ManifestObtainer.jsm => ManifestObtainer.js} (95%)
 rename dom/manifest/{ManifestProcessor.jsm => ManifestProcessor.js} (69%)
 rename dom/manifest/{ManifestValueExtractor.jsm => ValueExtractor.js} (77%)
 create mode 100644 dom/manifest/WebManifest.jsm

dom/ipc/manifestMessages.js

@@ -3,29 +3,22 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.*/
 /*
  * Manifest obtainer frame script implementation of:
- * http://w3c.github.io/manifest/#obtaining
+ * http://www.w3.org/TR/appmanifest/#obtaining
  *
  * It searches a top-level browsing context for
  * a <link rel=manifest> element. Then fetches
  * and processes the linked manifest.
  *
  * BUG: https://bugzilla.mozilla.org/show_bug.cgi?id=1083410
- * exported ManifestObtainer
  */
-/*globals content, ManifestProcessor, XPCOMUtils, sendAsyncMessage, addMessageListener, Components*/
+/*globals content, sendAsyncMessage, addMessageListener, Components*/
 'use strict';
 const {
   utils: Cu
 } = Components;
-
-Cu.import('resource://gre/modules/XPCOMUtils.jsm');
-
-XPCOMUtils.defineLazyModuleGetter(this, 'ManifestProcessor',
-  'resource://gre/modules/ManifestProcessor.jsm');
-XPCOMUtils.defineLazyModuleGetter(this, 'ManifestObtainer',
-  'resource://gre/modules/ManifestObtainer.jsm');
-XPCOMUtils.defineLazyModuleGetter(this, 'BrowserUtils',
-  'resource://gre/modules/BrowserUtils.jsm');
+const {
+  ManifestProcessor
+} = Cu.import('resource://gre/modules/WebManifest.jsm', {});
 
 addMessageListener('DOM:ManifestObtainer:Obtain', (aMsg) => {
   fetchManifest()

dom/manifest/ImageObjectProcessor.js

@@ -1,12 +1,12 @@
 /* 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/. */
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 /*
- * ManifestImageObjectProcessor
+ * ImageObjectProcessor
  * Implementation of Image Object processing algorithms from:
- * http://www.w3.org/2008/webapps/manifest/#image-object-and-its-members
+ * http://www.w3.org/TR/appmanifest/#image-object-and-its-members
  *
- * This is intended to be used in conjunction with ManifestProcessor.jsm
+ * This is intended to be used in conjunction with ManifestProcessor.js
  *
  * Creates an object to process Image Objects as defined by the
  * W3C specification. This is used to process things like the
@@ -18,27 +18,25 @@
  *
  */
 /*exported EXPORTED_SYMBOLS*/
-/*globals Components*/
+/*globals Components */
 'use strict';
-this.EXPORTED_SYMBOLS = ['ManifestImageObjectProcessor']; // jshint ignore:line
-const imports = {};
 const {
   utils: Cu,
-  classes: Cc,
-  interfaces: Ci
+  interfaces: Ci,
+  classes: Cc
 } = Components;
-Cu.import('resource://gre/modules/XPCOMUtils.jsm');
+
 Cu.importGlobalProperties(['URL']);
-imports.netutil = Cc['@mozilla.org/network/util;1']
+const netutil = Cc['@mozilla.org/network/util;1']
   .getService(Ci.nsINetUtil);
 
-function ManifestImageObjectProcessor(aConsole, aExtractor) {
+function ImageObjectProcessor(aConsole, aExtractor) {
   this.console = aConsole;
   this.extractor = aExtractor;
 }
 
 // Static getters
-Object.defineProperties(ManifestImageObjectProcessor, {
+Object.defineProperties(ImageObjectProcessor, {
   'decimals': {
     get: function() {
       return /^\d+$/;
@@ -51,7 +49,7 @@ Object.defineProperties(ManifestImageObjectProcessor, {
   }
 });
 
-ManifestImageObjectProcessor.prototype.process = function(
+ImageObjectProcessor.prototype.process = function(
   aManifest, aBaseURL, aMemberName
 ) {
   const spec = {
@@ -94,7 +92,7 @@ ManifestImageObjectProcessor.prototype.process = function(
     };
     let value = extractor.extractValue(spec);
     if (value) {
-      value = imports.netutil.parseContentType(value, charset, hadCharset);
+      value = netutil.parseContentType(value, charset, hadCharset);
     }
     return value || undefined;
   }
@@ -144,7 +142,7 @@ ManifestImageObjectProcessor.prototype.process = function(
     // Implementation of HTML's link@size attribute checker.
     function isValidSizeValue(aSize) {
       const size = aSize.toLowerCase();
-      if (ManifestImageObjectProcessor.anyRegEx.test(aSize)) {
+      if (ImageObjectProcessor.anyRegEx.test(aSize)) {
         return true;
       }
       if (!size.includes('x') || size.indexOf('x') !== size.lastIndexOf('x')) {
@@ -155,7 +153,7 @@ ManifestImageObjectProcessor.prototype.process = function(
       const w = widthAndHeight.shift();
       const h = widthAndHeight.join('x');
       const validStarts = !w.startsWith('0') && !h.startsWith('0');
-      const validDecimals = ManifestImageObjectProcessor.decimals.test(w + h);
+      const validDecimals = ImageObjectProcessor.decimals.test(w + h);
       return (validStarts && validDecimals);
     }
   }
@@ -171,4 +169,5 @@ ManifestImageObjectProcessor.prototype.process = function(
     return extractor.extractColorValue(spec);
   }
 };
-this.ManifestImageObjectProcessor = ManifestImageObjectProcessor; // jshint ignore:line
+this.ImageObjectProcessor = ImageObjectProcessor; // jshint ignore:line
+this.EXPORTED_SYMBOLS = ['ImageObjectProcessor']; // jshint ignore:line

dom/manifest/ManifestObtainer.js

@@ -21,8 +21,6 @@
  * exported ManifestObtainer
  */
 'use strict';
-this.EXPORTED_SYMBOLS = ['ManifestObtainer'];
-
 const MSG_KEY = 'DOM:ManifestObtainer:Obtain';
 let messageCounter = 0;
 // FIXME: Ideally, we would store a reference to the
@@ -83,3 +81,5 @@ ManifestObtainer.prototype = {
     }
   }
 };
+this.ManifestObtainer = ManifestObtainer; // jshint ignore:line
+this.EXPORTED_SYMBOLS = ['ManifestObtainer']; // jshint ignore:line

dom/manifest/ManifestProcessor.js

@@ -12,26 +12,21 @@
  *
  *   .process({jsonText,manifestURL,docURL});
  *
- * Depends on ManifestImageObjectProcessor to process things like
+ * Depends on ImageObjectProcessor to process things like
  * icons and splash_screens.
  *
  * TODO: The constructor should accept the UA's supported orientations.
  * TODO: The constructor should accept the UA's supported display modes.
  * TODO: hook up developer tools to console. (1086997).
  */
-/*exported EXPORTED_SYMBOLS */
-/*JSLint options in comment below: */
-/*globals Components, XPCOMUtils, Intl*/
+/*globals Components*/
 'use strict';
-this.EXPORTED_SYMBOLS = ['ManifestProcessor']; // jshint ignore:line
-const imports = {};
 const {
-  utils: Cu
+  utils: Cu,
+  interfaces: Ci,
+  classes: Cc
 } = Components;
-Cu.import('resource://gre/modules/XPCOMUtils.jsm');
 Cu.importGlobalProperties(['URL']);
-XPCOMUtils.defineLazyModuleGetter(imports, 'Services',
-  'resource://gre/modules/Services.jsm');
 const displayModes = new Set(['fullscreen', 'standalone', 'minimal-ui',
   'browser'
 ]);
@@ -41,14 +36,16 @@ const orientationTypes = new Set(['any', 'natural', 'landscape', 'portrait',
 ]);
 const {
   ConsoleAPI
-} = Cu.import('resource://gre/modules/devtools/Console.jsm');
+} = Cu.import('resource://gre/modules/devtools/Console.jsm', {});
+// ValueExtractor is used by the various processors to get values
+// from the manifest and to report errors.
 const {
-  ManifestImageObjectProcessor: ImgObjProcessor
-} = Cu.import('resource://gre/modules/ManifestImageObjectProcessor.jsm');
-
+  ValueExtractor
+} = Cu.import('resource://gre/modules/ValueExtractor.js', {});
+// ImageObjectProcessor is used to process things like icons and images
 const {
-  ManifestValueExtractor
-} = Cu.import('resource://gre/modules/ManifestValueExtractor.jsm');
+  ImageObjectProcessor
+} = Cu.import('resource://gre/modules/ImageObjectProcessor.js', {});
 
 function ManifestProcessor() {}
 
@@ -75,11 +72,11 @@ ManifestProcessor.prototype = {
   // process() method processes JSON text into a clean manifest
   // that conforms with the W3C specification. Takes an object
   // expecting the following dictionary items:
-  //  * aJsonText: the JSON string to be processed.
-  //  * aManifestURL: the URL of the manifest, to resolve URLs.
-  //  * aDocURL: the URL of the owner doc, for security checks
+  //  * jsonText: the JSON string to be processed.
+  //  * manifestURL: the URL of the manifest, to resolve URLs.
+  //  * docURL: the URL of the owner doc, for security checks
   process({
-    jsonText: aJsonText,
+    jsonText,
     manifestURL: aManifestURL,
     docURL: aDocURL
   }) {
@@ -90,39 +87,37 @@ ManifestProcessor.prototype = {
     const docURL = new URL(aDocURL);
     let rawManifest = {};
     try {
-      rawManifest = JSON.parse(aJsonText);
+      rawManifest = JSON.parse(jsonText);
     } catch (e) {}
     if (typeof rawManifest !== 'object' || rawManifest === null) {
       let msg = 'Manifest needs to be an object.';
       console.warn(msg);
       rawManifest = {};
     }
-    const extractor = new ManifestValueExtractor(console);
-    const imgObjProcessor = new ImgObjProcessor(console, extractor);
+    const extractor = new ValueExtractor(console);
+    const imgObjProcessor = new ImageObjectProcessor(console, extractor);
     const processedManifest = {
-      'lang': processLangMember(rawManifest),
-      'start_url': processStartURLMember(rawManifest, manifestURL, docURL),
-      'display': processDisplayMember(rawManifest),
-      'orientation': processOrientationMember(rawManifest),
-      'name': processNameMember(rawManifest),
+      'lang': processLangMember(),
+      'start_url': processStartURLMember(),
+      'display': processDisplayMember(),
+      'orientation': processOrientationMember(),
+      'name': processNameMember(),
       'icons': imgObjProcessor.process(
         rawManifest, manifestURL, 'icons'
       ),
       'splash_screens': imgObjProcessor.process(
         rawManifest, manifestURL, 'splash_screens'
       ),
-      'short_name': processShortNameMember(rawManifest),
-      'theme_color': processThemeColorMember(rawManifest),
+      'short_name': processShortNameMember(),
+      'theme_color': processThemeColorMember(),
     };
-    processedManifest.scope = processScopeMember(rawManifest, manifestURL,
-      docURL, new URL(processedManifest['start_url'])); // jshint ignore:line
-
+    processedManifest.scope = processScopeMember();
     return processedManifest;
 
-    function processNameMember(aManifest) {
+    function processNameMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'name',
         expectedType: 'string',
         trim: true
@@ -130,10 +125,10 @@ ManifestProcessor.prototype = {
       return extractor.extractValue(spec);
     }
 
-    function processShortNameMember(aManifest) {
+    function processShortNameMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'short_name',
         expectedType: 'string',
         trim: true
@@ -141,10 +136,10 @@ ManifestProcessor.prototype = {
       return extractor.extractValue(spec);
     }
 
-    function processOrientationMember(aManifest) {
+    function processOrientationMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'orientation',
         expectedType: 'string',
         trim: true
@@ -157,10 +152,10 @@ ManifestProcessor.prototype = {
       return '';
     }
 
-    function processDisplayMember(aManifest) {
+    function processDisplayMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'display',
         expectedType: 'string',
         trim: true
@@ -172,34 +167,35 @@ ManifestProcessor.prototype = {
       return ManifestProcessor.defaultDisplayMode;
     }
 
-    function processScopeMember(aManifest, aManifestURL, aDocURL, aStartURL) {
+    function processScopeMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'scope',
         expectedType: 'string',
         trim: false
       };
       let scopeURL;
+      const startURL = new URL(processedManifest.start_url);
       const value = extractor.extractValue(spec);
       if (value === undefined || value === '') {
         return undefined;
       }
       try {
-        scopeURL = new URL(value, aManifestURL);
+        scopeURL = new URL(value, manifestURL);
       } catch (e) {
         let msg = 'The URL of scope is invalid.';
         console.warn(msg);
         return undefined;
       }
-      if (scopeURL.origin !== aDocURL.origin) {
+      if (scopeURL.origin !== docURL.origin) {
         let msg = 'Scope needs to be same-origin as Document.';
         console.warn(msg);
         return undefined;
       }
       // If start URL is not within scope of scope URL:
-      let isSameOrigin = aStartURL && aStartURL.origin !== scopeURL.origin;
-      if (isSameOrigin || !aStartURL.pathname.startsWith(scopeURL.pathname)) {
+      let isSameOrigin = startURL && startURL.origin !== scopeURL.origin;
+      if (isSameOrigin || !startURL.pathname.startsWith(scopeURL.pathname)) {
         let msg =
           'The start URL is outside the scope, so scope is invalid.';
         console.warn(msg);
@@ -208,27 +204,27 @@ ManifestProcessor.prototype = {
       return scopeURL.href;
     }
 
-    function processStartURLMember(aManifest, aManifestURL, aDocURL) {
+    function processStartURLMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'start_url',
         expectedType: 'string',
         trim: false
       };
-      let result = new URL(aDocURL).href;
+      let result = new URL(docURL).href;
       const value = extractor.extractValue(spec);
       if (value === undefined || value === '') {
         return result;
       }
       let potentialResult;
       try {
-        potentialResult = new URL(value, aManifestURL);
+        potentialResult = new URL(value, manifestURL);
       } catch (e) {
         console.warn('Invalid URL.');
         return result;
       }
-      if (potentialResult.origin !== aDocURL.origin) {
+      if (potentialResult.origin !== docURL.origin) {
         let msg = 'start_url must be same origin as document.';
         console.warn(msg);
       } else {
@@ -237,10 +233,10 @@ ManifestProcessor.prototype = {
       return result;
     }
 
-    function processThemeColorMember(aManifest) {
+    function processThemeColorMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'theme_color',
         expectedType: 'string',
         trim: true
@@ -248,10 +244,10 @@ ManifestProcessor.prototype = {
       return extractor.extractColorValue(spec);
     }
 
-    function processLangMember(aManifest) {
+    function processLangMember() {
       const spec = {
         objectName: 'manifest',
-        object: aManifest,
+        object: rawManifest,
         property: 'lang',
         expectedType: 'string',
         trim: true
@@ -270,3 +266,4 @@ ManifestProcessor.prototype = {
   }
 };
 this.ManifestProcessor = ManifestProcessor; // jshint ignore:line
+this.EXPORTED_SYMBOLS = ['ManifestProcessor']; // jshint ignore:line

dom/manifest/ValueExtractor.js

@@ -1,27 +1,22 @@
 /* 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 https://www.mozilla.org/MPL/2.0/. */
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 /*
  * Helper functions extract values from manifest members
  * and reports conformance violations.
  */
 /*globals Components*/
 'use strict';
-const imports = {};
 const {
   classes: Cc,
   interfaces: Ci
 } = Components;
-imports.DOMUtils = Cc['@mozilla.org/inspector/dom-utils;1']
-  .getService(Ci.inIDOMUtils);
 
-this.EXPORTED_SYMBOLS = ['ManifestValueExtractor']; // jshint ignore:line
-
-function ManifestValueExtractor(aConsole) {
+function ValueExtractor(aConsole) {
   this.console = aConsole;
 }
 
-ManifestValueExtractor.prototype = {
+ValueExtractor.prototype = {
   // This function takes a 'spec' object and destructures
   // it to extract a value. If the value is of th wrong type, it
   // warns the developer and returns undefined.
@@ -31,8 +26,8 @@ ManifestValueExtractor.prototype = {
   //  property: the name of the property being extracted.
   //  trim: boolean, if the value should be trimmed (used by string type).
   extractValue({
-    expectedType, object, objectName, property, trim
-  }) {
+      expectedType, object, objectName, property, trim
+    }) {
     const value = object[property];
     const isArray = Array.isArray(value);
     // We need to special-case "array", as it's not a JS primitive.
@@ -54,15 +49,17 @@ ManifestValueExtractor.prototype = {
   },
   extractColorValue(spec) {
     const value = this.extractValue(spec);
+    const DOMUtils = Cc['@mozilla.org/inspector/dom-utils;1']
+      .getService(Ci.inIDOMUtils);
     let color;
-    if (imports.DOMUtils.isValidCSSColor(value)) {
+    if (DOMUtils.isValidCSSColor(value)) {
       color = value;
-    } else {
+    } else if (value) {
       const msg = `background_color: ${value} is not a valid CSS color.`;
       this.console.warn(msg);
     }
     return color;
   }
 };
-
-this.ManifestValueExtractor = ManifestValueExtractor; // jshint ignore:line
+this.ValueExtractor = ValueExtractor; // jshint ignore:line
+this.EXPORTED_SYMBOLS = ['ValueExtractor']; // jshint ignore:line

dom/manifest/WebManifest.jsm

@@ -0,0 +1,19 @@
+/* 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 https://mozilla.org/MPL/2.0/. */
+/*exported EXPORTED_SYMBOLS, ManifestProcessor, ManifestObtainer*/
+/*globals Components */
+'use strict';
+const {
+  utils: Cu
+} = Components;
+
+this.EXPORTED_SYMBOLS = [
+  'ManifestObtainer',
+  'ManifestProcessor'
+];
+
+// Export public interfaces
+for (let symbl of EXPORTED_SYMBOLS) {
+  Cu.import(`resource://gre/modules/${symbl}.js`);
+}

dom/manifest/moz.build

@@ -5,10 +5,11 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 EXTRA_JS_MODULES += [
-    'ManifestImageObjectProcessor.jsm',
-    'ManifestObtainer.jsm',
-    'ManifestProcessor.jsm',
-    'ManifestValueExtractor.jsm'
+    'ImageObjectProcessor.js',
+    'ManifestObtainer.js',
+    'ManifestProcessor.js',
+    'ValueExtractor.js',
+    'WebManifest.jsm'
 ]
 
 MOCHITEST_MANIFESTS += ['test/mochitest.ini']

dom/manifest/test/browser_ManifestObtainer_obtain.js

@@ -1,7 +1,10 @@
 //Used by JSHint:
-/*global Cu, ManifestObtainer, BrowserTestUtils, add_task, SpecialPowers, todo_is, gBrowser, Assert*/
+/*global Cu, BrowserTestUtils, add_task, SpecialPowers, gBrowser, Assert*/
 'use strict';
-Cu.import('resource://gre/modules/ManifestObtainer.jsm', this);
+const {
+  ManifestObtainer
+} = Cu.import('resource://gre/modules/WebManifest.jsm', {});
+
 requestLongerTimeout(4); // e10s tests take time.
 const defaultURL =
   'http://example.org/tests/dom/manifest/test/resource.sjs';
@@ -189,7 +192,7 @@ add_task(function*() {
   // Flood random browsers with requests. Once promises settle, check that
   // responses all pass.
   const results = yield Promise.all((
-    for (browser of randBrowsers(browsers, 1000)) obtainer.obtainManifest(browser)
+    for (browser of randBrowsers(browsers, 100)) obtainer.obtainManifest(browser)
   ));
   const expected = 'Expect every manifest to have name equal to `pass`.';
   const pass = results.every(manifest => manifest.name === 'pass');

dom/manifest/test/common.js

@@ -3,18 +3,20 @@
  **/
 
 'use strict';
-const bsp = SpecialPowers.Cu.import('resource://gre/modules/ManifestProcessor.jsm'),
-  processor = new bsp.ManifestProcessor(),
-  manifestURL = new URL(document.location.origin + '/manifest.json'),
-  docURL = document.location,
-  seperators = '\u2028\u2029\u0020\u00A0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000',
-  lineTerminators = '\u000D\u000A\u2028\u2029',
-  whiteSpace = `${seperators}${lineTerminators}`,
-  typeTests = [1, null, {},
-    [], false
-  ],
-  data = {
-    jsonText: '{}',
-    manifestURL: manifestURL,
-    docURL: docURL
-  };
+const {
+  ManifestProcessor
+} = SpecialPowers.Cu.import('resource://gre/modules/WebManifest.jsm');
+const processor = new ManifestProcessor();
+const manifestURL = new URL(document.location.origin + '/manifest.json');
+const docURL = document.location;
+const seperators = '\u2028\u2029\u0020\u00A0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000';
+const lineTerminators = '\u000D\u000A\u2028\u2029';
+const whiteSpace = `${seperators}${lineTerminators}`;
+const typeTests = [1, null, {},
+  [], false
+];
+const data = {
+  jsonText: '{}',
+  manifestURL: manifestURL,
+  docURL: docURL
+};