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

chrome/browser/resources/ntp4/other_sessions.js

 // Copyright (c) 2012 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 The menu that shows tabs from sessions on other devices.
  */
 
 cr.define('ntp', function() {
   'use strict';
 
   var localStrings = new LocalStrings();
   var Menu = cr.ui.Menu;
   var MenuItem = cr.ui.MenuItem;
   var MenuButton = cr.ui.MenuButton;
   var OtherSessionsMenuButton = cr.ui.define('button');
 
   // Histogram buckets for UMA tracking of menu usage.
   var HISTOGRAM_EVENT = {
       INITIALIZED: 0,
       SHOW_MENU: 1,
       LINK_CLICKED: 2,
       LINK_RIGHT_CLICKED: 3
   };
   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,
 
     decorate: function() {
       MenuButton.prototype.decorate.call(this);
       this.menu = new Menu;
       cr.ui.decorate(this.menu, Menu);
       this.menu.classList.add('footer-menu');
       this.menu.addEventListener('contextmenu',
                                  this.onContextMenu_.bind(this), true);
       document.body.appendChild(this.menu);
 
       this.sessions_ = [];
       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');
+    },
+
+    /**
      * Record an event in the UMA histogram.
      * @param {Number} eventId The id of the event to be recorded.
      */
     recordUmaEvent_: function(eventId) {
       chrome.send('metricsHandler:recordInHistogram',
           ['NewTabPage.OtherSessionsMenu', eventId, HISTOGRAM_EVENT_LIMIT]);
     },
 
     /**
      * Handle a context menu event for an object in the menu's DOM subtree.
@@ skipping 58 matching lines @@
           var clickHandler = this.makeClickHandler_(
               session.tag, String(window.sessionId), String(tab.sessionId));
           a.addEventListener('click', clickHandler);
           section.appendChild(a);
         }
       }
     },
 
     /**
      * 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);
     },
 
     /**
      * Sets the menu model data.
      * @param {Array} sessionList Array of objects describing the sessions
      * from other devices.
      */
     set sessions(sessionList) {
-      // Clear the current contents of the menu.
-      this.menu.innerHTML = '';
+      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]);
+        // 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);
     },
   };
 
   return {
     OtherSessionsMenuButton: OtherSessionsMenuButton,
   };
 });