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

chrome/browser/resources/hotword_helper/manager.js

 // Copyright (c) 2014 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.
 
 'use strict';
 
 /**
  * @fileoverview This extension manages communications between Chrome,
  * Google.com pages and the Chrome Hotword extension.
  *
@@ skipping 106 matching lines @@
           chrome.hotwordPrivate.setAudioLoggingEnabled) {
         chrome.hotwordPrivate.setAudioLoggingEnabled(false);
       }
       break;
     default:
       break;
   }
   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
  * valid pages are the Google HP and SERP (this will include the NTP).
  * @param {string} url Url to check.
  * @return {boolean} True if url is eligible hotword url.
  */
 OptInManager.prototype.isEligibleUrl = function(url) {
   if (!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'
   ];
 
-  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;
+  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 < 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;
 };
 
 
 /**
  * Initializes the extension.
  */
 OptInManager.prototype.initialize = function() {
   // TODO(rlp): Possibly remove the next line. It's proably not used, but
   // leaving for now to be safe. We should remove it once all messsage
   // relaying is removed form the content scripts.
   chrome.runtime.onMessage.addListener(this.handleMessage_.bind(this));
   chrome.runtime.onMessageExternal.addListener(
       this.handleMessage_.bind(this));
 };
 
 
 new OptInManager().initialize();