Trap errors when disconnecting in case the host plugin has crashed.

BUG=93833
TEST=Make the host plugin crash while waiting for a connection. Check that an errror is reported when the timeout expires.

Review URL: http://codereview.chromium.org/7792003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98995 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 49 insertions, 24 deletions

diff --git a/remoting/webapp/me2mom/choice.html b/remoting/webapp/me2mom/choice.html
index 5735b39..b87e2c1 100644
--- a/remoting/webapp/me2mom/choice.html
+++ b/remoting/webapp/me2mom/choice.html
@@ -137,9 +137,7 @@ found in the LICENSE file.
         </div> <!-- host.shared -->
 
         <div data-ui-mode="host.share-failed" class="message">
-          <span id="unable-to-get-token" class="error-state"
-                i18n-content="ERROR_UNABLE_TO_GET_TOKEN">
-          </span>
+          <span id="host-plugin-error" class="error-state"></span>
         </div> <!-- host.share-failed -->
 
         <div data-ui-mode="host.share-finished" class="message"
diff --git a/remoting/webapp/me2mom/l10n.js b/remoting/webapp/me2mom/l10n.js
index d504f90..7f00192 100644
--- a/remoting/webapp/me2mom/l10n.js
+++ b/remoting/webapp/me2mom/l10n.js
@@ -6,15 +6,15 @@
 var l10n = l10n || {};
 
 /**
- * Localize an element by setting its innerText according to its i18n-content
- * attribute, and an optional set of substitutions.
+ * Localize an element by setting its innerText according to the specified tag
+ * and an optional set of substitutions.
  * @param {Element} element The element to localize.
+ * @param {string} tag The localization tag.
  * @param {{string|Object}=} opt_substitutions An optional set of substitution
  *     strings corresponding to the "placeholders" attributes in messages.json.
  * @return {boolean} True if the localization was successful; false otherwise.
  */
-l10n.localizeElement = function(element, opt_substitutions) {
-  var tag = element.getAttribute('i18n-content');
+l10n.localizeElementFromTag = function(element, tag, opt_substitutions) {
   var translation = chrome.i18n.getMessage(tag, opt_substitutions);
   if (translation) {
     element.innerHTML = translation;
@@ -22,6 +22,19 @@ l10n.localizeElement = function(element, opt_substitutions) {
     console.error('Missing translation for "' + tag + '":', element);
   }
   return translation != null;
+}
+
+/**
+ * Localize an element by setting its innerText according to its i18n-content
+ * attribute, and an optional set of substitutions.
+ * @param {Element} element The element to localize.
+ * @param {{string|Object}=} opt_substitutions An optional set of substitution
+ *     strings corresponding to the "placeholders" attributes in messages.json.
+ * @return {boolean} True if the localization was successful; false otherwise.
+ */
+l10n.localizeElement = function(element, opt_substitutions) {
+  var tag = element.getAttribute('i18n-content');
+  return l10n.localizeElementFromTag(element, tag, opt_substitutions);
 };
 
 /**
diff --git a/remoting/webapp/me2mom/remoting.js b/remoting/webapp/me2mom/remoting.js
index 7b97a44..ca8e3c2 100644
--- a/remoting/webapp/me2mom/remoting.js
+++ b/remoting/webapp/me2mom/remoting.js
@@ -218,7 +218,7 @@ remoting.tryShare = function() {
     remoting.oauth2.refreshAccessToken(function() {
       if (remoting.oauth2.needsNewAccessToken()) {
         // If we still need it, we're going to infinite loop.
-        showShareError_('unable-to-get-token');
+        showShareError_(/*i18n-content*/'ERROR_UNABLE_TO_GET_TOKEN');
         throw 'Unable to get access token';
       }
       remoting.tryShare();
@@ -319,8 +319,8 @@ function onStateChanged_() {
       updateTimeoutStyles_();
       remoting.setMode(remoting.AppMode.HOST_WAITING_FOR_CONNECTION);
     } else {
-      // This can only happen if the access code takes more than 5m to get from
-      // the cloud to the web-app, so we don't care how clean this UX is.
+      // This can only happen if the cloud tells us that the code lifetime is
+      // <= 0s, which shouldn't happen so we don't care how clean this UX is.
       remoting.debug.log('Access code already invalid on receipt!');
       remoting.cancelShare();
     }
@@ -346,16 +346,23 @@ function onStateChanged_() {
 }
 
 /**
-* This is that callback that the host plugin invokes to indicate that there
+* 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.
 */
 function debugInfoCallback_(msg) {
   remoting.debug.log('plugin: ' + msg);
 }
 
-function showShareError_(errorCode) {
-  var errorDiv = document.getElementById(errorCode);
-  errorDiv.style.display = 'block';
+/**
+* Show a host-side error message.
+*
+* @param {string} errorCode The error message to be localized and displayed.
+* @return {void} Nothing.
+*/
+function showShareError_(errorTag) {
+  var errorDiv = document.getElementById('host-plugin-error');
+  l10n.localizeElementFromTag(errorDiv, errorTag);
   remoting.debug.log('Sharing error: ' + errorCode);
   remoting.setMode(remoting.AppMode.HOST_SHARE_FAILED);
 }
@@ -363,7 +370,17 @@ function showShareError_(errorCode) {
 remoting.cancelShare = function() {
   remoting.debug.log('Canceling share...');
   var plugin = document.getElementById(remoting.HOST_PLUGIN_ID);
-  plugin.disconnect();
+  try {
+    plugin.disconnect();
+  } catch (error) {
+    remoting.debug.log('Error disconnecting: ' + error.description +
+                       '. The host plugin probably crashed.');
+    // TODO(jamiewalch): Clean this up. We should have a class representing
+    // the host plugin, like we do for the client, which should handle crash
+    // reporting and it should use a more detailed error message than the
+    // default 'generic' one. See crbug.com/94624
+    showShareError_(/*i18n-content*/'ERROR_GENERIC');
+  }
   disableTimeoutCountdown_();
 }
 
@@ -462,18 +479,15 @@ function startSession_() {
 }
 
 /**
- * @param {ClientError} errorMsg The error to be localized and displayed.
+ * Show a client-side error message.
+ *
+ * @param {ClientError} errorTag The error to be localized and displayed.
  * @return {void} Nothing.
  */
-function showConnectError_(errorMsg) {
+function showConnectError_(errorTag) {
   remoting.debug.log('Connection failed: ' + errorMsg);
-  var translation = chrome.i18n.getMessage(errorMsg);
-  if (translation) {
-    var errorNode = document.getElementById('connect-error-message');
-    errorNode.innerText = translation;
-  } else {
-    remoting.debug.error('Missing translation for ' + errorMsg);
-  }
+  var errorDiv = document.getElementById('connect-error-message');
+  l10n.localizeElementFromTag(errorDiv, errorTag);
   remoting.accessCode = '';
   if (remoting.session) {
     remoting.session.disconnect();