MD History: add sign in promo in synced tabs when user isn't logged in

chrome/browser/resources/md_history/synced_device_manager.js

Index: chrome/browser/resources/md_history/synced_device_manager.js
diff --git a/chrome/browser/resources/md_history/synced_device_manager.js b/chrome/browser/resources/md_history/synced_device_manager.js
index ab273b3e73a71c7d4dddb73667f6e435bae428f2..ec420e0f30ad526eedaabc908f4d8cfd73fcc327 100644
--- a/chrome/browser/resources/md_history/synced_device_manager.js
+++ b/chrome/browser/resources/md_history/synced_device_manager.js
@@ -36,6 +36,16 @@ Polymer({
     syncedDevices_: {
       type: Array,
       value: function() { return []; }
+    },
+
+    signInState_: {
+      type: Boolean,
+      value: loadTimeData.getBoolean('isUserSignedIn'),
+    },
+
+    showNoSyncedMessage_: {
+      type: Boolean,
+      value: false,
     }
   },
 
@@ -81,6 +91,25 @@ Polymer({
     };
   },
 
+
+  onSignInTap_: function() {
+    chrome.send('SyncSetupShowSetupUI');
+    chrome.send('SyncSetupStartSignIn', [false]);
+  },
+
+  /**
+   * Change the message showing when user is logged in and there is no synced
+   * tabs. Also decide whether or not should display the message.
+   * @param {boolean} fetchingSyncedTabs
+   * @private
+   */
+  updateSyncedTabMessage_: function(fetchingSyncedTabs) {
+    var messageId = fetchingSyncedTabs ? 'loading' : 'noSyncedResults';
+    this.$['no-synced-tabs'].textContent = loadTimeData.getString(messageId);
+    this.showNoSyncedMessage_ = this.signInState_ &&
+                                (this.syncedDevices_.length == 0);

[calamity, 2016/06/17 06:25:45]

It would be 'more Polymer' to use a computed binding to change the text.

The two things you want to compute are 'whether the message should be displayed
at all' and 'what the message should be'.

So it should be:

noSyncedTabsMessage: function(signInState) {
  return loadTimeData.getString(signInState ? 'noSyncedResults' : 'loading');
}

showNoSyncedTabsMessage: function(signInState, syncedDevicesLength) {
  return signInState && syncedDevicesLength == 0;
}

then in the html,

...
hidden="[[!showNoSyncedTabsMessage(signInState_, syncedDevices_.length]]">
  [[noSyncedTabsMessage(signInState_)]]
</div>

[lshang, 2016/06/21 03:00:26]

Done.
It's really cool that computed binding will be recalculated whenever any of the
params changes.

+  },
+
   /**
    * Replaces the currently displayed synced tabs with |sessionList|. It is
    * common for only a single session within the list to have changed, We try to
@@ -90,8 +119,10 @@ Polymer({
    * @param {?Array<!ForeignSession>} sessionList
    */
   updateSyncedDevices: function(sessionList) {
-    if (!sessionList)
+    if (!sessionList) {
+      this.updateSyncedTabMessage_(false);
       return;
+    }
 
     // First, update any existing devices that have changed.
     var updateCount = Math.min(sessionList.length, this.syncedDevices_.length);
@@ -108,6 +139,27 @@ Polymer({
     for (var i = updateCount; i < sessionList.length; i++) {
       this.push('syncedDevices_', this.createInternalDevice_(sessionList[i]));
     }
+
+    this.updateSyncedTabMessage_(false);
+  },
+
+  updateSignInState: function(isUserSignedIn) {
+    // If user's sign in state didn't change, then don't change message or
+    // update synced devices.
+    if (this.signInState_ == isUserSignedIn)
+      return;

[calamity, 2016/06/17 06:25:45]

You can remove this if you do the above since notifications won't propagate
through when the value is the same.

[lshang, 2016/06/21 03:00:26]

When user changes state from signed out to signed in, then there is a short
period of time when it's fetching synced tabs and message 'Loading' is
displayed, then after the data returns, message will be 'no synced tabs'. We
want to make sure that 'Loading' only be displayed when user's state is changed
from signed out to signed in.

I did this because there are occasions when user has two Chrome windows with
different accounts A and B, A is logged in and B is not logged in. when user
also signs in account B, both A and B will receive updateSignInState
notifications from backend, but we would only want B to show the 'Loading'
message.

+    this.signInState_ = isUserSignedIn;
+
+    // User signed out, clear session list and show the sign in promo.
+    if (!isUserSignedIn) {
+      this.sessionList = [];
+      this.syncedDevices_ = [];

[calamity, 2016/06/17 06:25:45]

You should make a method called clearDisplayedSyncedDevices that just runs
this.syncedDevices = [].

It's getting confusing to remember why we need to clear both the session list
_and_ the syncedDevices array.

[lshang, 2016/06/21 03:00:26]

Done.
Yeah I think we just want to clear the UI here.

+      this.updateSyncedDevices(this.sessionList);

[calamity, 2016/06/17 06:25:45]

I don't think you'll need this line either.

[lshang, 2016/06/21 03:00:26]

Done.

+    } else {
+      // User signed in, show the loading message when querying for synced
+      // devices.
+      this.updateSyncedTabMessage_(true);
+    }

[calamity, 2016/06/17 06:25:45]

All of this is fairly nuanced, we should have a test in
history_synced_tabs_test.js that makes sure the right things are being hidden
and shown for different configurations of signin and sessions.

[lshang, 2016/06/21 03:00:26]

Done.

   },
 
   searchTermChanged: function(searchedTerm) {