[Chromoting] Show any startup errors in the LoadingWindow.

remoting/webapp/base/js/message_window.js

Index: remoting/webapp/base/js/message_window.js
diff --git a/remoting/webapp/base/js/message_window.js b/remoting/webapp/base/js/message_window.js
index f110e6f659a6162dd852f061f2497cb45bd0187d..2292f28e537a6198fcb213479a5f4ae69d5b4500 100644
--- a/remoting/webapp/base/js/message_window.js
+++ b/remoting/webapp/base/js/message_window.js
@@ -44,19 +44,16 @@ MessageWindowImpl.prototype.sendReply_ = function(
 };
 
 /**
- * Initializes the button with the label and the click handler.
+ * Updates the button label text.
  * Hides the button if the label is null or undefined.
  *
  * @param{HTMLElement} button
  * @param{?string} label
- * @param{Function} clickHandler
  * @private
  */
-MessageWindowImpl.prototype.initButton_ =
-    function(button, label, clickHandler) {
+MessageWindowImpl.prototype.updateButton_ = function(button, label) {
   if (label) {
     button.innerText = label;
-    button.addEventListener('click', clickHandler, false);
   }
   button.hidden = !Boolean(label);
 };
@@ -69,86 +66,87 @@ MessageWindowImpl.prototype.initButton_ =
  * @private
  */
 MessageWindowImpl.prototype.onMessage_ = function(event) {
-  switch (event.data['command']) {
-    case 'show':
-      // Validate the message.
-      var messageId = /** @type {number} */ (event.data['id']);
-      var title = /** @type {string} */ (event.data['title']);
-      var message = /** @type {string} */ (event.data['message']);
-      var infobox = /** @type {string} */ (event.data['infobox']);
-      var buttonLabel = /** @type {string} */ (event.data['buttonLabel']);
-      /** @type {string} */
-      var cancelButtonLabel = (event.data['cancelButtonLabel']);
-      var showSpinner = /** @type {boolean} */ (event.data['showSpinner']);
-      if (typeof(messageId) != 'number' ||
-          typeof(title) != 'string' ||
-          typeof(message) != 'string' ||
-          typeof(infobox) != 'string' ||
-          typeof(buttonLabel) != 'string' ||
-          typeof(showSpinner) != 'boolean') {
-        console.log('Bad show message:', event.data);
-        break;
-      }
-
-      // Set the dialog text.
-      var button = document.getElementById('button-primary');
-      var cancelButton = document.getElementById('button-secondary');
-      var messageDiv = document.getElementById('message');
-      var infoboxDiv = document.getElementById('infobox');
-
-      document.getElementById('title').innerText = title;
-      document.querySelector('title').innerText = title;
-      messageDiv.innerHTML = message;
-
-      if (showSpinner) {
-        messageDiv.classList.add('waiting');
-        messageDiv.classList.add('prominent');
-      }
-      if (infobox != '') {
-        infoboxDiv.innerText = infobox;
-      } else {
-        infoboxDiv.hidden = true;
-      }
-
-      this.initButton_(
-          button,
-          buttonLabel,
-          this.sendReply_.bind(this, event.source, messageId, 1));
-
-      this.initButton_(
-          cancelButton,
-          cancelButtonLabel,
-          this.sendReply_.bind(this, event.source, messageId, 0));
-
-      var buttonToFocus = (cancelButtonLabel) ? cancelButton : button;
-      buttonToFocus.focus();
-
-      // Add a close handler in case the window is closed without clicking one
-      // of the buttons. This will send a 0 as the result.
-      // Note that when a button is pressed, this will result in sendReply_
-      // being called multiple times (once for the button, once for close).
-      chrome.app.window.current().onClosed.addListener(
-          this.sendReply_.bind(this, event.source, messageId, 0));
-
-      base.resizeWindowToContent();
-      chrome.app.window.current().show();
-      break;
-
-    case 'update_message':
-      var message = /** @type {string} */ (event.data['message']);
-      if (typeof(message) != 'string') {
-        console.log('Bad update_message message:', event.data);
-        break;
-      }
-
-      var messageDiv = document.getElementById('message');
-      messageDiv.innerText = message;
-
-      base.resizeWindowToContent(true);
-      break;
-
-    default:
-      console.error('Unexpected message:', event.data);
+  var command = /** @type {string} */ (event.data['command']);
+  if (command !== 'show' && command !== 'update') {
+    console.error('Unexpected message: ' + command);
+    return;
+  }
+
+  // Validate the message.
+  var messageId = /** @type {number} */ (event.data['id']);
+  var title = /** @type {string} */ (event.data['title']);
+  var message = /** @type {string} */ (event.data['message']);
+  var infobox = /** @type {string} */ (event.data['infobox']);
+  var buttonLabel = /** @type {string} */ (event.data['buttonLabel']);
+  var cancelButtonLabel = /** @type {string} */
+      (event.data['cancelButtonLabel']);
+  var showSpinner = /** @type {boolean} */ (event.data['showSpinner']);
+
+  // For the 'show' command, these values are required; whereas they are
+  // all optional for the 'update' command.

[Jamie, 2015/05/11 22:54:53]

I think we should do the type check regardless. For 'show', you should
additionally check that they're not undefined. It would also make the
conditionals later on clearer, ie:

  if (title !== undefined) {...}

instead of

  if (typeof(title) === 'string') {...}

[garykac, 2015/05/13 22:05:27]

The same needs to be done for cancelButtonLabel (which is an optional argument
for 'show').

+  if (command === 'show') {
+    if (typeof(messageId) !== 'number' ||
+        typeof(title) !== 'string' ||
+        typeof(message) !== 'string' ||
+        typeof(infobox) !== 'string' ||
+        typeof(buttonLabel) !== 'string' ||
+        typeof(showSpinner) !== 'boolean') {
+      console.log('Bad show message: ' + event.data);
+      return;
+    }
+  }
+
+  var button = document.getElementById('button-primary');
+  var cancelButton = document.getElementById('button-secondary');
+  var messageDiv = document.getElementById('message');
+  var infoboxDiv = document.getElementById('infobox');
+
+  if (typeof(title) === 'string') {
+    document.getElementById('title').innerText = title;
+    document.querySelector('title').innerText = title;
+  }
+  if (typeof(message) === 'string') {
+    messageDiv.innerText = message;
+  }
+  if (typeof(infobox) === 'string') {
+    if (infobox != '') {
+      infoboxDiv.innerText = infobox;
+    } else {
+      infoboxDiv.hidden = true;
+    }
+  }
+  if (typeof(showSpinner) === 'boolean') {
+    if (showSpinner) {
+      messageDiv.classList.add('waiting');
+      messageDiv.classList.add('prominent');
+    } else {
+      messageDiv.classList.remove('waiting');
+      messageDiv.classList.remove('prominent');
+    }
+  }
+  this.updateButton_(button, buttonLabel);
+  this.updateButton_(cancelButton, cancelButtonLabel);
+
+  base.resizeWindowToContent();
+
+  if (command === 'show') {
+    // Set up clickhandlers for the buttons.

[Jamie, 2015/05/11 22:54:53]

s/clickhandlers/click-handlers/

[garykac, 2015/05/13 22:05:27]

Done.

+    button.addEventListener(
+        'click', this.sendReply_.bind(this, event.source, messageId, 1), false);
+    cancelButton.addEventListener(
+        'click', this.sendReply_.bind(this, event.source, messageId, 0), false);

[Jamie, 2015/05/11 22:54:53]

Optional: Consider moving the event handlers to the ctor, which is more
consistent with what we do with other classes.

[garykac, 2015/05/13 22:05:27]

That doesn't feel like it belongs in this cl.

It might change the behavior of when message (like close window) are sent.
Currently, we'll only send close messages for windows that have been shown.

[Jamie, 2015/05/13 23:43:17]

Acknowledged.

+
+    var buttonToFocus = (cancelButtonLabel) ? cancelButton : button;
+    buttonToFocus.focus();
+
+    // Add a close handler in case the window is closed without clicking one
+    // of the buttons. This will send a 0 as the result.
+    // Note that when a button is pressed, this will result in sendReply_
+    // being called multiple times (once for the button, once for close).
+    chrome.app.window.current().onClosed.addListener(
+        this.sendReply_.bind(this, event.source, messageId, 0));
+
+    chrome.app.window.current().show();
   }
 };