[Hotword] Add new base URLs for the additional languages so it works on their respective local sear…

chrome/browser/resources/hotword_helper/manager.js

Index: chrome/browser/resources/hotword_helper/manager.js
diff --git a/chrome/browser/resources/hotword_helper/manager.js b/chrome/browser/resources/hotword_helper/manager.js
index 3fcdb05653ff594c011a009a53b0208a89a87df7..b573f093ad51f7df65685b6cdfb734e4217fc2f4 100644
--- a/chrome/browser/resources/hotword_helper/manager.js
+++ b/chrome/browser/resources/hotword_helper/manager.js
@@ -124,6 +124,30 @@ OptInManager.prototype.handleMessage_ = function(
   return false;
 };
 
+/**
+ * Helper function to test urls as being valid for running the

[James Hawkins, 2014/05/12 18:13:43]

nit: URLs.

[rpetterson, 2014/05/12 21:09:05]

Done.

+ * hotwording extension. It's used by isEligibleUrl to make that
+ * function clearer.
+ * @param {string} url Url to check.
+ * @param {string} base Base url to compare against..
+ * @return {boolean} True if url is eligible hotword url.
+ */
+OptInManager.prototype.checkEligibleUrl = function(url, base) {
+  if (!url)
+    return false;
+
+  if (url === base ||
+      url === base + '/' ||
+      url.indexOf(base + '/_/chrome/newtab?') === 0 ||  // Appcache NTP.
+      url.indexOf(base + '/?') === 0 ||
+      url.indexOf(base + '/#') === 0 ||
+      url.indexOf(base + '/webhp') === 0 ||
+      url.indexOf(base + '/search') === 0) {
+    return true;
+  }
+  return false;
+
+};
 
 /**
  * Determines if a URL is eligible for hotwording. For now, the
@@ -136,20 +160,28 @@ OptInManager.prototype.isEligibleUrl = function(url) {
     return false;
 
   var baseUrls = [
-    'https://www.google.com',
-    'chrome://newtab',
-    'https://encrypted.google.com'
+    'chrome://newtab'
+  ];
+  var baseGoogleUrls = [
+    'https://www.google.',
+    'https://encrypted.google.'
   ];
+  var tlds = [
+    'com',
+    'co.uk',
+    'de',
+    'fr',
+    'ru'
+  ];
+
+  if (this.checkEligibleUrl(url, baseUrls[0]))

[James Hawkins, 2014/05/12 18:13:43]

Can you clarify why we need to check baseUrls[0] separately from the loop below?
 Or maybe document what that loop is doing.

[rpetterson, 2014/05/12 19:42:39]

chrome://newtab doesn't have the local tlds (.com, .de, .fr, etc.). I left it as
an array for when we add additional ones, but if you think it would be clearer,
I can just hardcode a check for
if (this.checkEligibleUrl(url, 'chrome://newtab')

Let me know what you think.

[James Hawkins, 2014/05/12 19:44:39]

No, that's fine.  Just add a comment to the code to clarify the intent.

[rpetterson, 2014/05/12 21:09:05]

Done.

+    return true;
 
-  for (var i = 0; i < baseUrls.length; i++) {
-    var base = baseUrls[i];
-    if (url === base + '/' ||
-        url.indexOf(base + '/_/chrome/newtab?') === 0 ||  // Appcache NTP.
-        url.indexOf(base + '/?') === 0 ||
-        url.indexOf(base + '/#') === 0 ||
-        url.indexOf(base + '/webhp') === 0 ||
-        url.indexOf(base + '/search') === 0) {
-      return true;
+  for (var i = 0; i < baseGoogleUrls.length; i++) {
+    for (var j = 0; j < tlds.length; j++) {
+      var base = baseGoogleUrls[i] + tlds[j];
+      if (this.checkEligibleUrl(url, base))
+        return true;
     }
   }
   return false;