[Webapp Refactor] Reliably cancels a connection.

remoting/webapp/base/js/modal_dialogs.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
 (function() {
 
 'use strict';
@@ skipping 38 matching lines @@
       new base.DomEventHook(this.cancelButton_, 'click', onCancel, false));
   base.debug.assert(this.deferred_ === null);
   this.deferred_ = new base.Deferred();
   remoting.setMode(this.appMode_);
   return this.deferred_.promise();
 };
 
 /** @return {HTMLElement} */
 remoting.InputDialog.prototype.inputField = function() {
   return this.inputField_;
-}
+};
 
 /** @private */
 remoting.InputDialog.prototype.onSubmit_ = function() {
   this.deferred_.resolve(this.inputField_.value);
-}
+};
 
 /** @private */
 remoting.InputDialog.prototype.onCancel_ = function() {
   this.deferred_.reject(new remoting.Error(remoting.Error.Tag.CANCELLED));
-}
+};
 
 /**
  * @param {function():void} handler
  * @return {Function}
  * @private
  */
 remoting.InputDialog.prototype.createFormEventHandler_ = function(handler) {
   var that = this;
   return function (/** Event */ e) {
     // Prevents form submission from reloading the v1 app.
@@ skipping 49 matching lines @@
         this.onClicked_.bind(this, remoting.MessageDialog.Result.SECONDARY),
         false));
   }
 
   base.debug.assert(this.deferred_ === null);
   this.deferred_ = new base.Deferred();
   remoting.setMode(this.mode_);
   return this.deferred_.promise();
 };
 
+remoting.MessageDialog.prototype.reset = function() {

[Jamie, 2015/04/22 23:11:55]

Why not call this dispose() and use the base.Disposable pattern? Alternatively,
make it private.

[kelvinp, 2015/04/23 01:17:23]

Done.

+  base.dispose(this.eventHooks_);
+  this.eventHooks_ = null;
+  this.deferred_ = null;

[Jamie, 2015/04/22 23:11:55]

If this remains public, you should call deferred_.reject(CANCELLED) if it's not
null, since the caller can't know if there's something pending.

[kelvinp, 2015/04/23 01:17:23]

Done.

+};
+
 /**
  * @param {remoting.MessageDialog.Result} result
  * @return {Function}
  * @private
  */
 remoting.MessageDialog.prototype.onClicked_ = function(result) {
   this.deferred_.resolve(result);
-  base.dispose(this.eventHooks_);
-  this.eventHooks_ = null;
-  this.deferred_ = null;
+  this.reset();
+};
+
+/**
+ * @param {Function} cancelCallback The callback to invoke when the user clicks
+ *     on the cancel button.

[Jamie, 2015/04/22 23:11:55]

Why not make show return a Promise?

[kelvinp, 2015/04/23 01:17:23]

The cancel dialog has a different use case than other modal dialogs.
For other dialogs, its dismissal is mainly user driven and we would like to
block until the user clicks on a button. A promise is ideal for that.

For the cancel dialog, its dismissal driven by the connection transiting to the
next state (PIN is prompted) AND user cancelation.  By using a callback, instead
of a then handler, it provides a stronger guarantee that the stop callback won't
be invoked multiple times.

[Jamie, 2015/04/24 17:59:18]

I don't understand. A Promise is guaranteed one-shot, whereas a callback can be
invoked by anything that has a reference to it as many times as it likes.

[kelvinp, 2015/04/24 19:03:52]

While a promise only resolves once, every time when then() is called, another
callback is registered to it and will be invoked if the promise has already
resolved.

Taking me2me as an example, show() is called in multiple places (before and
after PIN prompt, with essentially the same callback.  Using a cancelCallback in
the constructor saves us from duplicating the canceling code.

As discussed, we can revisit this in the future to whether repurpose the PIN
dialog as a 
connecting dialog.

+ * @constructor
+ */
+remoting.ConnectingDialog = function(cancelCallback) {
+  /** @private */
+  this.dialog_ = new remoting.MessageDialog(
+      remoting.AppMode.CLIENT_CONNECTING,
+      document.getElementById('cancel-connect-button'));
+  /** @private */
+  this.onCancel_ = cancelCallback;
+};
+
+remoting.ConnectingDialog.prototype.show = function() {
+  var that = this;
+  this.dialog_.show().then(function() {
+    remoting.setMode(remoting.AppMode.HOME);
+    that.onCancel_();
+  });
+};
+
+remoting.ConnectingDialog.prototype.hide = function() {
+  this.dialog_.reset();
 };
 
 })();
 
 /**
  * Define the enum at the end of file as JSCompile doesn't understand enums that
  * are defined within an IIFE (Immediately Invoked Function Expression).
  * @enum {number}
  */
 remoting.MessageDialog.Result = {
   PRIMARY: 0,
   SECONDARY: 1
 };