Fix It2Me Host doesn't kill the native process after canceling a share

remoting/webapp/crd/js/host_screen.js

Index: remoting/webapp/crd/js/host_screen.js
diff --git a/remoting/webapp/crd/js/host_screen.js b/remoting/webapp/crd/js/host_screen.js
index 10304d06723c7ab128ccf18202c7ee25823f779e..e1ddda47abf6b858c55c2d7495f74e089b4a8fbf 100644
--- a/remoting/webapp/crd/js/host_screen.js
+++ b/remoting/webapp/crd/js/host_screen.js
@@ -7,18 +7,19 @@
  * Functions related to the 'host screen' for Chromoting.
  */
 
-'use strict';
-
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
+(function(){
+
+'use strict';
+
 /** @type {remoting.HostSession} */
-remoting.hostSession = null;
+var hostSession_ = null;

[Sergey Ulanov, 2015/02/04 19:52:46]

I think this can be removed completely. tryShareWithToken_() just need to pass
it to all callbacks.

[kelvinp, 2015/02/04 21:36:15]

I think ultimately, we would want to encapsulate the host sharing flow in an
object.  For this CL, I just want to get rid of the global to prevent further
dependency on this object.  I would probably clean up this file in a separate
CL.

 
 /**
  * @type {boolean} Whether or not the last share was cancelled by the user.
  *     This controls what screen is shown when the host signals completion.
- * @private
  */
 var lastShareWasCancelled_ = false;
 
@@ -37,7 +38,7 @@ remoting.tryShare = function() {
 
   var tryInitializeFacade = function() {
     hostFacade.initialize(onFacadeInitialized, onFacadeInitializationFailed);
-  }
+  };
 
   var onFacadeInitialized = function () {
     // Host already installed.
@@ -47,7 +48,7 @@ remoting.tryShare = function() {
   var onFacadeInitializationFailed = function() {
     // If we failed to initialize the dispatcher then prompt the user to install
     // the host manually.
-    var hasHostDialog = (hostInstallDialog != null);  /** jscompile hack */
+    var hasHostDialog = (hostInstallDialog !== null);  /** jscompile hack */
     if (!hasHostDialog) {
       hostInstallDialog = new remoting.HostInstallDialog();
       hostInstallDialog.show(tryInitializeFacade, onInstallError);
@@ -63,7 +64,7 @@ remoting.tryShare = function() {
     } else {
       showShareError_(error);
     }
-  }
+  };
 
   tryInitializeFacade();
 };
@@ -91,9 +92,10 @@ remoting.tryShareWithToken_ = function(hostFacade, token) {
   document.getElementById('cancel-share-button').disabled = false;
   disableTimeoutCountdown_();
 
-  remoting.hostSession = new remoting.HostSession();
+  base.debug.assert(hostSession_ === null);
+  hostSession_ = new remoting.HostSession();
   var email = /** @type {string} */ (remoting.identity.getCachedEmail());
-  remoting.hostSession.connect(
+  hostSession_.connect(
       hostFacade, email, token, onHostStateChanged_,
       onNatTraversalPolicyChanged_, logDebugInfo_, it2meConnectFailed_);
 };
@@ -102,7 +104,6 @@ remoting.tryShareWithToken_ = function(hostFacade, token) {
  * Callback for the host plugin to notify the web app of state changes.
  * @param {remoting.HostSession.State} state The new state of the plugin.
  * @return {void} Nothing.
- * @private

[kelvinp, 2015/02/04 01:38:10]

This CL wraps the entire file in an IEFE, so that onHostChanged is truly private
and invisible in other files.  Thus no need the @private annotation.

  */
 function onHostStateChanged_(state) {
   if (state == remoting.HostSession.State.STARTING) {
@@ -115,7 +116,7 @@ function onHostStateChanged_(state) {
 
   } else if (state == remoting.HostSession.State.RECEIVED_ACCESS_CODE) {
     console.log('Host state: RECEIVED_ACCESS_CODE');
-    var accessCode = remoting.hostSession.getAccessCode();
+    var accessCode = hostSession_.getAccessCode();
     var accessCodeDisplay = document.getElementById('access-code-display');
     accessCodeDisplay.innerText = '';
     // Display the access code in groups of four digits for readability.
@@ -126,7 +127,7 @@ function onHostStateChanged_(state) {
       nextFourDigits.innerText = accessCode.substring(i, i + kDigitsPerGroup);
       accessCodeDisplay.appendChild(nextFourDigits);
     }
-    accessCodeExpiresIn_ = remoting.hostSession.getAccessCodeLifetime();
+    accessCodeExpiresIn_ = hostSession_.getAccessCodeLifetime();
     if (accessCodeExpiresIn_ > 0) {  // Check it hasn't expired.
       accessCodeTimerId_ = setInterval(decrementAccessCodeTimeout_, 1000);
       timerRunning_ = true;
@@ -143,7 +144,7 @@ function onHostStateChanged_(state) {
   } else if (state == remoting.HostSession.State.CONNECTED) {
     console.log('Host state: CONNECTED');
     var element = document.getElementById('host-shared-message');
-    var client = remoting.hostSession.getClient();
+    var client = hostSession_.getClient();
     l10n.localizeElement(element, client);
     remoting.setMode(remoting.AppMode.HOST_SHARED);
     disableTimeoutCountdown_();
@@ -163,6 +164,7 @@ function onHostStateChanged_(state) {
         remoting.setMode(remoting.AppMode.HOST_SHARE_FINISHED);
       }
     }
+    cleanUp();
   } else if (state == remoting.HostSession.State.ERROR) {
     console.error('Host state: ERROR');
     showShareError_(remoting.Error.UNEXPECTED);
@@ -178,7 +180,6 @@ function onHostStateChanged_(state) {
  * This is the callback that the host plugin invokes to indicate that there
  * is additional debug log info to display.
  * @param {string} msg The message (which will not be localized) to be logged.
- * @private
  */
 function logDebugInfo_(msg) {
   console.log('plugin: ' + msg);
@@ -189,20 +190,19 @@ function logDebugInfo_(msg) {
  *
  * @param {string} errorTag The error message to be localized and displayed.
  * @return {void} Nothing.
- * @private
  */
 function showShareError_(errorTag) {
   var errorDiv = document.getElementById('host-plugin-error');
   l10n.localizeElementFromTag(errorDiv, errorTag);
   console.error('Sharing error: ' + errorTag);
   remoting.setMode(remoting.AppMode.HOST_SHARE_FAILED);
+  cleanUp();
 }
 
 /**
  * Show a sharing error with error code UNEXPECTED .
  *
  * @return {void} Nothing.
- * @private
  */
 function it2meConnectFailed_() {
   // TODO (weitaosu): Instruct the user to install the native messaging host.
@@ -212,6 +212,11 @@ function it2meConnectFailed_() {
   showShareError_(remoting.Error.UNEXPECTED);
 }
 
+function cleanUp() {
+  base.dispose(hostSession_);
+  hostSession_ = null;
+}
+
 /**
  * Cancel an active or pending it2me share operation.
  *
@@ -222,7 +227,7 @@ remoting.cancelShare = function() {
   console.log('Canceling share...');
   remoting.lastShareWasCancelled = true;
   try {
-    remoting.hostSession.disconnect();
+    hostSession_.disconnect();
   } catch (/** @type {*} */ error) {
     console.error('Error disconnecting: ' + error +
                   '. The host probably crashed.');
@@ -237,36 +242,31 @@ remoting.cancelShare = function() {
 
 /**
  * @type {boolean} Whether or not the access code timeout countdown is running.
- * @private
  */
 var timerRunning_ = false;
 
 /**
  * @type {number} The id of the access code expiry countdown timer.
- * @private
  */
 var accessCodeTimerId_ = 0;
 
 /**
  * @type {number} The number of seconds until the access code expires.
- * @private
  */
 var accessCodeExpiresIn_ = 0;
 
 /**
  * The timer callback function
  * @return {void} Nothing.
- * @private
  */
 function decrementAccessCodeTimeout_() {
   --accessCodeExpiresIn_;
   updateAccessCodeTimeoutElement_();
-};
+}
 
 /**
  * Stop the access code timeout countdown if it is running.
  * @return {void} Nothing.
- * @private
  */
 function disableTimeoutCountdown_() {
   if (timerRunning_) {
@@ -278,7 +278,6 @@ function disableTimeoutCountdown_() {
 
 /**
  * Constants controlling the access code timer countdown display.
- * @private
  */
 var ACCESS_CODE_TIMER_DISPLAY_THRESHOLD_ = 30;
 var ACCESS_CODE_RED_THRESHOLD_ = 10;
@@ -289,7 +288,6 @@ var ACCESS_CODE_RED_THRESHOLD_ = 10;
  *
  * @return {boolean} True if the timeout is in progress, false if it has
  * expired.
- * @private
  */
 function updateTimeoutStyles_() {
   if (timerRunning_) {
@@ -314,7 +312,6 @@ function updateTimeoutStyles_() {
  * Update the text and appearance of the access code timeout element to
  * reflect the time remaining.
  * @return {void} Nothing.
- * @private
  */
 function updateAccessCodeTimeoutElement_() {
   var pad = (accessCodeExpiresIn_ < 10) ? '0:0' : '0:';
@@ -329,7 +326,6 @@ function updateAccessCodeTimeoutElement_() {
  * Callback to show or hide the NAT traversal warning when the policy changes.
  * @param {boolean} enabled True if NAT traversal is enabled.
  * @return {void} Nothing.
- * @private
  */
 function onNatTraversalPolicyChanged_(enabled) {
   var natBox = document.getElementById('nat-box');
@@ -339,3 +335,5 @@ function onNatTraversalPolicyChanged_(enabled) {
     natBox.classList.remove('traversal-enabled');
   }
 }
+
+})();