From 25a9e2348cdccaf88660303a21f23315209038d2 Mon Sep 17 00:00:00 2001 From: Matthew Noorenberghe Date: Thu, 30 Nov 2017 18:13:21 -0800 Subject: [PATCH] Bug 1418226 - Create a store for payment dialog unprivileged UI state. r=jaws MozReview-Commit-ID: HHEYdXahhcI --HG-- extra : rebase_source : 09dbe6851331dad7b7e81b58c65ac6f6c2eb28f6 --- .../static/browser_all_files_referenced.js | 3 + toolkit/components/payments/.eslintrc.js | 2 +- toolkit/components/payments/jar.mn | 3 +- toolkit/components/payments/moz.build | 2 + .../components/payments/res/PaymentsStore.js | 92 +++++++++++++ .../payments/test/unit/.eslintrc.js | 7 + toolkit/components/payments/test/unit/head.js | 11 ++ .../payments/test/unit/test_PaymentsStore.js | 126 ++++++++++++++++++ .../payments/test/unit/xpcshell.ini | 4 + 9 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 toolkit/components/payments/res/PaymentsStore.js create mode 100644 toolkit/components/payments/test/unit/.eslintrc.js create mode 100644 toolkit/components/payments/test/unit/head.js create mode 100644 toolkit/components/payments/test/unit/test_PaymentsStore.js create mode 100644 toolkit/components/payments/test/unit/xpcshell.ini diff --git a/browser/base/content/test/static/browser_all_files_referenced.js b/browser/base/content/test/static/browser_all_files_referenced.js index 602dc584694d..e48c9e388f2e 100644 --- a/browser/base/content/test/static/browser_all_files_referenced.js +++ b/browser/base/content/test/static/browser_all_files_referenced.js @@ -20,6 +20,9 @@ var gExceptionPaths = [ "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/", diff --git a/toolkit/components/payments/.eslintrc.js b/toolkit/components/payments/.eslintrc.js index 3beca6564bfb..06f0bb37ea47 100644 --- a/toolkit/components/payments/.eslintrc.js +++ b/toolkit/components/payments/.eslintrc.js @@ -30,7 +30,7 @@ module.exports = { "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", { diff --git a/toolkit/components/payments/jar.mn b/toolkit/components/payments/jar.mn index a05a66c1d82b..d577756f2caa 100644 --- a/toolkit/components/payments/jar.mn +++ b/toolkit/components/payments/jar.mn @@ -10,8 +10,9 @@ toolkit.jar: 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/*) diff --git a/toolkit/components/payments/moz.build b/toolkit/components/payments/moz.build index ad93a1b8a3ee..b461b7fb3d18 100644 --- a/toolkit/components/payments/moz.build +++ b/toolkit/components/payments/moz.build @@ -23,3 +23,5 @@ SPHINX_TREES['docs'] = 'docs' TESTING_JS_MODULES += [ 'test/PaymentTestUtils.jsm', ] + +XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini'] diff --git a/toolkit/components/payments/res/PaymentsStore.js b/toolkit/components/payments/res/PaymentsStore.js new file mode 100644 index 000000000000..a1b6518aa375 --- /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); + } +} diff --git a/toolkit/components/payments/test/unit/.eslintrc.js b/toolkit/components/payments/test/unit/.eslintrc.js new file mode 100644 index 000000000000..70fe35407782 --- /dev/null +++ b/toolkit/components/payments/test/unit/.eslintrc.js @@ -0,0 +1,7 @@ +"use strict"; + +module.exports = { + "extends": [ + "plugin:mozilla/xpcshell-test" + ] +}; diff --git a/toolkit/components/payments/test/unit/head.js b/toolkit/components/payments/test/unit/head.js new file mode 100644 index 000000000000..29f5aea01eee --- /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 */ +// ================================================ diff --git a/toolkit/components/payments/test/unit/test_PaymentsStore.js b/toolkit/components/payments/test/unit/test_PaymentsStore.js new file mode 100644 index 000000000000..686cfed32fa1 --- /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"); +}); diff --git a/toolkit/components/payments/test/unit/xpcshell.ini b/toolkit/components/payments/test/unit/xpcshell.ini new file mode 100644 index 000000000000..66707caf446b --- /dev/null +++ b/toolkit/components/payments/test/unit/xpcshell.ini @@ -0,0 +1,4 @@ +[DEFAULT] +head = head.js + +[test_PaymentsStore.js]