Add link to Hangouts Remote Desktop in the It2Me section of the remoting app.

remoting/webapp/butter_bar.js

+// Copyright 2013 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.
+
+/**
+ * @fileoverview
+ * ButterBar class that is used to show the butter bar with various
+ * notifications.
+ */
+
+'use strict';
+
+/** @suppress {duplicate} */
+var remoting = remoting || {};
+
+/**
+ * Shows butter bar with the specified |message| and updates |storageKey| after
+ * the bar is dismissed.
+ *
+ * @param {string} message
+ * @param {string} storageKey
+ * @constructor
+ */
+remoting.ButterBar = function(message, storageKey) {
+  this.storageKey = storageKey;
+
+  var messageElement = document.getElementById(remoting.ButterBar.kMessageId);
+  messageElement.innerHTML = message;

[Jamie, 2013/05/30 01:10:17]

innerHTML makes me nervous :( The only other place we use it is in
l10n.localizeElementFromTag. Would it be possible to change this code to use
that? That way, if we want to sanitize the HTML (for example, whitelisting only
the tags we use) there's only one place to do it.

[Sergey Ulanov, 2013/05/30 22:04:54]

The alternative is what I had before - have each message inserted in main.html
and show one of them depending on what's requested. I can revert back to that
approach.

It should be safe to use innerHTML as long as we control the inserted message.
In this case the message comes from resources and substitutions are hardcoded,
so it should be safe.

[Jamie, 2013/05/31 00:12:42]

I like this approach more--the HTML is simpler this way and I think it's cleaner
overall. However, I think you can reuse localizeElementFromTag if you pass in
the i18n-content tag and substitution parameters to the ctor instead of
localizing it beforehand.
Yes, I think it's probably safe enough as written here, but I'd like to minimize
the use as long as it doesn't make the code any messier than it needs to be.

[Sergey Ulanov, 2013/06/03 19:11:48]

Done.

+  var acceptLink =
+      /** @type{Element} */ messageElement.getElementsByTagName('a')[0];
+  acceptLink.addEventListener(
+      'click', this.dismiss.bind(this, true), false);
+
+  document.getElementById(remoting.ButterBar.kDismissId).addEventListener(
+      'click', this.dismiss.bind(this, false), false);
+
+  document.getElementById(remoting.ButterBar.kId).hidden = false;
+}
+
+/** @const */

[Jamie, 2013/05/30 01:10:17]

I think these can all be made private (in which case they should end in an
underscore).

[Sergey Ulanov, 2013/05/30 22:04:54]

Done.

+remoting.ButterBar.kId = 'butter-bar';
+
+/** @const */
+remoting.ButterBar.kMessageId = 'butter-bar-message';
+/** @const */
+remoting.ButterBar.kDismissId = 'butter-bar-dismiss';
+
+/** @const */
+remoting.kSurveyStorageKey = 'feedback-survey-dismissed';
+/** @const */
+remoting.kHangoutsStorageKey = 'hangouts-notice-dismissed';
+
+/**
+ * Hide the butter bar request and record some basic information about the
+ * current state of the world in synced storage. This may be useful in the
+ * future if we want to show the request again. At the moment, the data itself
+ * is ignored; only its presence or absence is important.
+ *
+ * @param {boolean} accepted True if the user clicked the "accept" link;
+ *     false if they clicked the close icon.
+ */
+remoting.ButterBar.prototype.dismiss = function(accepted) {
+  var value = {};
+  value[this.storageKey] = {
+    accepted: accepted,
+    date: new Date(),
+    version: chrome.runtime.getManifest().version
+  };
+  chrome.storage.sync.set(value);
+
+  document.getElementById(remoting.ButterBar.kId).hidden = true;
+};
+
+/**
+ * Show the butter bar if there is a message to show.
+ */
+remoting.initButterBar = function() {
+  /** @param {Object} syncValues
+    * @param {Object} localValues */
+  var onStateLoaded = function(syncValues, localValues) {
+    /** @type {boolean} */
+    var surveyDismissed = !!syncValues[remoting.kSurveyStorageKey];
+    /** @type {boolean} */
+    var hangoutsDismissed = !!syncValues[remoting.kHangoutsStorageKey];
+    /** @type {boolean} */
+    var it2meExpanded = !!localValues[remoting.kIT2MeVisitedStorageKey];
+
+    var showSurvey = !surveyDismissed;
+    var showHangouts = it2meExpanded && !hangoutsDismissed;
+
+    // If both messages can be shown choose only one randomly.
+    if (showSurvey && showHangouts) {
+      if (Math.random() > 0.5) {
+        showSurvey = false;
+      } else {
+        showHangouts = false;
+      }
+    }
+
+    if (showSurvey) {
+      new remoting.ButterBar(

[Jamie, 2013/05/30 01:10:17]

I'm always skeptical of a 'new' statement that doesn't assign to anything. In
this case, I think it would be better to create a remoting.butterBar variable
somewhere (most likely in remoting.init, where most of our globals are created),
then have a non-static init method that selects the content to display.

[Sergey Ulanov, 2013/05/30 22:04:54]

Done.

+          chrome.i18n.getMessage(
+            /*i18n-content*/'SURVEY_INVITATION',
+            ['<a href="http://goo.gl/njH2q" target="_blank">', '</a>']),
+          remoting.kSurveyStorageKey);
+    } else if (showHangouts) {
+      new remoting.ButterBar(
+          chrome.i18n.getMessage(
+              /*i18n-content*/'HANGOUTS_INVITATION',
+              ['<a id="hangouts-accept" ' +
+               'href="https://plus.google.com/hangouts/_?gid=818572447316">',
+               '</a>']),
+          remoting.kHangoutsStorageKey);
+    }
+  };
+
+  /** @param {Object} syncValues */
+  var onSyncDataLoaded = function(syncValues) {
+    chrome.storage.local.get(
+        remoting.kIT2MeVisitedStorageKey,
+        onStateLoaded.bind(null, syncValues));
+  };
+
+  chrome.storage.sync.get(
+      [remoting.kSurveyStorageKey, remoting.kHangoutsStorageKey],
+      onSyncDataLoaded);
+}