[Chromoting] Show any startup errors in the LoadingWindow.

remoting/webapp/base/js/message_window.js

 // Copyright 2014 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.
 
 'use strict';
 
 /**
  * @constructor
  */
 function MessageWindowImpl() {
@@ skipping 26 matching lines @@
     };
     parentWindow.postMessage(message, '*');
     this.sentReply_ = true;
   } else {
     // Make sure that the reply we're ignoring is from the window close.
     base.debug.assert(result == 0);
   }
 };
 
 /**
- * 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);
 };
 
 /**
  * Event-handler callback, invoked when the parent window supplies the
  * message content.
  *
  * @param{Event} event
  * @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;
-      }
+  var command = /** @type {string} */ (event.data['command']);
+  if (command !== 'show' && command !== 'update') {
+    console.error('Unexpected message: ' + command);
+    return;
+  }
 
-      // 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');
+  // 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']);
 
-      document.getElementById('title').innerText = title;
-      document.querySelector('title').innerText = title;
-      messageDiv.innerHTML = message;
+  // 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;
+    }
+  }
 
-      if (showSpinner) {
-        messageDiv.classList.add('waiting');
-        messageDiv.classList.add('prominent');
-      }
-      if (infobox != '') {
-        infoboxDiv.innerText = infobox;
-      } else {
-        infoboxDiv.hidden = true;
-      }
+  var button = document.getElementById('button-primary');
+  var cancelButton = document.getElementById('button-secondary');
+  var messageDiv = document.getElementById('message');
+  var infoboxDiv = document.getElementById('infobox');
 
-      this.initButton_(
-          button,
-          buttonLabel,
-          this.sendReply_.bind(this, event.source, messageId, 1));
+  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);
 
-      this.initButton_(
-          cancelButton,
-          cancelButtonLabel,
-          this.sendReply_.bind(this, event.source, messageId, 0));
+  base.resizeWindowToContent();
 
-      var buttonToFocus = (cancelButtonLabel) ? cancelButton : button;
-      buttonToFocus.focus();
+  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.

 
-      // 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));
+    var buttonToFocus = (cancelButtonLabel) ? cancelButton : button;
+    buttonToFocus.focus();
 
-      base.resizeWindowToContent();
-      chrome.app.window.current().show();
-      break;
+    // 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));
 
-    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);
+    chrome.app.window.current().show();
   }
 };
 
 var messageWindow = new MessageWindowImpl();