diff --git a/browser/base/content/browser-loop.js b/browser/base/content/browser-loop.js index ac6b2ed80627..56d349ef61a1 100644 --- a/browser/base/content/browser-loop.js +++ b/browser/base/content/browser-loop.js @@ -343,5 +343,63 @@ XPCOMUtils.defineLazyModuleGetter(this, "PanelFrame", "resource:///modules/Panel this.activeSound.addEventListener("ended", () => this.activeSound = undefined, false); }, + + /** + * Adds a listener for browser sharing. It will inform the listener straight + * away for the current windowId, and then on every tab change. + * + * Listener parameters: + * - {Object} err If there is a error this will be defined, null otherwise. + * - {Integer} windowId The new windowId for the browser. + * + * @param {Function} listener The listener to receive information on when the + * windowId changes. + */ + addBrowserSharingListener: function(listener) { + if (!this._tabChangeListeners) { + this._tabChangeListeners = new Set(); + gBrowser.addEventListener("select", this); + } + + this._tabChangeListeners.add(listener); + + // Get the first window Id for the listener. + listener(null, gBrowser.selectedTab.linkedBrowser.outerWindowID); + }, + + /** + * Removes a listener from browser sharing. + * + * @param {Function} listener The listener to remove from the list. + */ + removeBrowserSharingListener: function(listener) { + if (!this._tabChangeListeners) { + return; + } + + if (this._tabChangeListeners.has(listener)) { + this._tabChangeListeners.delete(listener); + } + + if (!this._tabChangeListeners.size) { + gBrowser.removeEventListener("select", this); + delete this._tabChangeListeners; + } + }, + + /** + * Handles events from gBrowser. + */ + handleEvent: function(event) { + // We only should get "select" events. + if (event.type != "select") { + return; + } + + // We've changed the tab, so get the new window id. + for (let listener of this._tabChangeListeners) { + listener(null, gBrowser.selectedTab.linkedBrowser.outerWindowID); + }; + }, }; })(); diff --git a/browser/components/loop/MozLoopAPI.jsm b/browser/components/loop/MozLoopAPI.jsm index b7576638984a..4320a3f92d7c 100644 --- a/browser/components/loop/MozLoopAPI.jsm +++ b/browser/components/loop/MozLoopAPI.jsm @@ -196,6 +196,7 @@ function injectLoopAPI(targetWindow) { let contactsAPI; let roomsAPI; let callsAPI; + let savedWindowListeners = new Map(); let api = { /** @@ -266,10 +267,21 @@ function injectLoopAPI(targetWindow) { } }, - getActiveTabWindowId: { + /** + * Adds a listener to the most recent window for browser/tab sharing. The + * listener will be notified straight away of the current tab id, then every + * time there is a change of tab. + * + * Listener parameters: + * - {Object} err If there is a error this will be defined, null otherwise. + * - {Number} windowId The new windowId after a change of tab. + * + * @param {Function} listener The listener to handle the windowId changes. + */ + addBrowserSharingListener: { enumerable: true, writable: true, - value: function(callback) { + value: function(listener) { let win = Services.wm.getMostRecentWindow("navigator:browser"); let browser = win && win.gBrowser.selectedTab.linkedBrowser; if (!win || !browser) { @@ -277,16 +289,39 @@ function injectLoopAPI(targetWindow) { // window left. let err = new Error("No tabs available to share."); MozLoopService.log.error(err); - callback(cloneValueInto(err, targetWindow)); + listener(cloneValueInto(err, targetWindow)); + return; + } + win.LoopUI.addBrowserSharingListener(listener); + + savedWindowListeners.set(listener, Cu.getWeakReference(win)); + } + }, + + /** + * Removes a listener that was previously added. + * + * @param {Function} listener The listener to handle the windowId changes. + */ + removeBrowserSharingListener: { + enumerable: true, + writable: true, + value: function(listener) { + if (!savedWindowListeners.has(listener)) { return; } - let mm = browser.messageManager; - mm.addMessageListener("webrtc:response:StartBrowserSharing", function listener(message) { - mm.removeMessageListener("webrtc:response:StartBrowserSharing", listener); - callback(null, message.data.windowID); - }); - mm.sendAsyncMessage("webrtc:StartBrowserSharing"); + let win = savedWindowListeners.get(listener).get(); + + // Remove the element, regardless of if the window exists or not so + // that we clean the map. + savedWindowListeners.delete(listener); + + if (!win) { + return; + } + + win.LoopUI.removeBrowserSharingListener(listener); } }, diff --git a/browser/components/loop/content/shared/js/activeRoomStore.js b/browser/components/loop/content/shared/js/activeRoomStore.js index 32caa936ec61..f529a04eef7a 100644 --- a/browser/components/loop/content/shared/js/activeRoomStore.js +++ b/browser/components/loop/content/shared/js/activeRoomStore.js @@ -395,6 +395,38 @@ loop.store.ActiveRoomStore = (function() { this.setStoreState({receivingScreenShare: actionData.receiving}); }, + /** + * Handles switching browser (aka tab) sharing to a new window. Should + * only be used for browser sharing. + * + * @param {Number} windowId The new windowId to start sharing. + */ + _handleSwitchBrowserShare: function(err, windowId) { + if (err) { + console.error("Error getting the windowId: " + err); + return; + } + + var screenSharingState = this.getStoreState().screenSharingState; + + if (screenSharingState === SCREEN_SHARE_STATES.INACTIVE) { + // Screen sharing is still pending, so assume that we need to kick it off. + var options = { + videoSource: "browser", + constraints: { + browserWindow: windowId, + scrollWithPage: true + }, + }; + this._sdkDriver.startScreenShare(options); + } else if (screenSharingState === SCREEN_SHARE_STATES.ACTIVE) { + // Just update the current share. + this._sdkDriver.switchAcquiredWindow(windowId); + } else { + console.error("Unexpectedly received windowId for browser sharing when pending"); + } + }, + /** * Initiates a screen sharing publisher. * @@ -409,19 +441,12 @@ loop.store.ActiveRoomStore = (function() { videoSource: actionData.type }; if (options.videoSource === "browser") { - this._mozLoop.getActiveTabWindowId(function(err, windowId) { - if (err || !windowId) { - this.dispatchAction(new sharedActions.ScreenSharingState({ - state: SCREEN_SHARE_STATES.INACTIVE - })); - return; - } - options.constraints = { - browserWindow: windowId, - scrollWithPage: true - }; - this._sdkDriver.startScreenShare(options); - }.bind(this)); + this._browserSharingListener = this._handleSwitchBrowserShare.bind(this); + + // Set up a listener for watching screen shares. This will get notified + // with the first windowId when it is added, so we start off the sharing + // from within the listener. + this._mozLoop.addBrowserSharingListener(this._browserSharingListener); } else { this._sdkDriver.startScreenShare(options); } @@ -431,6 +456,12 @@ loop.store.ActiveRoomStore = (function() { * Ends an active screenshare session. */ endScreenShare: function() { + if (this._browserSharingListener) { + // Remove the browser sharing listener as we don't need it now. + this._mozLoop.removeBrowserSharingListener(this._browserSharingListener); + this._browserSharingListener = null; + } + if (this._sdkDriver.endScreenShare()) { this.dispatchAction(new sharedActions.ScreenSharingState({ state: SCREEN_SHARE_STATES.INACTIVE diff --git a/browser/components/loop/content/shared/js/otSdkDriver.js b/browser/components/loop/content/shared/js/otSdkDriver.js index e181f6b05d48..e3328b710234 100644 --- a/browser/components/loop/content/shared/js/otSdkDriver.js +++ b/browser/components/loop/content/shared/js/otSdkDriver.js @@ -100,6 +100,12 @@ loop.OTSdkDriver = (function() { * @param {Object} options Hash containing options for the SDK */ startScreenShare: function(options) { + // For browser sharing, we store the window Id so that we can avoid unnecessary + // re-triggers. + if (options.videoSource === "browser") { + this._windowId = options.constraints.browserWindow; + } + var config = _.extend(this._getCopyPublisherConfig(), options); this.screenshare = this.sdk.initPublisher(this.getScreenShareElementFunc(), @@ -108,6 +114,20 @@ loop.OTSdkDriver = (function() { this.screenshare.on("accessDenied", this._onScreenShareDenied.bind(this)); }, + /** + * Initiates switching the browser window that is being shared. + * + * @param {Integer} windowId The windowId of the browser. + */ + switchAcquiredWindow: function(windowId) { + if (windowId === this._windowId) { + return; + } + + this._windowId = windowId; + this.screenshare._.switchAcquiredWindow(windowId); + }, + /** * Ends an active screenshare session. Return `true` when an active screen- * sharing session was ended or `false` when no session is active. @@ -123,6 +143,7 @@ loop.OTSdkDriver = (function() { this.screenshare.off("accessAllowed accessDenied"); this.screenshare.destroy(); delete this.screenshare; + delete this._windowId; return true; }, diff --git a/browser/components/loop/test/mochitest/browser.ini b/browser/components/loop/test/mochitest/browser.ini index 20391398ed66..c90d00b4a992 100644 --- a/browser/components/loop/test/mochitest/browser.ini +++ b/browser/components/loop/test/mochitest/browser.ini @@ -19,8 +19,8 @@ skip-if = e10s [browser_mozLoop_prefs.js] [browser_mozLoop_doNotDisturb.js] skip-if = buildapp == 'mulet' -[browser_toolbarbutton.js] [browser_mozLoop_pluralStrings.js] -[browser_mozLoop_tabSharing.js] +[browser_mozLoop_sharingListeners.js] [browser_mozLoop_telemetry.js] skip-if = e10s +[browser_toolbarbutton.js] diff --git a/browser/components/loop/test/mochitest/browser_mozLoop_sharingListeners.js b/browser/components/loop/test/mochitest/browser_mozLoop_sharingListeners.js new file mode 100644 index 000000000000..5f9fb177c042 --- /dev/null +++ b/browser/components/loop/test/mochitest/browser_mozLoop_sharingListeners.js @@ -0,0 +1,125 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* + * This file contains tests for the window.LoopUI active tab trackers. + */ +"use strict"; + +const {injectLoopAPI} = Cu.import("resource:///modules/loop/MozLoopAPI.jsm"); +gMozLoopAPI = injectLoopAPI({}); + +let handlers = [ + { + resolve: null, + windowId: null, + listener: function(err, windowId) { + handlers[0].windowId = windowId; + handlers[0].resolve(); + } + }, + { + resolve: null, + windowId: null, + listener: function(err, windowId) { + handlers[1].windowId = windowId; + handlers[1].resolve(); + } + } +]; + +function promiseWindowIdReceivedOnAdd(handler) { + return new Promise(resolve => { + handler.resolve = resolve; + gMozLoopAPI.addBrowserSharingListener(handler.listener); + }); +}; + +let createdTabs = []; + +function promiseWindowIdReceivedNewTab(handlers) { + let promiseHandlers = []; + + handlers.forEach(handler => { + promiseHandlers.push(new Promise(resolve => { + handler.resolve = resolve; + })); + }); + + let createdTab = gBrowser.selectedTab = gBrowser.addTab(); + createdTabs.push(createdTab); + + promiseHandlers.push(promiseTabLoadEvent(createdTab, "about:mozilla")); + + return Promise.all(promiseHandlers); +}; + +function removeTabs() { + for (let createdTab of createdTabs) { + gBrowser.removeTab(createdTab); + } + + createdTabs = []; +} + +add_task(function* test_singleListener() { + yield promiseWindowIdReceivedOnAdd(handlers[0]); + + let initialWindowId = handlers[0].windowId; + + Assert.notEqual(initialWindowId, null, "window id should be valid"); + + // Check that a new tab updates the window id. + yield promiseWindowIdReceivedNewTab([handlers[0]]); + + let newWindowId = handlers[0].windowId; + + Assert.notEqual(initialWindowId, newWindowId, "Tab contentWindow IDs shouldn't be the same"); + + // Now remove the listener. + gMozLoopAPI.removeBrowserSharingListener(handlers[0].listener); + + removeTabs(); +}); + +add_task(function* test_multipleListener() { + yield promiseWindowIdReceivedOnAdd(handlers[0]); + + let initialWindowId0 = handlers[0].windowId; + + Assert.notEqual(initialWindowId0, null, "window id should be valid"); + + yield promiseWindowIdReceivedOnAdd(handlers[1]); + + let initialWindowId1 = handlers[1].windowId; + + Assert.notEqual(initialWindowId1, null, "window id should be valid"); + Assert.equal(initialWindowId0, initialWindowId1, "window ids should be the same"); + + // Check that a new tab updates the window id. + yield promiseWindowIdReceivedNewTab(handlers); + + let newWindowId0 = handlers[0].windowId; + let newWindowId1 = handlers[1].windowId; + + Assert.equal(newWindowId0, newWindowId1, "Listeners should have the same windowId"); + Assert.notEqual(initialWindowId0, newWindowId0, "Tab contentWindow IDs shouldn't be the same"); + + // Now remove the first listener. + gMozLoopAPI.removeBrowserSharingListener(handlers[0].listener); + + // Check that a new tab updates the window id. + yield promiseWindowIdReceivedNewTab([handlers[1]]); + + let nextWindowId0 = handlers[0].windowId; + let nextWindowId1 = handlers[1].windowId; + + Assert.equal(newWindowId0, nextWindowId0, "First listener shouldn't have updated"); + Assert.notEqual(newWindowId1, nextWindowId1, "Second listener should have updated"); + + // Cleanup. + gMozLoopAPI.removeBrowserSharingListener(handlers[1].listener); + + removeTabs(); +}); + diff --git a/browser/components/loop/test/mochitest/browser_mozLoop_tabSharing.js b/browser/components/loop/test/mochitest/browser_mozLoop_tabSharing.js deleted file mode 100644 index 928e214aa7fd..000000000000 --- a/browser/components/loop/test/mochitest/browser_mozLoop_tabSharing.js +++ /dev/null @@ -1,41 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -/** - * This is an integration test to make sure that passing window IDs is working as - * expected, with or without e10s enabled - rather than just testing MozLoopAPI - * alone. - */ - -const {injectLoopAPI} = Cu.import("resource:///modules/loop/MozLoopAPI.jsm"); -gMozLoopAPI = injectLoopAPI({}); - -let promiseTabWindowId = function() { - return new Promise(resolve => { - gMozLoopAPI.getActiveTabWindowId((err, windowId) => { - Assert.equal(null, err, "No error should've occurred."); - Assert.equal(typeof windowId, "number", "We should have a window ID"); - resolve(windowId); - }); - }); -}; - -add_task(function* test_windowIdFetch_simple() { - Assert.ok(gMozLoopAPI, "mozLoop should exist"); - - yield promiseTabWindowId(); -}); - -add_task(function* test_windowIdFetch_multipleTabs() { - let previousTab = gBrowser.selectedTab; - let previousTabId = yield promiseTabWindowId(); - - let tab = gBrowser.selectedTab = gBrowser.addTab(); - yield promiseTabLoadEvent(tab, "about:mozilla"); - let tabId = yield promiseTabWindowId(); - Assert.ok(tabId !== previousTabId, "Tab contentWindow IDs shouldn't be the same"); - gBrowser.removeTab(tab); - - tabId = yield promiseTabWindowId(); - Assert.equal(previousTabId, tabId, "Window IDs should be back to what they were"); -}); diff --git a/browser/components/loop/test/shared/activeRoomStore_test.js b/browser/components/loop/test/shared/activeRoomStore_test.js index f046f9570585..df5395aeb4d3 100644 --- a/browser/components/loop/test/shared/activeRoomStore_test.js +++ b/browser/components/loop/test/shared/activeRoomStore_test.js @@ -21,8 +21,10 @@ describe("loop.store.ActiveRoomStore", function () { sandbox.stub(dispatcher, "dispatch"); fakeMozLoop = { - setLoopPref: sandbox.stub(), - addConversationContext: sandbox.stub(), + setLoopPref: sinon.stub(), + addConversationContext: sinon.stub(), + addBrowserSharingListener: sinon.stub(), + removeBrowserSharingListener: sinon.stub(), rooms: { get: sinon.stub(), join: sinon.stub(), @@ -36,11 +38,12 @@ describe("loop.store.ActiveRoomStore", function () { }; fakeSdkDriver = { - connectSession: sandbox.stub(), - disconnectSession: sandbox.stub(), - forceDisconnectAll: sandbox.stub().callsArg(0), - startScreenShare: sandbox.stub(), - endScreenShare: sandbox.stub().returns(true) + connectSession: sinon.stub(), + disconnectSession: sinon.stub(), + forceDisconnectAll: sinon.stub().callsArg(0), + startScreenShare: sinon.stub(), + switchAcquiredWindow: sinon.stub(), + endScreenShare: sinon.stub().returns(true) }; fakeMultiplexGum = { @@ -717,12 +720,20 @@ describe("loop.store.ActiveRoomStore", function () { }); }); - it("should invoke the SDK driver with the correct options for tab sharing", function() { + it("should add a browser sharing listener for tab sharing", function() { store.startScreenShare(new sharedActions.StartScreenShare({ type: "browser" })); - sinon.assert.calledOnce(fakeMozLoop.getActiveTabWindowId); + sinon.assert.calledOnce(fakeMozLoop.addBrowserSharingListener); + }); + + it("should invoke the SDK driver with the correct options for tab sharing", function() { + fakeMozLoop.addBrowserSharingListener.callsArgWith(0, null, 42); + + store.startScreenShare(new sharedActions.StartScreenShare({ + type: "browser" + })); sinon.assert.calledOnce(fakeSdkDriver.startScreenShare); sinon.assert.calledWith(fakeSdkDriver.startScreenShare, { @@ -732,7 +743,31 @@ describe("loop.store.ActiveRoomStore", function () { scrollWithPage: true } }); - }) + }); + }); + + describe("Screen share Events", function() { + var listener; + + beforeEach(function() { + store.startScreenShare(new sharedActions.StartScreenShare({ + type: "browser" + })); + + // Listener is the first argument of the first call. + listener = fakeMozLoop.addBrowserSharingListener.args[0][0]; + + store.setStoreState({ + screenSharingState: SCREEN_SHARE_STATES.ACTIVE + }); + }); + + it("should update the SDK driver when a new window id is received", function() { + listener(null, 72); + + sinon.assert.calledOnce(fakeSdkDriver.switchAcquiredWindow); + sinon.assert.calledWithExactly(fakeSdkDriver.switchAcquiredWindow, 72); + }); }); describe("#endScreenShare", function() { @@ -745,6 +780,18 @@ describe("loop.store.ActiveRoomStore", function () { state: SCREEN_SHARE_STATES.INACTIVE })); }); + + it("should remove the sharing listener", function() { + // Setup the listener. + store.startScreenShare(new sharedActions.StartScreenShare({ + type: "browser" + })); + + // Now stop the screen share. + store.endScreenShare(); + + sinon.assert.calledOnce(fakeMozLoop.removeBrowserSharingListener); + }); }); describe("#remotePeerConnected", function() { diff --git a/browser/components/loop/test/shared/otSdkDriver_test.js b/browser/components/loop/test/shared/otSdkDriver_test.js index 3ad0b412da98..7c8e48f6e846 100644 --- a/browser/components/loop/test/shared/otSdkDriver_test.js +++ b/browser/components/loop/test/shared/otSdkDriver_test.js @@ -46,7 +46,10 @@ describe("loop.OTSdkDriver", function () { publisher = _.extend({ destroy: sinon.stub(), publishAudio: sinon.stub(), - publishVideo: sinon.stub() + publishVideo: sinon.stub(), + _: { + switchAcquiredWindow: sinon.stub() + } }, Backbone.Events); sdk = { @@ -143,8 +146,10 @@ describe("loop.OTSdkDriver", function () { // has multiple options. var options = { videoSource: "browser", - browserWindow: 42, - scrollWithPage: true + constraints: { + browserWindow: 42, + scrollWithPage: true + } }; driver.startScreenShare(options); @@ -153,6 +158,37 @@ describe("loop.OTSdkDriver", function () { }); }); + describe("#switchAcquiredWindow", function() { + beforeEach(function() { + var options = { + videoSource: "browser", + constraints: { + browserWindow: 42, + scrollWithPage: true + } + }; + driver.getScreenShareElementFunc = function() { + return fakeScreenElement; + }; + sandbox.stub(dispatcher, "dispatch"); + + driver.startScreenShare(options); + }); + + it("should switch to the acquired window", function() { + driver.switchAcquiredWindow(72); + + sinon.assert.calledOnce(publisher._.switchAcquiredWindow); + sinon.assert.calledWithExactly(publisher._.switchAcquiredWindow, 72); + }); + + it("should not switch if the window is the same as the currently selected one", function() { + driver.switchAcquiredWindow(42); + + sinon.assert.notCalled(publisher._.switchAcquiredWindow); + }); + }); + describe("#endScreenShare", function() { beforeEach(function() { driver.getScreenShareElementFunc = function() {};