Hotword: Make the new extension work with the NTP opt-in promo.

Hotword: Make the new extension work with the NTP opt-in promo.

BUG=436681
Committed: https://crrev.com/69cfa7bbd88391cef0ad02f4e95697c674404e77
Cr-Commit-Position: refs/heads/master@{#319844}

[kcarattini, 2015-03-09 05:21:50]

kcarattini@chromium.org changed reviewers:
+ amistry@chromium.org

[Anand Mistry (off Chromium), 2015-03-10 02:47:29]

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/constants.js (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/constants.js:145: * injected scripts and then
passed to the extension.
s/scripts/script while you're here

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/manifest.json (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/manifest.json:31:
"*://*.syd.corp.google.com/*",
You probably don't want to check this in, nor the same line below.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/page_audio_manager.js (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:113: if
(this.checkUrlPathIsEligible_(url,
'http://kcarattini-z620.syd.corp.google.com:8888/'))
Remove.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:429:
chrome.hotwordPrivate && chrome.hotwordPrivate.getStatus) {
Remove these checks. We already assume that chrome.hotwordPrivate exists.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:442: //
chrome.hotwordPrivate.getStatus(
Remove.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:508:
chrome.runtime.onMessage.addListener(this.messageListener_);
Isn't this one unnecessary? If so, it should be removed.

[kcarattini, 2015-03-10 04:34:05]

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/constants.js (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/constants.js:145: * injected scripts and then
passed to the extension.
On 2015/03/10 02:47:28, Anand Mistry wrote:
> s/scripts/script while you're here

Done.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/manifest.json (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/manifest.json:31:
"*://*.syd.corp.google.com/*",
On 2015/03/10 02:47:28, Anand Mistry wrote:
> You probably don't want to check this in, nor the same line below.

Done.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/page_audio_manager.js (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:113: if
(this.checkUrlPathIsEligible_(url,
'http://kcarattini-z620.syd.corp.google.com:8888/'))
On 2015/03/10 02:47:29, Anand Mistry wrote:
> Remove.

Done.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:429:
chrome.hotwordPrivate && chrome.hotwordPrivate.getStatus) {
On 2015/03/10 02:47:29, Anand Mistry wrote:
> Remove these checks. We already assume that chrome.hotwordPrivate exists.

Where do we make that assumption? I don't see it used anywhere else in this
file.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:442: //
chrome.hotwordPrivate.getStatus(
On 2015/03/10 02:47:29, Anand Mistry wrote:
> Remove.

Done.

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:508:
chrome.runtime.onMessage.addListener(this.messageListener_);
On 2015/03/10 02:47:29, Anand Mistry wrote:
> Isn't this one unnecessary? If so, it should be removed.

Done.

[Anand Mistry (off Chromium), 2015-03-10 04:40:54]

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/page_audio_manager.js (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:429:
chrome.hotwordPrivate && chrome.hotwordPrivate.getStatus) {
On 2015/03/10 04:34:05, kcarattini wrote:
> On 2015/03/10 02:47:29, Anand Mistry wrote:
> > Remove these checks. We already assume that chrome.hotwordPrivate exists.
> 
> Where do we make that assumption? I don't see it used anywhere else in this
> file.

Look at other files in this directory, such as state_manager.js. Also, the check
doesn't make sense since this code is chrome-only and never runs in any context
where hotwordPrivate is not available.

[kcarattini, 2015-03-10 04:50:58]

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
File chrome/browser/resources/hotword/page_audio_manager.js (right):

https://codereview.chromium.org/988263002/diff/1/chrome/browser/resources/hot...
chrome/browser/resources/hotword/page_audio_manager.js:429:
chrome.hotwordPrivate && chrome.hotwordPrivate.getStatus) {
On 2015/03/10 04:40:54, Anand Mistry wrote:
> On 2015/03/10 04:34:05, kcarattini wrote:
> > On 2015/03/10 02:47:29, Anand Mistry wrote:
> > > Remove these checks. We already assume that chrome.hotwordPrivate exists.
> > 
> > Where do we make that assumption? I don't see it used anywhere else in this
> > file.
> 
> Look at other files in this directory, such as state_manager.js. Also, the
check
> doesn't make sense since this code is chrome-only and never runs in any
context
> where hotwordPrivate is not available.

Done.

[Anand Mistry (off Chromium), 2015-03-10 05:58:57]

lgtm

[kcarattini, 2015-03-10 06:07:49]

The CQ bit was checked by kcarattini@chromium.org

[commit-bot: I haz the power, 2015-03-10 06:08:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/988263002/40001

[commit-bot: I haz the power, 2015-03-10 06:51:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-10 06:51:36]

Patchset 3 (id:??) landed as
https://crrev.com/69cfa7bbd88391cef0ad02f4e95697c674404e77
Cr-Commit-Position: refs/heads/master@{#319844}