From d1a84c73057c2c5f5dfdb7503760f52a02a40cc4 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Wed, 11 Mar 2015 16:53:41 -0700 Subject: [PATCH] Backed out changeset 35827fc86c80 (bug 1140481) under suspicion of making wpt-reftests intermittently fail on a CLOSED TREE --- .../loop/content/js/conversation.js | 33 +- .../loop/content/js/conversation.jsx | 33 +- .../loop/content/js/conversationViews.js | 33 +- .../loop/content/js/conversationViews.jsx | 33 +- .../loop/content/shared/js/store.js | 2 +- .../desktop-local/conversationViews_test.js | 10 +- .../test/desktop-local/conversation_test.js | 7 +- .../components/loop/test/shared/store_test.js | 283 ++++++++---------- 8 files changed, 214 insertions(+), 220 deletions(-) diff --git a/browser/components/loop/content/js/conversation.js b/browser/components/loop/content/js/conversation.js index eddf678e890d..f39698b7eabd 100644 --- a/browser/components/loop/content/js/conversation.js +++ b/browser/components/loop/content/js/conversation.js @@ -27,11 +27,7 @@ loop.conversation = (function(mozL10n) { * in progress, and hence, which view to display. */ var AppControllerView = React.createClass({displayName: "AppControllerView", - mixins: [ - Backbone.Events, - loop.store.StoreMixin("conversationAppStore"), - sharedMixins.WindowCloseMixin - ], + mixins: [Backbone.Events, sharedMixins.WindowCloseMixin], propTypes: { // XXX Old types required for incoming call view. @@ -41,12 +37,26 @@ loop.conversation = (function(mozL10n) { sdk: React.PropTypes.object.isRequired, // XXX New types for flux style + conversationAppStore: React.PropTypes.instanceOf( + loop.store.ConversationAppStore).isRequired, + conversationStore: React.PropTypes.instanceOf(loop.store.ConversationStore) + .isRequired, dispatcher: React.PropTypes.instanceOf(loop.Dispatcher).isRequired, roomStore: React.PropTypes.instanceOf(loop.store.RoomStore) }, getInitialState: function() { - return this.getStoreState(); + return this.props.conversationAppStore.getStoreState(); + }, + + componentWillMount: function() { + this.listenTo(this.props.conversationAppStore, "change", function() { + this.setState(this.props.conversationAppStore.getStoreState()); + }, this); + }, + + componentWillUnmount: function() { + this.stopListening(this.props.conversationAppStore); }, render: function() { @@ -57,11 +67,12 @@ loop.conversation = (function(mozL10n) { conversation: this.props.conversation, sdk: this.props.sdk, isDesktop: true, - conversationAppStore: this.getStore()} + conversationAppStore: this.props.conversationAppStore} )); } case "outgoing": { return (React.createElement(OutgoingConversationView, { + store: this.props.conversationStore, dispatcher: this.props.dispatcher} )); } @@ -150,11 +161,7 @@ loop.conversation = (function(mozL10n) { feedbackClient: feedbackClient }); - loop.store.StoreMixin.register({ - conversationAppStore: conversationAppStore, - conversationStore: conversationStore, - feedbackStore: feedbackStore, - }); + loop.store.StoreMixin.register({feedbackStore: feedbackStore}); // XXX Old class creation for the incoming conversation view, whilst // we transition across (bug 1072323). @@ -184,7 +191,9 @@ loop.conversation = (function(mozL10n) { }); React.render(React.createElement(AppControllerView, { + conversationAppStore: conversationAppStore, roomStore: roomStore, + conversationStore: conversationStore, client: client, conversation: conversation, dispatcher: dispatcher, diff --git a/browser/components/loop/content/js/conversation.jsx b/browser/components/loop/content/js/conversation.jsx index 8936e708fb3a..d6cf8624588d 100644 --- a/browser/components/loop/content/js/conversation.jsx +++ b/browser/components/loop/content/js/conversation.jsx @@ -27,11 +27,7 @@ loop.conversation = (function(mozL10n) { * in progress, and hence, which view to display. */ var AppControllerView = React.createClass({ - mixins: [ - Backbone.Events, - loop.store.StoreMixin("conversationAppStore"), - sharedMixins.WindowCloseMixin - ], + mixins: [Backbone.Events, sharedMixins.WindowCloseMixin], propTypes: { // XXX Old types required for incoming call view. @@ -41,12 +37,26 @@ loop.conversation = (function(mozL10n) { sdk: React.PropTypes.object.isRequired, // XXX New types for flux style + conversationAppStore: React.PropTypes.instanceOf( + loop.store.ConversationAppStore).isRequired, + conversationStore: React.PropTypes.instanceOf(loop.store.ConversationStore) + .isRequired, dispatcher: React.PropTypes.instanceOf(loop.Dispatcher).isRequired, roomStore: React.PropTypes.instanceOf(loop.store.RoomStore) }, getInitialState: function() { - return this.getStoreState(); + return this.props.conversationAppStore.getStoreState(); + }, + + componentWillMount: function() { + this.listenTo(this.props.conversationAppStore, "change", function() { + this.setState(this.props.conversationAppStore.getStoreState()); + }, this); + }, + + componentWillUnmount: function() { + this.stopListening(this.props.conversationAppStore); }, render: function() { @@ -57,11 +67,12 @@ loop.conversation = (function(mozL10n) { conversation={this.props.conversation} sdk={this.props.sdk} isDesktop={true} - conversationAppStore={this.getStore()} + conversationAppStore={this.props.conversationAppStore} />); } case "outgoing": { return (); } @@ -150,11 +161,7 @@ loop.conversation = (function(mozL10n) { feedbackClient: feedbackClient }); - loop.store.StoreMixin.register({ - conversationAppStore: conversationAppStore, - conversationStore: conversationStore, - feedbackStore: feedbackStore, - }); + loop.store.StoreMixin.register({feedbackStore: feedbackStore}); // XXX Old class creation for the incoming conversation view, whilst // we transition across (bug 1072323). @@ -184,7 +191,9 @@ loop.conversation = (function(mozL10n) { }); React.render(); } diff --git a/browser/components/loop/content/shared/js/store.js b/browser/components/loop/content/shared/js/store.js index 165ffa4d84d1..4f2d8125e601 100644 --- a/browser/components/loop/content/shared/js/store.js +++ b/browser/components/loop/content/shared/js/store.js @@ -136,7 +136,7 @@ loop.store.StoreMixin = (function() { }, this); }, componentWillUnmount: function() { - this.getStore().off("change", null, this); + this.getStore().off("change"); } }; } diff --git a/browser/components/loop/test/desktop-local/conversationViews_test.js b/browser/components/loop/test/desktop-local/conversationViews_test.js index 7dc1144f3de8..36245f95a1cd 100644 --- a/browser/components/loop/test/desktop-local/conversationViews_test.js +++ b/browser/components/loop/test/desktop-local/conversationViews_test.js @@ -283,6 +283,7 @@ describe("loop.conversationViews", function () { return TestUtils.renderIntoDocument( React.createElement(loop.conversationViews.CallFailedView, { dispatcher: dispatcher, + store: store, contact: options.contact })); } @@ -293,10 +294,6 @@ describe("loop.conversationViews", function () { mozLoop: navigator.mozLoop, sdkDriver: {} }); - loop.store.StoreMixin.register({ - conversationStore: store - }); - fakeAudio = { play: sinon.spy(), pause: sinon.spy(), @@ -585,6 +582,7 @@ describe("loop.conversationViews", function () { return TestUtils.renderIntoDocument( React.createElement(loop.conversationViews.OutgoingConversationView, { dispatcher: dispatcher, + store: store })); } @@ -594,10 +592,6 @@ describe("loop.conversationViews", function () { mozLoop: fakeMozLoop, sdkDriver: {} }); - loop.store.StoreMixin.register({ - conversationStore: store - }); - feedbackStore = new loop.store.FeedbackStore(dispatcher, { feedbackClient: {} }); diff --git a/browser/components/loop/test/desktop-local/conversation_test.js b/browser/components/loop/test/desktop-local/conversation_test.js index 28e808b341c7..5a5780ec71b5 100644 --- a/browser/components/loop/test/desktop-local/conversation_test.js +++ b/browser/components/loop/test/desktop-local/conversation_test.js @@ -139,6 +139,8 @@ describe("loop.conversation", function() { conversation: conversation, roomStore: roomStore, sdk: {}, + conversationStore: conversationStore, + conversationAppStore: conversationAppStore, dispatcher: dispatcher, mozLoop: navigator.mozLoop })); @@ -174,11 +176,6 @@ describe("loop.conversation", function() { dispatcher: dispatcher, mozLoop: navigator.mozLoop }); - - loop.store.StoreMixin.register({ - conversationAppStore: conversationAppStore, - conversationStore: conversationStore - }); }); afterEach(function() { diff --git a/browser/components/loop/test/shared/store_test.js b/browser/components/loop/test/shared/store_test.js index 14f6112ffd88..91726c4468b2 100644 --- a/browser/components/loop/test/shared/store_test.js +++ b/browser/components/loop/test/shared/store_test.js @@ -4,202 +4,157 @@ var expect = chai.expect; -describe("loop.store", function () { +describe("loop.store.createStore", function () { "use strict"; - var dispatcher; var sandbox; var sharedActions = loop.shared.actions; beforeEach(function() { sandbox = sinon.sandbox.create(); - dispatcher = new loop.Dispatcher(); }); afterEach(function() { sandbox.restore(); }); - describe("loop.store.createStore", function() { - it("should create a store constructor", function() { - expect(loop.store.createStore({})).to.be.a("function"); - }); - - it("should implement Backbone.Events", function() { - expect(loop.store.createStore({}).prototype).to.include.keys(["on", "off"]); - }); - - describe("Store API", function() { - describe("#constructor", function() { - it("should require a dispatcher", function() { - var TestStore = loop.store.createStore({}); - expect(function() { - new TestStore(); - }).to.Throw(/required dispatcher/); - }); - - it("should call initialize() when constructed, if defined", function() { - var initialize = sandbox.spy(); - var TestStore = loop.store.createStore({initialize: initialize}); - var options = {fake: true}; - - new TestStore(dispatcher, options); - - sinon.assert.calledOnce(initialize); - sinon.assert.calledWithExactly(initialize, options); - }); - - it("should register actions", function() { - sandbox.stub(dispatcher, "register"); - var TestStore = loop.store.createStore({ - actions: ["a", "b"], - a: function() {}, - b: function() {} - }); - - var store = new TestStore(dispatcher); - - sinon.assert.calledOnce(dispatcher.register); - sinon.assert.calledWithExactly(dispatcher.register, store, ["a", "b"]); - }); - - it("should throw if a registered action isn't implemented", function() { - var TestStore = loop.store.createStore({ - actions: ["a", "b"], - a: function() {} // missing b - }); - - expect(function() { - new TestStore(dispatcher); - }).to.Throw(/should implement an action handler for b/); - }); - }); - - describe("#getInitialStoreState", function() { - it("should set initial store state if provided", function() { - var TestStore = loop.store.createStore({ - getInitialStoreState: function() { - return {foo: "bar"}; - } - }); - - var store = new TestStore(dispatcher); - - expect(store.getStoreState()).eql({foo: "bar"}); - }); - }); - - describe("#dispatchAction", function() { - it("should dispatch an action", function() { - sandbox.stub(dispatcher, "dispatch"); - var TestStore = loop.store.createStore({}); - var TestAction = sharedActions.Action.define("TestAction", {}); - var action = new TestAction({}); - var store = new TestStore(dispatcher); - - store.dispatchAction(action); - - sinon.assert.calledOnce(dispatcher.dispatch); - sinon.assert.calledWithExactly(dispatcher.dispatch, action); - }); - }); - - describe("#getStoreState", function() { - var TestStore = loop.store.createStore({}); - var store; - - beforeEach(function() { - store = new TestStore(dispatcher); - store.setStoreState({foo: "bar", bar: "baz"}); - }); - - it("should retrieve the whole state by default", function() { - expect(store.getStoreState()).eql({foo: "bar", bar: "baz"}); - }); - - it("should retrieve a given property state", function() { - expect(store.getStoreState("bar")).eql("baz"); - }); - }); - - describe("#setStoreState", function() { - var TestStore = loop.store.createStore({}); - var store; - - beforeEach(function() { - store = new TestStore(dispatcher); - store.setStoreState({foo: "bar"}); - }); - - it("should update store state data", function() { - store.setStoreState({foo: "baz"}); - - expect(store.getStoreState("foo")).eql("baz"); - }); - - it("should trigger a `change` event", function(done) { - store.once("change", function() { - done(); - }); - - store.setStoreState({foo: "baz"}); - }); - - it("should trigger a `change:` event", function(done) { - store.once("change:foo", function() { - done(); - }); - - store.setStoreState({foo: "baz"}); - }); - }); - }); + it("should create a store constructor", function() { + expect(loop.store.createStore({})).to.be.a("function"); }); - describe("loop.store.StoreMixin", function() { - var view1, view2, store, storeClass, testComp; + it("should implement Backbone.Events", function() { + expect(loop.store.createStore({}).prototype).to.include.keys(["on", "off"]) + }); + + describe("Store API", function() { + var dispatcher; beforeEach(function() { - storeClass = loop.store.createStore({}); + dispatcher = new loop.Dispatcher(); + }); - store = new storeClass(dispatcher); - - loop.store.StoreMixin.register({store: store}); - - testComp = React.createClass({ - mixins: [loop.store.StoreMixin("store")], - render: function() { - return React.DOM.div(); - } + describe("#constructor", function() { + it("should require a dispatcher", function() { + var TestStore = loop.store.createStore({}); + expect(function() { + new TestStore(); + }).to.Throw(/required dispatcher/); }); - view1 = TestUtils.renderIntoDocument(React.createElement(testComp)); + it("should call initialize() when constructed, if defined", function() { + var initialize = sandbox.spy(); + var TestStore = loop.store.createStore({initialize: initialize}); + var options = {fake: true}; + + new TestStore(dispatcher, options); + + sinon.assert.calledOnce(initialize); + sinon.assert.calledWithExactly(initialize, options); + }); + + it("should register actions", function() { + sandbox.stub(dispatcher, "register"); + var TestStore = loop.store.createStore({ + actions: ["a", "b"], + a: function() {}, + b: function() {} + }); + + var store = new TestStore(dispatcher); + + sinon.assert.calledOnce(dispatcher.register); + sinon.assert.calledWithExactly(dispatcher.register, store, ["a", "b"]); + }); + + it("should throw if a registered action isn't implemented", function() { + var TestStore = loop.store.createStore({ + actions: ["a", "b"], + a: function() {} // missing b + }); + + expect(function() { + new TestStore(dispatcher); + }).to.Throw(/should implement an action handler for b/); + }); }); - it("should update the state when the store changes", function() { - store.setStoreState({test: true}); + describe("#getInitialStoreState", function() { + it("should set initial store state if provided", function() { + var TestStore = loop.store.createStore({ + getInitialStoreState: function() { + return {foo: "bar"}; + } + }); - expect(view1.state).eql({test: true}); + var store = new TestStore(dispatcher); + + expect(store.getStoreState()).eql({foo: "bar"}); + }); }); - it("should stop listening to state changes", function() { - // There's no easy way in TestUtils to unmount, so simulate it. - view1.componentWillUnmount(); + describe("#dispatchAction", function() { + it("should dispatch an action", function() { + sandbox.stub(dispatcher, "dispatch"); + var TestStore = loop.store.createStore({}); + var TestAction = sharedActions.Action.define("TestAction", {}); + var action = new TestAction({}); + var store = new TestStore(dispatcher); - store.setStoreState({test2: true}); + store.dispatchAction(action); - expect(view1.state).eql(null); + sinon.assert.calledOnce(dispatcher.dispatch); + sinon.assert.calledWithExactly(dispatcher.dispatch, action); + }); }); - it("should not stop listening to state changes on other components", function() { - view2 = TestUtils.renderIntoDocument(React.createElement(testComp)); + describe("#getStoreState", function() { + var TestStore = loop.store.createStore({}); + var store; - // There's no easy way in TestUtils to unmount, so simulate it. - view1.componentWillUnmount(); + beforeEach(function() { + store = new TestStore(dispatcher); + store.setStoreState({foo: "bar", bar: "baz"}); + }); - store.setStoreState({test3: true}); + it("should retrieve the whole state by default", function() { + expect(store.getStoreState()).eql({foo: "bar", bar: "baz"}); + }); - expect(view2.state).eql({test3: true}); + it("should retrieve a given property state", function() { + expect(store.getStoreState("bar")).eql("baz"); + }); + }); + + describe("#setStoreState", function() { + var TestStore = loop.store.createStore({}); + var store; + + beforeEach(function() { + store = new TestStore(dispatcher); + store.setStoreState({foo: "bar"}); + }); + + it("should update store state data", function() { + store.setStoreState({foo: "baz"}); + + expect(store.getStoreState("foo")).eql("baz"); + }); + + it("should trigger a `change` event", function(done) { + store.once("change", function() { + done(); + }); + + store.setStoreState({foo: "baz"}); + }); + + it("should trigger a `change:` event", function(done) { + store.once("change:foo", function() { + done(); + }); + + store.setStoreState({foo: "baz"}); + }); }); }); });