Bug 1418226 - Create a store for payment dialog unprivileged UI state. r=jaws draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 30 Nov 2017 18:13:21 -0800
changeset 706036 09dbe6851331dad7b7e81b58c65ac6f6c2eb28f6
parent 706034 e54bf53bda00c3543925a56baa12aab5d323625f
child 706429 33452110c0810194c825af76efa4201c047bfea5
push id91671
push usermozilla@noorenberghe.ca
push dateFri, 01 Dec 2017 02:13:46 +0000
reviewersjaws
bugs1418226
milestone59.0a1
Bug 1418226 - Create a store for payment dialog unprivileged UI state. r=jaws MozReview-Commit-ID: HHEYdXahhcI
browser/base/content/test/static/browser_all_files_referenced.js
toolkit/components/payments/.eslintrc.js
toolkit/components/payments/jar.mn
toolkit/components/payments/moz.build
toolkit/components/payments/res/PaymentsStore.js
toolkit/components/payments/test/unit/.eslintrc.js
toolkit/components/payments/test/unit/head.js
toolkit/components/payments/test/unit/test_PaymentsStore.js
toolkit/components/payments/test/unit/xpcshell.ini
--- a/browser/base/content/test/static/browser_all_files_referenced.js
+++ b/browser/base/content/test/static/browser_all_files_referenced.js
@@ -15,16 +15,19 @@ var gExceptionPaths = [
   "chrome://browser/locale/searchplugins/",
   "resource://app/defaults/blocklists/",
   "resource://app/defaults/pinning/",
   "resource://app/defaults/preferences/",
   "resource://gre/modules/commonjs/",
   "resource://gre/defaults/pref/",
   "resource://shield-recipe-client/node_modules/jexl/lib/",
 
+  // These resources are referenced using relative paths from html files.
+  "resource://payments/",
+
   // https://github.com/mozilla/normandy/issues/577
   "resource://shield-recipe-client/test/",
 
   // https://github.com/mozilla/activity-stream/issues/3053
   "resource://activity-stream/data/content/tippytop/images/",
   // https://github.com/mozilla/activity-stream/issues/3758
   "resource://activity-stream/prerendered/",
 
--- a/toolkit/components/payments/.eslintrc.js
+++ b/toolkit/components/payments/.eslintrc.js
@@ -25,17 +25,17 @@ module.exports = {
       },
       // XXX: following line is used in eslint v4 to not throw an error when chaining methods
       //MemberExpression: "off",
       outerIIFEBody: 0,
     }],
     "max-len": ["error", 100],
     "max-nested-callbacks": ["error", 4],
     "new-parens": "error",
-    "no-console": "error",
+    "no-console": ["error", { allow: ["error"] }],
     "no-fallthrough": "error",
     "no-multi-str": "error",
     "no-multiple-empty-lines": ["error", {
       max: 2,
     }],
     "no-proto": "error",
     "no-throw-literal": "error",
     "no-unused-expressions": "error",
--- a/toolkit/components/payments/jar.mn
+++ b/toolkit/components/payments/jar.mn
@@ -5,13 +5,14 @@
 toolkit.jar:
 %   content payments %content/payments/
     content/payments/paymentDialog.css                (content/paymentDialog.css)
     content/payments/paymentDialog.js                 (content/paymentDialog.js)
     content/payments/paymentDialogFrameScript.js      (content/paymentDialogFrameScript.js)
     content/payments/paymentDialog.xhtml              (content/paymentDialog.xhtml)
 %   resource payments %res/payments/
     res/payments                                      (res/paymentRequest.*)
+    res/payments/components/                          (res/components/*.js)
     res/payments/debugging.html                       (res/debugging.html)
     res/payments/debugging.js                         (res/debugging.js)
-    res/payments/components/                          (res/components/*.js)
     res/payments/mixins/                              (res/mixins/*.js)
+    res/payments/PaymentsStore.js                     (res/PaymentsStore.js)
     res/payments/vendor/                              (res/vendor/*)
--- a/toolkit/components/payments/moz.build
+++ b/toolkit/components/payments/moz.build
@@ -18,8 +18,10 @@ JAR_MANIFESTS += ['jar.mn']
 
 MOCHITEST_MANIFESTS += ['test/mochitest/mochitest.ini']
 
 SPHINX_TREES['docs'] = 'docs'
 
 TESTING_JS_MODULES += [
     'test/PaymentTestUtils.jsm',
 ]
+
+XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
new file mode 100644
--- /dev/null
+++ b/toolkit/components/payments/res/PaymentsStore.js
@@ -0,0 +1,92 @@
+/* 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/. */
+
+"use strict";
+
+/**
+ * The PaymentsStore class provides lightweight storage with an async publish/subscribe mechanism.
+ * Synchronous state changes are batched to improve application performance and to reduce partial
+ * state propagation.
+ */
+
+/* exported PaymentsStore */
+
+class PaymentsStore {
+  /**
+   * @param {object} [defaultState = {}] The initial state of the store.
+   */
+  constructor(defaultState = {}) {
+    this._state = defaultState;
+    this._nextNotifification = 0;
+    this._subscribers = new Set();
+  }
+
+  /**
+   * Get the current state as a shallow clone with a shallow freeze.
+   * You shouldn't modify any part of the returned state object as that would bypass notifying
+   * subscribers and could lead to subscribers assuming old state.
+   *
+   * @returns {Object} containing the current state
+   */
+  getState() {
+    return Object.freeze(Object.assign({}, this._state));
+  }
+
+  /**
+   * Augment the current state with the keys of `obj` and asynchronously notify
+   * state subscribers. As a result, multiple synchronous state changes will lead
+   * to a single subscriber notification which leads to better performance and
+   * reduces partial state changes.
+   *
+   * @param {Object} obj The object to augment the state with. Keys in the object
+   *                     will be shallow copied with Object.assign.
+   *
+   * @example If the state is currently {a:3} then setState({b:"abc"}) will result in a state of
+   *          {a:3, b:"abc"}.
+   */
+  async setState(obj) {
+    Object.assign(this._state, obj);
+    let thisChangeNum = ++this._nextNotifification;
+
+    // Let any synchronous setState calls that happen after the current setState call
+    // complete first.
+    // Their effects on the state will be batched up before the callback is actually called below.
+    await Promise.resolve();
+
+    // Don't notify for state changes that are no longer the most recent. We only want to call the
+    // callback once with the latest state.
+    if (thisChangeNum !== this._nextNotifification) {
+      return;
+    }
+
+    for (let subscriber of this._subscribers) {
+      try {
+        subscriber.stateChangeCallback(this.getState());
+      } catch (ex) {
+        console.error(ex);
+      }
+    }
+  }
+
+  /**
+   * Subscribe the object to state changes notifications via a `stateChangeCallback` method.
+   *
+   * @param {Object} component to receive state change callbacks via a `stateChangeCallback` method.
+   *                           If the component is already subscribed, do nothing.
+   */
+  subscribe(component) {
+    if (this._subscribers.has(component)) {
+      return;
+    }
+
+    this._subscribers.add(component);
+  }
+
+  /**
+   * @param {Object} component to stop receiving state change callbacks.
+   */
+  unsubscribe(component) {
+    this._subscribers.delete(component);
+  }
+}
new file mode 100644
--- /dev/null
+++ b/toolkit/components/payments/test/unit/.eslintrc.js
@@ -0,0 +1,7 @@
+"use strict";
+
+module.exports = {
+  "extends": [
+    "plugin:mozilla/xpcshell-test"
+  ]
+};
new file mode 100644
--- /dev/null
+++ b/toolkit/components/payments/test/unit/head.js
@@ -0,0 +1,11 @@
+const {interfaces: Ci, classes: Cc, results: Cr, utils: Cu} = Components;
+
+Cu.import("resource://gre/modules/Services.jsm");
+
+// ================================================
+// Load mocking/stubbing library, sinon
+// docs: http://sinonjs.org/releases/v2.3.2/
+Cu.import("resource://gre/modules/Timer.jsm");
+Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js", this);
+/* globals sinon */
+// ================================================
new file mode 100644
--- /dev/null
+++ b/toolkit/components/payments/test/unit/test_PaymentsStore.js
@@ -0,0 +1,126 @@
+"use strict";
+
+/* import-globals-from ../../res/PaymentsStore.js */
+Services.scriptloader.loadSubScript("resource://payments/PaymentsStore.js", this);
+
+add_task(async function test_defaultState() {
+  do_check_true(!!PaymentsStore, "Check PaymentsStore import");
+  let ps = new PaymentsStore({
+    foo: "bar",
+  });
+
+  let state = ps.getState();
+  do_check_true(!!state, "Check state is truthy");
+  do_check_eq(state.foo, "bar", "Check .foo");
+
+  Assert.throws(() => state.foo = "new", TypeError, "Assigning to existing prop. should throw");
+  Assert.throws(() => state.other = "something", TypeError, "Adding a new prop. should throw");
+  Assert.throws(() => delete state.foo, TypeError, "Deleting a prop. should throw");
+});
+
+add_task(async function test_setState() {
+  let ps = new PaymentsStore({});
+
+  ps.setState({
+    one: "one",
+  });
+  let state = ps.getState();
+  do_check_eq(Object.keys(state).length, 1, "Should only have 1 prop. set");
+  do_check_eq(state.one, "one", "Check .one");
+
+  ps.setState({
+    two: 2,
+  });
+  state = ps.getState();
+  do_check_eq(Object.keys(state).length, 2, "Should have 2 props. set");
+  do_check_eq(state.one, "one", "Check .one");
+  do_check_eq(state.two, 2, "Check .two");
+
+  ps.setState({
+    one: "a",
+    two: "b",
+  });
+  state = ps.getState();
+  do_check_eq(state.one, "a", "Check .one");
+  do_check_eq(state.two, "b", "Check .two");
+
+  do_print("check consecutive setState for the same prop");
+  ps.setState({
+    one: "c",
+  });
+  ps.setState({
+    one: "d",
+  });
+  state = ps.getState();
+  do_check_eq(Object.keys(state).length, 2, "Should have 2 props. set");
+  do_check_eq(state.one, "d", "Check .one");
+  do_check_eq(state.two, "b", "Check .two");
+});
+
+add_task(async function test_subscribe_unsubscribe() {
+  let ps = new PaymentsStore({});
+  let subscriber = {
+    stateChangePromise: null,
+    _stateChangeResolver: null,
+
+    reset() {
+      this.stateChangePromise = new Promise(resolve => {
+        this._stateChangeResolver = resolve;
+      });
+    },
+
+    stateChangeCallback(state) {
+      this._stateChangeResolver(state);
+      this.stateChangePromise = new Promise(resolve => {
+        this._stateChangeResolver = resolve;
+      });
+    },
+  };
+
+  sinon.spy(subscriber, "stateChangeCallback");
+  subscriber.reset();
+  ps.subscribe(subscriber);
+  do_print("subscribe the same listener twice to ensure it still doesn't call the callback");
+  ps.subscribe(subscriber);
+  do_check_true(subscriber.stateChangeCallback.notCalled,
+                "Check not called synchronously when subscribing");
+
+  let changePromise = subscriber.stateChangePromise;
+  ps.setState({
+    a: 1,
+  });
+  do_check_true(subscriber.stateChangeCallback.notCalled,
+                "Check not called synchronously for changes");
+  let state = await changePromise;
+  do_check_eq(state, subscriber.stateChangeCallback.getCall(0).args[0],
+              "Check resolved state is last state");
+  do_check_eq(JSON.stringify(state), `{"a":1}`, "Check callback state");
+
+  do_print("Testing consecutive setState");
+  subscriber.reset();
+  subscriber.stateChangeCallback.reset();
+  changePromise = subscriber.stateChangePromise;
+  ps.setState({
+    a: 2,
+  });
+  ps.setState({
+    a: 3,
+  });
+  do_check_true(subscriber.stateChangeCallback.notCalled,
+                "Check not called synchronously for changes");
+  state = await changePromise;
+  do_check_eq(state, subscriber.stateChangeCallback.getCall(0).args[0],
+              "Check resolved state is last state");
+  do_check_eq(JSON.stringify(subscriber.stateChangeCallback.getCall(0).args[0]), `{"a":3}`,
+              "Check callback state matches second setState");
+
+  do_print("test unsubscribe");
+  subscriber.stateChangeCallback = function unexpectedChange() {
+    do_check_true(false, "stateChangeCallback shouldn't be called after unsubscribing");
+  };
+  ps.unsubscribe(subscriber);
+  ps.setState({
+    a: 4,
+  });
+  await Promise.resolve("giving a chance for the callback to be called");
+});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/payments/test/unit/xpcshell.ini
@@ -0,0 +1,4 @@
+[DEFAULT]
+head = head.js
+
+[test_PaymentsStore.js]