Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(34)

Issue 774003002: [Hotword] Reverse the experimental flag so that it is on by default. (Closed)

Created:
6 years, 3 months ago by rpetterson
Modified:
6 years, 3 months ago
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, extensions-reviews_chromium.org, skanuj+watch_chromium.org, melevin+watch_chromium.org, tfarina, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, rlp+watch_chromium.org, jfweitz+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, chromium-apps-reviews_chromium.org, Jered
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

[Hotword] Reverse the experimental flag so that it is on by default. This means that v2 hotword (using the shared module instead of the CWS extension) will be the default. Updates tests to account for new default. The next CL will take care of cleaning up usage of IsExperimentalHotwordingEnabled(). BUG=432995 Committed: https://crrev.com/2cd75c128306f0975c904574750c12c6cf8ea367 Cr-Commit-Position: refs/heads/master@{#306730}

Patch Set 1 #

Patch Set 2 : fix spacing #

Total comments: 2

Patch Set 3 : fix comment #

Patch Set 4 : update histograms file #

Patch Set 5 : fix browser test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -22 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/hotword_private/hotword_private_apitest.cc View 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/extensions/hotword_browsertest.cc View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/search/hotword_service.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/search/hotword_service_unittest.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/app_list/speech_recognizer_browsertest.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 16 (3 generated)
rpetterson
amistry - overall benwells -- OWNERS for speech_recognizer_browsertest and hotword_private_apitest
6 years, 3 months ago (2014-12-03 00:50:26 UTC) #2
benwells
The two files I was asked to look at lgtm
6 years, 3 months ago (2014-12-03 03:55:17 UTC) #3
Anand Mistry (off Chromium)
LGTM with one change. Yay! This calls for cake. https://codereview.chromium.org/774003002/diff/20001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/774003002/diff/20001/chrome/common/chrome_switches.cc#newcode454 chrome/common/chrome_switches.cc:454: ...
6 years, 3 months ago (2014-12-03 04:04:53 UTC) #4
rpetterson
I agree that cake is in order :) This is the best I can do ...
6 years, 3 months ago (2014-12-03 06:54:39 UTC) #5
rpetterson
On 2014/12/03 06:54:39, rpetterson wrote: > I agree that cake is in order :) This ...
6 years, 3 months ago (2014-12-03 06:55:30 UTC) #6
rpetterson
mpearson@ -- please take a look at histograms.xml. Note: I did not remove the old ...
6 years, 3 months ago (2014-12-03 08:23:59 UTC) #8
rpetterson
Anand, PTAL. I had to update the HotwordBrowserTest as well. Once we go to stable, ...
6 years, 3 months ago (2014-12-03 19:08:54 UTC) #9
Mark P
histograms.xml lgtm On 2014/12/03 08:23:59, rpetterson wrote: > mpearson@ -- please take a look at ...
6 years, 3 months ago (2014-12-03 19:09:50 UTC) #10
Anand Mistry (off Chromium)
LGTM On 2014/12/03 19:08:54, rpetterson wrote: > Anand, PTAL. I had to update the HotwordBrowserTest ...
6 years, 3 months ago (2014-12-03 23:13:39 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/774003002/80001
6 years, 3 months ago (2014-12-03 23:15:26 UTC) #13
commit-bot: I haz the power
Committed patchset #5 (id:80001)
6 years, 3 months ago (2014-12-04 00:26:52 UTC) #14
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/2cd75c128306f0975c904574750c12c6cf8ea367 Cr-Commit-Position: refs/heads/master@{#306730}
6 years, 3 months ago (2014-12-04 00:27:56 UTC) #15
Anand Mistry (off Chromium)
6 years, 3 months ago (2014-12-04 05:14:19 UTC) #16
Message was sent while issue was closed.
On 2014/12/03 06:55:30, rpetterson wrote:
> On 2014/12/03 06:54:39, rpetterson wrote:
> > I agree that cake is in order :) This is the best I can do until you all
come
> > back to MTV ;)
> > 
> >      |------|
> >      |           |
> >   |---------|
> >   |                 |
> > |------------|
> > |                      |
> > |______________|
> > 
> >
>
https://codereview.chromium.org/774003002/diff/20001/chrome/common/chrome_swi...
> > File chrome/common/chrome_switches.cc (right):
> > 
> >
>
https://codereview.chromium.org/774003002/diff/20001/chrome/common/chrome_swi...
> > chrome/common/chrome_switches.cc:454: // Enables v2 hotword detection
> features.
> > These features include
> > On 2014/12/03 04:04:53, Anand Mistry wrote:
> > > s/Enables/Disables
> > 
> > Done.
> 
> And apparently codereview did not like my cake :-/

Maybe it likes this one:
https://drive.google.com/a/google.com/file/d/0BxPuWSeoHD_uTlhaWjNSOGJPQVFnLXN...

Powered by Google App Engine
This is Rietveld 408576698