Add a sign-in promo message to the Other Devices menu.

chrome/browser/resources/ntp4/other_sessions.js

Index: chrome/browser/resources/ntp4/other_sessions.js
diff --git a/chrome/browser/resources/ntp4/other_sessions.js b/chrome/browser/resources/ntp4/other_sessions.js
index ccd693d67b8f1385a813ace1e3733ac51e0199bf..0de06d26a36d8f523c24c600e4e95c6b3255a162 100644
--- a/chrome/browser/resources/ntp4/other_sessions.js
+++ b/chrome/browser/resources/ntp4/other_sessions.js
@@ -24,6 +24,11 @@ cr.define('ntp', function() {
   };
   var HISTOGRAM_EVENT_LIMIT = HISTOGRAM_EVENT.LINK_RIGHT_CLICKED + 1;
 
+  var isUserSignedIn = false;
+
+  // Function to show the sync sign-in UI.
+  var showSignInUI;

[Dan Beam, 2012/03/23 21:12:47]

s/UI/Ui/ (and if it's like this anywhere else, fix, :P)

[Patrick Dubroy, 2012/03/26 15:16:46]

Hmmm. The style guide says, "Prefer to capitalize well-known abbreviations as if
they were words" but it seems to be much more common to use UI rather than Ui.
If I change it here, I'm not sure where to stop. There are 81 instances of
"LoginUI" in the codebase.

[Dan Beam, 2012/03/26 17:08:33]

Then we need to eventually update those to *Ui* if being used as identifiers.
It's in the style guide:

  Prefer to capitalize well-known abbreviations as if they were words: HttpUrl
instead of HTTPURL.

http://dev.chromium.org/developers/coding-style#Naming

[Patrick Dubroy, 2012/03/26 19:36:46]

I'm pretty sure "prefer" != "must". There is also an overriding rule that it's
better to be consistent, which I think applies here.

+
   OtherSessionsMenuButton.prototype = {
     __proto__: MenuButton.prototype,
 
@@ -40,8 +45,24 @@ cr.define('ntp', function() {
       this.anchorType = cr.ui.AnchorType.ABOVE;
       this.invertLeftRight = true;
 
-      chrome.send('getForeignSessions');
       this.recordUmaEvent_(HISTOGRAM_EVENT.INITIALIZED);
+      this.classList.remove('invisible');
+    },
+
+    /**
+     * Initialize the menu.
+     * @param {Boolean} signedIn Is the current user signed in?

[Dan Beam, 2012/03/23 21:12:47]

s/Boolean/boolean/

[Patrick Dubroy, 2012/03/26 15:16:46]

Done.

+     * @param {function} signInCallback A function to open the sign-in UI.
+     */
+    initialize: function(signedIn, signInCallback) {
+      isUserSignedIn = signedIn;
+      if (signInCallback)
+        showSignInUI = signInCallback;
+
+      // Always show the promo, because getForeignSessions can be a no-op.
+      this.showPromo_();
+      if (signedIn)
+        chrome.send('getForeignSessions');
     },
 
     /**
@@ -123,12 +144,25 @@ cr.define('ntp', function() {
 
     /**
      * Create the UI for the promo and place it inside the menu.
-     * The promo is shown instead of foreign session data when tab sync is
-     * not enabled for a profile.
+     * The promo is shown when there is no foreign session data available.
      */
     showPromo_: function() {
-      var message = localStrings.getString('otherSessionsEmpty');
-      this.menu.appendChild(this.ownerDocument.createTextNode(message));
+      // Clear the current contents of the menu.
+      this.menu.innerHTML = '';
+
+      var emptyMessage = localStrings.getString('otherSessionsEmpty');
+      var doc = this.ownerDocument;
+      this.menu.appendChild(doc.createTextNode(emptyMessage));

[Dan Beam, 2012/03/23 21:12:47]

eh, just put this in the markup and hide/unhide, will make code simpler.

[Patrick Dubroy, 2012/03/26 15:16:46]

I put it into the markup in the template section. We still need to insert it
dynamically, because the menu itself is not defined in markup.

+      this.menu.appendChild(doc.createElement('p'));
+      var link = doc.createElement('span');
+      link.classList.add('link-span');
+      link.textContent = localStrings.getString('otherSessionsSignInText');
+      link.addEventListener('click', function(e) {
+        console.log(showSignInUI);

[Dan Beam, 2012/03/23 21:12:47]

remove

[Patrick Dubroy, 2012/03/26 15:16:46]

Done.

+        showSignInUI(this);

[Dan Beam, 2012/03/23 21:12:47]

use e.currentTarget, don't use |this| in JavaScript as much as possible, it's
fragile a source of subtle bugs, i.e.

  if (this.booleanFlag) // if scope changes this will go unnoticed |this| is
                        // guaranteed to be an object and undefined co-erces

[Patrick Dubroy, 2012/03/26 15:16:46]

Done. Thanks, that's a good thing to remember for all JS code that I write.

+        e.preventDefault();
+      });
+      this.menu.appendChild(link);
     },
 
     /**
@@ -137,20 +171,25 @@ cr.define('ntp', function() {
      * from other devices.
      */
     set sessions(sessionList) {
-      // Clear the current contents of the menu.
-      this.menu.innerHTML = '';
-
-      // Rebuild the menu with the new data.
-      for (var i = 0; i < sessionList.length; i++) {
-        this.addSession_(sessionList[i]);
+      this.sessions_ = sessionList;
+      if (sessionList.length == 0) {
+        this.showPromo_();
+      } else {
+        // Clear the current contents of the menu.
+        this.menu.innerHTML = '';

[Dan Beam, 2012/03/23 21:12:47]

can you move this to central function that's called both places?

[Patrick Dubroy, 2012/03/26 15:16:46]

Done.

+
+        // Rebuild the menu with the new data.
+        for (var i = 0; i < sessionList.length; i++) {
+          this.addSession_(sessionList[i]);
+        }
       }
+    },
 
-      if (sessionList.length == 0)
-        this.classList.add('invisible');
-      else
-        this.classList.remove('invisible');
-
-      this.sessions_ = sessionList;
+    /**
+     * Called from the new tab page when the user's signed in state changes.

[Dan Beam, 2012/03/23 21:12:47]

doc param

[Patrick Dubroy, 2012/03/26 15:16:46]

Done.

+     */
+    updateSignInState: function(signedIn) {
+      this.initialize(signedIn);
     },
   };