NTP Zombie Code Slayer III, The Saga Continues: Foreign Sessions

chrome/browser/resources/history/other_devices.js

 // Copyright (c) 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 The section of the history page that shows tabs from sessions
                  on other devices.
  */
 
 ///////////////////////////////////////////////////////////////////////////////
 // Globals:
 /** @const */ var MAX_NUM_COLUMNS = 3;
 /** @const */ var NB_ENTRIES_FIRST_ROW_COLUMN = 6;
 /** @const */ var NB_ENTRIES_OTHER_ROWS_COLUMN = 0;
 
 // Histogram buckets for UMA tracking of menu usage.
-// Using the same values as the Other Devices button in the NTP.
 /** @const */ var HISTOGRAM_EVENT = {
   INITIALIZED: 0,
   SHOW_MENU: 1,
   LINK_CLICKED: 2,
   LINK_RIGHT_CLICKED: 3,
   SESSION_NAME_RIGHT_CLICKED: 4,
   SHOW_SESSION_MENU: 5,
   COLLAPSE_SESSION: 6,
   EXPAND_SESSION: 7,
   OPEN_ALL: 8,
   LIMIT: 9  // Should always be the last one.
 };
 
 /**
  * Record an event in the UMA histogram.
  * @param {number} eventId The id of the event to be recorded.
  * @private
  */
 function recordUmaEvent_(eventId) {
   chrome.send('metricsHandler:recordInHistogram',
       ['HistoryPage.OtherDevicesMenu', eventId, HISTOGRAM_EVENT.LIMIT]);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // DeviceContextMenuController:
 
 /**
  * Controller for the context menu for device names in the list of sessions.
- * This class is designed to be used as a singleton. Also copied from existing
- * other devices button in NTP.
- * TODO(mad): Should we extract/reuse/share with ntp4/other_sessions.js?
- *
  * @constructor
  */
 function DeviceContextMenuController() {
   this.__proto__ = DeviceContextMenuController.prototype;
   this.initialize();
 }
 cr.addSingletonGetter(DeviceContextMenuController);
 
 // DeviceContextMenuController, Public: ---------------------------------------
 
@@ skipping 495 matching lines @@
 
 ///////////////////////////////////////////////////////////////////////////////
 // Document Functions:
 /**
  * Window onload handler, sets up the other devices view.
  */
 function load() {
   if (!loadTimeData.getBoolean('isInstantExtendedApiEnabled'))
     return;
 
-  // We must use this namespace to reuse the handler code for foreign session
-  // and login.
+  // TODO(dbeam): why is the ntp.* namespace being used from the history page?

[Evan Stade, 2015/05/18 18:28:59]

doesn't the old comment answer that question for you?

[Dan Beam, 2015/05/18 18:38:15]

i disagree with the original comment.  nothing about this code "must use this
namespace".  it just wasn't changed to be shared.

[Evan Stade, 2015/05/18 18:49:07]

It doesn't have to make sense to answer the question of why it's being done.

You should leave the comment and add a TODO under it:
// TODO(dbeam): change the call site in foobar.cc so this hack isn't necessary.

[Dan Beam, 2015/05/18 20:00:34]

Done.

   cr.define('ntp', function() {
     return {
-      setForeignSessions: setForeignSessions,
       updateLogin: updateLogin
     };
   });
 
   devicesView = new DevicesView();
 
   // Create the context menu that appears when the user right clicks
   // on a device name or hit click on the button besides the device name
   document.body.appendChild(DeviceContextMenuController.getInstance().menu);
 
   var doSearch = function(e) {
     devicesView.setSearchText($('search-field').value);
   };
   $('search-field').addEventListener('search', doSearch);
   $('search-button').addEventListener('click', doSearch);
 }
 
 // Add handlers to HTML elements.
 document.addEventListener('DOMContentLoaded', load);