Cleanup: Change LogMostVisitedImpression|Navigation APIs to take an enum

chrome/browser/resources/local_ntp/most_visited_util.js

Index: chrome/browser/resources/local_ntp/most_visited_util.js
diff --git a/chrome/browser/resources/local_ntp/most_visited_util.js b/chrome/browser/resources/local_ntp/most_visited_util.js
index 60e30cf7b8f832a8e26a5632caeb797da2adf522..1807ae48d0d6b4537aa17a6aa918a3beb2dae888 100644
--- a/chrome/browser/resources/local_ntp/most_visited_util.js
+++ b/chrome/browser/resources/local_ntp/most_visited_util.js
@@ -47,18 +47,15 @@ var NTP_LOGGING_EVENT_TYPE = {
 };
 
 /**
- * Type of the impression provider for a generic client-provided suggestion.
- * @type {string}
- * @const
- */
-var CLIENT_PROVIDER_NAME = 'client';
-
-/**
- * Type of the impression provider for a generic server-provided suggestion.
- * @type {string}
+ * The different sources that an NTP tile can have.
+ * Note: Keep in sync with common/ntp_logging_events.h
+ * @enum {number}
  * @const
  */
-var SERVER_PROVIDER_NAME = 'server';
+var NTP_LOGGING_TILE_SOURCE = {

[Bernhard Bauer, 2016/07/05 14:31:54]

Why do we duplicate this again?

[Marc Treib, 2016/07/05 14:37:53]

most_visited_util.js is used by the old, multi-iframe NTP.
most_visited_single.js is the new, single-iframe version, which has copies of
this stuff rather than some common include. I don't know why - I guess the
original plan might have been to delete the old stuff at some point? Anyway,
that's not gonna happen, so all this should be moved to a common file.
Do you want that to happen in the same CL?

[Bernhard Bauer, 2016/07/05 14:43:42]

Up to you. A cleanup CL is not the worst place to make that change, but it could
also be a separate CL.

[Marc Treib, 2016/07/05 14:46:42]

Since that would be a fairly big and orthogonal change, and this CL is already
large enough, I'll go for followup CL. :)

[Bernhard Bauer, 2016/07/05 14:59:42]

Add a TODO then? :)

[Marc Treib, 2016/07/05 15:10:16]

Done.

+  NTP_TILE_SOURCE_CLIENT: 0,
+  NTP_TILE_SOURCE_SERVER: 1,
+};
 
 /**
  * The origin of this request.
@@ -98,12 +95,11 @@ function parseQueryParams(location) {
  * @param {string} title The title for the link.
  * @param {string|undefined} text The text for the link or none.
  * @param {string|undefined} direction The text direction.
- * @param {string|undefined} provider A provider name (max 8 alphanumeric
- *     characters) used for logging. Undefined if suggestion is not coming from
- *     the server.
+ * @param {number} tileSource The source from TILE_SOURCE.
  * @return {HTMLAnchorElement} A new link element.
  */
-function createMostVisitedLink(params, href, title, text, direction, provider) {
+function createMostVisitedLink(
+    params, href, title, text, direction, tileSource) {
   var styles = getMostVisitedStyles(params, !!text);
   var link = document.createElement('a');
   link.style.color = styles.color;
@@ -158,7 +154,7 @@ function createMostVisitedLink(params, href, title, text, direction, provider) {
     var ntpApiHandle = chrome.embeddedSearch.newTabPage;
     if ('pos' in params && isFinite(params.pos)) {
       ntpApiHandle.logMostVisitedNavigation(parseInt(params.pos, 10),
-                                            provider || '');
+                                            tileSource);
     }
 
     // Follow <a> normally, so transition type will be LINK.
@@ -277,23 +273,21 @@ function fillMostVisited(location, fill) {
       title: params.ti || '',
       direction: params.di || '',
       domain: params.dom || '',
-      provider: params.pr || SERVER_PROVIDER_NAME
+      tileSource: NTP_LOGGING_TILE_SOURCE.NTP_TILE_SOURCE_SERVER
     };
   } else {
     var apiHandle = chrome.embeddedSearch.searchBox;
     data = apiHandle.getMostVisitedItemData(params.rid);
     if (!data)
       return;
-    // Allow server-side provider override.
-    data.provider = params.pr || CLIENT_PROVIDER_NAME;
+    data.tileSource: NTP_LOGGING_TILE_SOURCE.NTP_TILE_SOURCE_CLIENT;
   }
 
   if (isFinite(params.dummy) && parseInt(params.dummy, 10)) {
     data.dummy = true;
   }
   if (/^javascript:/i.test(data.url) ||
-      /^javascript:/i.test(data.thumbnailUrl) ||
-      !/^[a-z0-9]{0,8}$/i.test(data.provider))
+      /^javascript:/i.test(data.thumbnailUrl))
     return;
   if (data.direction)
     document.body.dir = data.direction;