From add0aec088f13a96855f51c00e30c8a0742e94d1 Mon Sep 17 00:00:00 2001 From: kelvinp Date: Thu, 9 Apr 2015 16:08:00 -0700 Subject: [Webapp Refactor] Clean up public interface of SessionConnector. This CL cleans up the public interface of SessionConnector - Merges closeSession(), cancel() and resetConnection() into one single method closeSession(). - Moves the smart reconnector to Me2MeActivity. - Moves the Me2Me specific retry logic retryConnectMe2Me() into Me2MeActivity. Once this is done, getHostId() and updatePairingInfo() is no longer needed. - Moves ConnectXX2XX function into the appropriate activities. BUG=474840 TEST=All Remoting Webapp Unittests and all chromoting browser tests passed. https://chromium-swarm.appspot.com/user/task/26b40d0e44857d10 Review URL: https://codereview.chromium.org/1070903004 Cr-Commit-Position: refs/heads/master@{#324533} --- remoting/webapp/app_remoting/js/app_remoting.js | 6 +- remoting/webapp/base/js/modal_dialogs.js | 1 + remoting/webapp/crd/js/me2me_activity.js | 55 +++++--- remoting/webapp/crd/js/session_connector.js | 73 +--------- remoting/webapp/crd/js/session_connector_impl.js | 161 +++-------------------- remoting/webapp/crd/js/smart_reconnector.js | 9 +- 6 files changed, 69 insertions(+), 236 deletions(-) diff --git a/remoting/webapp/app_remoting/js/app_remoting.js b/remoting/webapp/app_remoting/js/app_remoting.js index c0630c7..541168c 100644 --- a/remoting/webapp/app_remoting/js/app_remoting.js +++ b/remoting/webapp/app_remoting/js/app_remoting.js @@ -157,7 +157,11 @@ remoting.AppRemoting.prototype.startApplication_ = function(token) { host['sharedSecret']); }; - that.sessionConnector_.connectMe2App(host, fetchThirdPartyToken); + that.sessionConnector_.connect( + remoting.Application.Mode.APP_REMOTING, host, + new remoting.CredentialsProvider( + {fetchThirdPartyToken: fetchThirdPartyToken})); + } else if (response && response.status == 'pending') { that.onError_(new remoting.Error( remoting.Error.Tag.SERVICE_UNAVAILABLE)); diff --git a/remoting/webapp/base/js/modal_dialogs.js b/remoting/webapp/base/js/modal_dialogs.js index 0aa3416..f8837e5 100644 --- a/remoting/webapp/base/js/modal_dialogs.js +++ b/remoting/webapp/base/js/modal_dialogs.js @@ -117,6 +117,7 @@ remoting.MessageDialog = function(mode, primaryButton, opt_secondaryButton) { * the button clicked. */ remoting.MessageDialog.prototype.show = function() { + base.debug.assert(this.eventHooks_ === null); this.eventHooks_ = new base.Disposables(new base.DomEventHook( this.primaryButton_, 'click', this.onClicked_.bind(this, remoting.MessageDialog.Result.PRIMARY), diff --git a/remoting/webapp/crd/js/me2me_activity.js b/remoting/webapp/crd/js/me2me_activity.js index 11f156e..6ebf4ba 100644 --- a/remoting/webapp/crd/js/me2me_activity.js +++ b/remoting/webapp/crd/js/me2me_activity.js @@ -30,6 +30,9 @@ remoting.Me2MeActivity = function(sessionConnector, host) { /** @private */ this.retryOnHostOffline_ = true; + + /** @private {remoting.SmartReconnector} */ + this.reconnector_ = null; }; remoting.Me2MeActivity.prototype.dispose = function() {}; @@ -41,7 +44,7 @@ remoting.Me2MeActivity.prototype.start = function() { this.hostUpdateDialog_.showIfNecessary(webappVersion).then(function() { return that.host_.options.load(); }).then(function() { - that.connect_(); + that.connect_(true); }).catch(function(/** remoting.Error */ error) { if (error.hasTag(remoting.Error.Tag.CANCELLED)) { remoting.setMode(remoting.AppMode.HOME); @@ -49,8 +52,11 @@ remoting.Me2MeActivity.prototype.start = function() { }); }; -/** @private */ -remoting.Me2MeActivity.prototype.connect_ = function() { +/** + * @param {boolean} suppressHostOfflineError + * @private + */ +remoting.Me2MeActivity.prototype.connect_ = function(suppressHostOfflineError) { remoting.setMode(remoting.AppMode.CLIENT_CONNECTING); var host = this.host_; @@ -83,28 +89,35 @@ remoting.Me2MeActivity.prototype.connect_ = function() { }); }; - var pairingInfo = host.options.pairingInfo; - this.connector_.connectMe2Me(host, requestPin, fetchThirdPartyToken, - pairingInfo.clientId, pairingInfo.sharedSecret); + var pairingInfo = /** @type{remoting.PairingInfo} */ ( + base.deepCopy(host.options.pairingInfo)); + var credentialsProvider = new remoting.CredentialsProvider({ + fetchPin: requestPin, + pairingInfo: pairingInfo, + fetchThirdPartyToken: fetchThirdPartyToken + }); + + this.connector_.connect( + remoting.Application.Mode.ME2ME, + host, credentialsProvider, suppressHostOfflineError); }; /** * @param {!remoting.Error} error */ remoting.Me2MeActivity.prototype.onConnectionFailed = function(error) { - var that = this; - var onHostListRefresh = function(/** boolean */ success) { - if (success) { - // Get the host from the hostList for the refreshed JID. - var host = remoting.hostList.getHostForId(that.host_.hostId); - that.connector_.retryConnectMe2Me(host); - return; - } - that.onError(error); - }; - if (error.hasTag(remoting.Error.Tag.HOST_IS_OFFLINE) && this.retryOnHostOffline_) { + var that = this; + var onHostListRefresh = function(/** boolean */ success) { + if (success) { + // Get the host from the hostList for the refreshed JID. + that.host_ = remoting.hostList.getHostForId(that.host_.hostId); + that.connect_(false); + return; + } + that.onError(error); + }; this.retryOnHostOffline_ = false; // The plugin will be re-created when the host finished refreshing @@ -128,6 +141,10 @@ remoting.Me2MeActivity.prototype.onConnected = function(connectionInfo) { plugin.extensions().register(new remoting.GnubbyAuthHandler()); this.pinDialog_.requestPairingIfNecessary(connectionInfo.plugin(), this.connector_); + + base.dispose(this.reconnector_); + this.reconnector_ = new remoting.SmartReconnector( + this.connect_.bind(this, false), connectionInfo.session()); }; remoting.Me2MeActivity.prototype.onDisconnected = function() { @@ -161,8 +178,7 @@ remoting.Me2MeActivity.prototype.showFinishDialog_ = function(mode) { if (result === Result.PRIMARY) { remoting.setMode(remoting.AppMode.HOME); } else { - that.connector_.reconnect(); - remoting.setMode(remoting.AppMode.CLIENT_CONNECTING); + that.connect_(true); } }); }; @@ -263,7 +279,6 @@ remoting.PinDialog.prototype.requestPairingIfNecessary = that.host_.options.pairingInfo.clientId = clientId; that.host_.options.pairingInfo.sharedSecret = sharedSecret; that.host_.options.save(); - connector.updatePairingInfo(clientId, sharedSecret); }; // Use the platform name as a proxy for the local computer name. diff --git a/remoting/webapp/crd/js/session_connector.js b/remoting/webapp/crd/js/session_connector.js index fab0b33..7094c43 100644 --- a/remoting/webapp/crd/js/session_connector.js +++ b/remoting/webapp/crd/js/session_connector.js @@ -18,84 +18,19 @@ var remoting = remoting || {}; remoting.SessionConnector = function() {}; /** - * Initiate a Me2Me connection. - * - * @param {remoting.Host} host The Me2Me host to which to connect. - * @param {function(boolean, function(string):void):void} fetchPin Function to - * interactively obtain the PIN from the user. - * @param {function(string, string, string, - * function(string, string): void): void} - * fetchThirdPartyToken Function to obtain a token from a third party - * authentication server. - * @param {string} clientPairingId The client id issued by the host when - * this device was paired, if it is already paired. - * @param {string} clientPairedSecret The shared secret issued by the host when - * this device was paired, if it is already paired. - * @return {void} Nothing. - */ -remoting.SessionConnector.prototype.connectMe2Me = - function(host, fetchPin, fetchThirdPartyToken, - clientPairingId, clientPairedSecret) {}; - -/** - * Retry connecting to a Me2Me host after a connection failure. - * - * @param {remoting.Host} host The Me2Me host to refresh. - * @return {void} Nothing. - */ -remoting.SessionConnector.prototype.retryConnectMe2Me = function(host) {}; - -/** - * Initiate a Me2App connection. - * - * @param {remoting.Host} host The Me2Me host to which to connect. - * @param {function(string, string, string, - * function(string, string): void): void} - * fetchThirdPartyToken Function to obtain a token from a third party - * authentication server. - * @return {void} Nothing. - */ -remoting.SessionConnector.prototype.connectMe2App = - function(host, fetchThirdPartyToken) {}; - -/** - * Update the pairing info so that the reconnect function will work correctly. - * - * @param {string} clientId The paired client id. - * @param {string} sharedSecret The shared secret. - */ -remoting.SessionConnector.prototype.updatePairingInfo = - function(clientId, sharedSecret) {}; - -/** * Initiates a remote connection. * * @param {remoting.Application.Mode} mode * @param {remoting.Host} host * @param {remoting.CredentialsProvider} credentialsProvider + * @param {boolean=} opt_suppressOfflineError Suppress the host offline error + * as we use stale JID's when initiating a Me2Me. This parameter will be + * removed when we get rid of sessionConnector altogether in a future CL. * @return {void} Nothing. */ remoting.SessionConnector.prototype.connect = - function(mode, host, credentialsProvider) {}; + function(mode, host, credentialsProvider, opt_suppressOfflineError) {}; -/** - * Reconnect a closed connection. - * - * @return {void} Nothing. - */ -remoting.SessionConnector.prototype.reconnect = function() {}; - -/** - * Cancel a connection-in-progress. - */ -remoting.SessionConnector.prototype.cancel = function() {}; - -/** - * Get host ID. - * - * @return {string} - */ -remoting.SessionConnector.prototype.getHostId = function() {}; /** * Closes the session and removes the plugin element. diff --git a/remoting/webapp/crd/js/session_connector_impl.js b/remoting/webapp/crd/js/session_connector_impl.js index d38c522..6bad8b3 100644 --- a/remoting/webapp/crd/js/session_connector_impl.js +++ b/remoting/webapp/crd/js/session_connector_impl.js @@ -17,7 +17,7 @@ var remoting = remoting || {}; * connector has invoked its onOk callback. * TODO(garykac): Have this owned by someone instead of being global. */ -remoting.clientSession = null; +remoting.clientSession; /** * @param {HTMLElement} clientContainer Container element for the client view. @@ -49,19 +49,13 @@ remoting.SessionConnectorImpl = function(clientContainer, onConnected, onError, /** @private {Array} */ this.requiredCapabilities_ = requiredCapabilities; - /** @private {remoting.SignalStrategy} */ - this.signalStrategy_ = null; - - /** @private {remoting.SmartReconnector} */ - this.reconnector_ = null; - /** @private */ this.bound_ = { onStateChange : this.onStateChange_.bind(this) }; // Initialize/declare per-connection state. - this.resetConnection_(); + this.closeSession(); }; /** @@ -69,9 +63,7 @@ remoting.SessionConnectorImpl = function(clientContainer, onConnected, onError, * second connection. Note the none of the shared WCS state is reset. * @private */ -remoting.SessionConnectorImpl.prototype.resetConnection_ = function() { - this.closeSession(); - +remoting.SessionConnectorImpl.prototype.closeSession = function() { // It's OK to initialize these member variables here because the // constructor calls this method. @@ -81,89 +73,21 @@ remoting.SessionConnectorImpl.prototype.resetConnection_ = function() { /** @private {boolean} */ this.logHostOfflineErrors_ = false; - /** @private {remoting.ClientPlugin} */ - this.plugin_ = null; - + base.dispose(this.clientSession_); /** @private {remoting.ClientSession} */ this.clientSession_ = null; + remoting.clientSession = null; + + base.dispose(this.plugin_); + /** @private {remoting.ClientPlugin} */ + this.plugin_ = null; /** @private {remoting.CredentialsProvider} */ this.credentialsProvider_ = null; -}; - -/** - * Initiate a Me2Me connection. - * - * This doesn't report host-offline errors because the connection will - * be retried and retryConnectMe2Me is responsible for reporting these errors. - * - * @param {remoting.Host} host The Me2Me host to which to connect. - * @param {function(boolean, function(string):void):void} fetchPin Function to - * interactively obtain the PIN from the user. - * @param {string} clientPairingId The client id issued by the host when - * this device was paired, if it is already paired. - * @param {string} clientPairedSecret The shared secret issued by the host when - * this device was paired, if it is already paired. - * @return {void} Nothing. - */ -remoting.SessionConnectorImpl.prototype.connectMe2Me = - function(host, fetchPin, fetchThirdPartyToken, - clientPairingId, clientPairedSecret) { - this.logHostOfflineErrors_ = false; - var credentialsProvider = new remoting.CredentialsProvider({ - fetchPin: fetchPin, - pairingInfo: {clientId: clientPairingId, sharedSecret: clientPairedSecret}, - fetchThirdPartyToken: fetchThirdPartyToken - }); - this.connect(remoting.Application.Mode.ME2ME, host, credentialsProvider); -}; - -/** - * Retry connecting to a Me2Me host after a connection failure. - * - * This is the same as connectMe2Me except that is will log errors if the - * host is offline. - * - * @param {remoting.Host} host The Me2Me host to refresh. - * @return {void} Nothing. - */ -remoting.SessionConnectorImpl.prototype.retryConnectMe2Me = function(host) { - this.logHostOfflineErrors_ = true; - this.connect(remoting.Application.Mode.ME2ME, host, - this.credentialsProvider_); -}; - -/** - * Initiate a Me2App connection. - * - * @param {remoting.Host} host The Me2Me host to which to connect. - * @param {function(string, string, string, - * function(string, string): void): void} - * fetchThirdPartyToken Function to obtain a token from a third party - * authenticaiton server. - * @return {void} Nothing. - */ -remoting.SessionConnectorImpl.prototype.connectMe2App = - function(host, fetchThirdPartyToken) { - this.logHostOfflineErrors_ = true; - var credentialsProvider = new remoting.CredentialsProvider({ - fetchThirdPartyToken : fetchThirdPartyToken - }); - this.connect(remoting.Application.Mode.APP_REMOTING, host, - credentialsProvider); -}; -/** - * Update the pairing info so that the reconnect function will work correctly. - * - * @param {string} clientId The paired client id. - * @param {string} sharedSecret The shared secret. - */ -remoting.SessionConnectorImpl.prototype.updatePairingInfo = - function(clientId, sharedSecret) { - var pairingInfo = this.credentialsProvider_.getPairingInfo(); - pairingInfo.clientId = clientId; - pairingInfo.sharedSecret = sharedSecret; + base.dispose(this.signalStrategy_); + /** @private {remoting.SignalStrategy} */ + this.signalStrategy_ = null; }; /** @@ -172,51 +96,25 @@ remoting.SessionConnectorImpl.prototype.updatePairingInfo = * @param {remoting.Application.Mode} mode * @param {remoting.Host} host the Host to connect to. * @param {remoting.CredentialsProvider} credentialsProvider + * @param {boolean=} opt_suppressOfflineError * @return {void} Nothing. * @private */ remoting.SessionConnectorImpl.prototype.connect = - function(mode, host, credentialsProvider) { - // Cancel any existing connect operation. - this.cancel(); + function(mode, host, credentialsProvider, opt_suppressOfflineError) { + // In some circumstances, the WCS