[Local NTP] Cleanup: Remove a bunch of dead code

chrome/browser/resources/local_ntp/local_ntp.js

 // Copyright 2015 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 local InstantExtended NTP.
  */
 
 
@@ skipping 75 matching lines @@
 /**
  * Enum for classnames.
  * @enum {string}
  * @const
  */
 var CLASSES = {
   ALTERNATE_LOGO: 'alternate-logo', // Shows white logo if required by theme
   DARK: 'dark',
   DEFAULT_THEME: 'default-theme',
   DELAYED_HIDE_NOTIFICATION: 'mv-notice-delayed-hide',
-  FAKEBOX_DISABLE: 'fakebox-disable', // Makes fakebox non-interactive
   FAKEBOX_FOCUS: 'fakebox-focused', // Applies focus styles to the fakebox
   // Applies drag focus style to the fakebox
   FAKEBOX_DRAG_FOCUS: 'fakebox-drag-focused',
   HIDE_FAKEBOX_AND_LOGO: 'hide-fakebox-logo',
   HIDE_NOTIFICATION: 'mv-notice-hide',
   LEFT_ALIGN_ATTRIBUTION: 'left-align-attribution',
   // Vertically centers the most visited section for a non-Google provided page.
   NON_GOOGLE_PAGE: 'non-google-page',
   RTL: 'rtl'  // Right-to-left language text.
 };
@@ skipping 26 matching lines @@
  * Enum for keycodes.
  * @enum {number}
  * @const
  */
 var KEYCODE = {
   ENTER: 13
 };
 
 
 /**
- * Enum for the state of the NTP when it is disposed.
- * @enum {number}
- * @const
- */
-var NTP_DISPOSE_STATE = {
-  NONE: 0,  // Preserve the NTP appearance and functionality
-  DISABLE_FAKEBOX: 1,
-  HIDE_FAKEBOX_AND_LOGO: 2
-};
-
-
-/**
  * The notification displayed when a page is blacklisted.
  * @type {Element}
  */
 var notification;
 
 
 /**
  * The container for the theme attribution.
  * @type {Element}
  */
@@ skipping 38 matching lines @@
 var ntpApiHandle;
 
 
 /**
  * The browser embeddedSearch.searchBox object.
  * @type {Object}
  */
 var searchboxApiHandle;
 
 
-/**
- * The state of the NTP when a query is entered into the Omnibox.
- * @type {NTP_DISPOSE_STATE}
- */
-var omniboxInputBehavior = NTP_DISPOSE_STATE.NONE;
-
-
-/**
- * The state of the NTP when a query is entered into the Fakebox.
- * @type {NTP_DISPOSE_STATE}
- */
-var fakeboxInputBehavior = NTP_DISPOSE_STATE.HIDE_FAKEBOX_AND_LOGO;

[Marc Treib, 2017/02/03 09:16:58]

These two were effectively constants. I just inlined the values below, which
exposed some more simplification.

-
-
 /** @type {number} @const */
 var MAX_NUM_TILES_TO_SHOW = 8;
 
 
 /** @type {number} @const */
 var MIN_NUM_COLUMNS = 2;
 
 
 /** @type {number} @const */
 var MAX_NUM_COLUMNS = 4;
@@ skipping 88 matching lines @@
   renderTheme();
 }
 
 
 /**
  * Updates the NTP style according to theme.
  * @param {Object=} opt_themeInfo The information about the theme. If it is
  * omitted the style will be reverted to the default.
  * @private
  */
+// TODO(treib): We actually never call this without a themeInfo. Should we?
 function setCustomThemeStyle(opt_themeInfo) {
   var customStyleElement = $(IDS.CUSTOM_THEME_STYLE);
   var head = document.head;
   if (opt_themeInfo && !opt_themeInfo.usingDefaultTheme) {
     ntpContents.classList.remove(CLASSES.DEFAULT_THEME);
     var themeStyle =
       '#attribution {' +
       '  color: ' + convertToRGBAColor(opt_themeInfo.textColorLightRgba) + ';' +
       '}' +
       '#mv-msg {' +
@@ skipping 67 matching lines @@
  * @private
  */
 function setAttributionVisibility_(show) {
   if (attribution) {
     attribution.style.display = show ? '' : 'none';
   }
 }
 
 
  /**
- * Converts an Array of color components into RRGGBBAA format.
- * @param {Array<number>} color Array of rgba color components.
- * @return {string} Color string in RRGGBBAA format.
- * @private
- */
-function convertToRRGGBBAAColor(color) {

[Marc Treib, 2017/02/03 09:16:59]

Never called

-  return color.map(function(t) {
-    return ('0' + t.toString(16)).slice(-2);  // To 2-digit, 0-padded hex.
-  }).join('');
-}
-
-
- /**
  * Converts an Array of color components into RGBA format "rgba(R,G,B,A)".
  * @param {Array<number>} color Array of rgba color components.
  * @return {string} CSS color in RGBA format.
  * @private
  */
 function convertToRGBAColor(color) {
   return 'rgba(' + color[0] + ',' + color[1] + ',' + color[2] + ',' +
                     color[3] / 255 + ')';
 }
 
@@ skipping 114 matching lines @@
 
 
 /**
  * Handles new input by disposing the NTP, according to where the input was
  * entered.
  */
 function onInputStart() {
   if (fakebox && isFakeboxFocused()) {
     setFakeboxFocus(false);
     setFakeboxDragFocus(false);
-    disposeNtp(true);
-  } else if (!isFakeboxFocused()) {
-    disposeNtp(false);

[Marc Treib, 2017/02/03 09:16:59]

disposeNtp(false) didn't do anything, see below

+    setFakeboxAndLogoVisibility(false);
   }
 }
 
 
 /**
- * Disposes the NTP, according to where the input was entered.
- * @param {boolean} wasFakeboxInput True if the input was in the fakebox.
- */
-function disposeNtp(wasFakeboxInput) {
-  var behavior = wasFakeboxInput ? fakeboxInputBehavior : omniboxInputBehavior;
-  if (behavior == NTP_DISPOSE_STATE.DISABLE_FAKEBOX)

[Marc Treib, 2017/02/03 09:16:58]

This could never happen (DISABLE_FAKEBOX is unused).

-    setFakeboxActive(false);
-  else if (behavior == NTP_DISPOSE_STATE.HIDE_FAKEBOX_AND_LOGO)
-    setFakeboxAndLogoVisibility(false);

[Marc Treib, 2017/02/03 09:16:59]

This was inlined in onInputStart

-}
-
-
-/**
  * Restores the NTP (re-enables the fakebox and unhides the logo.)
  */
-function restoreNtp() {
-  setFakeboxActive(true);

[Marc Treib, 2017/02/03 09:16:59]

Pointless, because it's never called with false

+function onInputCancel() {
   setFakeboxAndLogoVisibility(true);
 }
 
 
 /**
  * @param {boolean} focus True to focus the fakebox.
  */
 function setFakeboxFocus(focus) {
   document.body.classList.toggle(CLASSES.FAKEBOX_FOCUS, focus);
 }
 
 /**
  * @param {boolean} focus True to show a dragging focus to the fakebox.
  */
 function setFakeboxDragFocus(focus) {
   document.body.classList.toggle(CLASSES.FAKEBOX_DRAG_FOCUS, focus);
 }
 
 /**
  * @return {boolean} True if the fakebox has focus.
  */
 function isFakeboxFocused() {
   return document.body.classList.contains(CLASSES.FAKEBOX_FOCUS) ||
       document.body.classList.contains(CLASSES.FAKEBOX_DRAG_FOCUS);
 }
 
 
 /**
- * @param {boolean} enable True to enable the fakebox.
- */
-function setFakeboxActive(enable) {
-  document.body.classList.toggle(CLASSES.FAKEBOX_DISABLE, !enable);
-}
-
-
-/**
  * @param {!Event} event The click event.
  * @return {boolean} True if the click occurred in an enabled fakebox.
  */
 function isFakeboxClick(event) {
-  return fakebox.contains(event.target) &&
-      !document.body.classList.contains(CLASSES.FAKEBOX_DISABLE);

[Marc Treib, 2017/02/03 09:16:58]

This class is never set

+  return fakebox.contains(event.target);
 }
 
 
 /**
  * @param {boolean} show True to show the fakebox and logo.
  */
 function setFakeboxAndLogoVisibility(show) {
   document.body.classList.toggle(CLASSES.HIDE_FAKEBOX_AND_LOGO, !show);
 }
 
 
 /**
- * Shortcut for document.getElementById.
- * @param {string} id of the element.
- * @return {HTMLElement} with the id.
- */
-function $(id) {

[Marc Treib, 2017/02/03 09:16:58]

There's an exact duplicate above

-  return document.getElementById(id);
-}
-
-
-/**
- * Utility function which creates an element with an optional classname and
- * appends it to the specified parent.
- * @param {Element} parent The parent to append the new element.
- * @param {string} name The name of the new element.
- * @param {string=} opt_class The optional classname of the new element.
- * @return {Element} The new element.
- */
-function createAndAppendElement(parent, name, opt_class) {

[Marc Treib, 2017/02/03 09:16:58]

There was only one caller, which turned out to be completely pointless

-  var child = document.createElement(name);
-  if (opt_class)
-    child.classList.add(opt_class);
-  parent.appendChild(child);
-  return child;
-}
-
-
-/**
  * @param {!Element} element The element to register the handler for.
  * @param {number} keycode The keycode of the key to register.
  * @param {!Function} handler The key handler to register.
  */
 function registerKeyHandler(element, keycode, handler) {
   element.addEventListener('keydown', function(event) {
     if (event.keyCode == keycode)
       handler(event);
   });
 }
@@ skipping 24 matching lines @@
     lastBlacklistedTile = args.tid;
 
     ntpApiHandle.deleteMostVisitedItem(args.tid);
   }
   // TODO(treib): Should we also handle the 'loaded' message from the iframe
   // here? We could hide the page until it arrives, to avoid flicker.
 }
 
 
 /**
- * Prepares the New Tab Page by adding listeners, rendering the current
- * theme, the most visited pages section, and Google-specific elements for a
- * Google-provided page.
+ * Prepares the New Tab Page by adding listeners, the most visited pages
+ * section, and Google-specific elements for a Google-provided page.
  */
 function init() {
   notification = $(IDS.NOTIFICATION);
   attribution = $(IDS.ATTRIBUTION);
   ntpContents = $(IDS.NTP_CONTENTS);
 
   if (configData.isGooglePage) {
     var logo = document.createElement('div');
     logo.id = IDS.LOGO;
     logo.title = 'Google';
@@ skipping 20 matching lines @@
   notificationMessage.textContent =
       configData.translatedStrings.thumbnailRemovedNotification;
 
   var undoLink = $(IDS.UNDO_LINK);
   undoLink.addEventListener('click', onUndo);
   registerKeyHandler(undoLink, KEYCODE.ENTER, onUndo);
   undoLink.textContent = configData.translatedStrings.undoThumbnailRemove;
 
   var restoreAllLink = $(IDS.RESTORE_ALL_LINK);
   restoreAllLink.addEventListener('click', onRestoreAll);
-  registerKeyHandler(restoreAllLink, KEYCODE.ENTER, onUndo);
+  registerKeyHandler(restoreAllLink, KEYCODE.ENTER, onRestoreAll);
   restoreAllLink.textContent =
       configData.translatedStrings.restoreThumbnailsShort;
 
   $(IDS.ATTRIBUTION_TEXT).textContent =
       configData.translatedStrings.attributionIntro;
 
-  var notificationCloseButton = $(IDS.NOTIFICATION_CLOSE_BUTTON);
-  createAndAppendElement(
-      notificationCloseButton, 'div', CLASSES.BLACKLIST_BUTTON_INNER);

[Marc Treib, 2017/02/03 09:16:59]

No idea what this was supposed to do. The class doesn't even exist, so all this
did was append an empty div.

-  notificationCloseButton.addEventListener('click', hideNotification);
+  $(IDS.NOTIFICATION_CLOSE_BUTTON).addEventListener('click', hideNotification);
 
   window.addEventListener('resize', onResize);
   updateContentWidth();
 
   var topLevelHandle = getEmbeddedSearchApiHandle();
 
   ntpApiHandle = topLevelHandle.newTabPage;
   ntpApiHandle.onthemechange = onThemeChange;
   ntpApiHandle.onmostvisitedchange = onMostVisitedChange;
 
   ntpApiHandle.oninputstart = onInputStart;
-  ntpApiHandle.oninputcancel = restoreNtp;
+  ntpApiHandle.oninputcancel = onInputCancel;
 
   if (ntpApiHandle.isInputInProgress)
     onInputStart();
 
   searchboxApiHandle = topLevelHandle.searchBox;
 
   if (fakebox) {
     // Listener for updating the key capture state.
     document.body.onmousedown = function(event) {
       if (isFakeboxClick(event))
@@ skipping 77 matching lines @@
 
 return {
   init: init,
   listen: listen
 };
 }
 
 if (!window.localNTPUnitTest) {
   LocalNTP().listen();
 }