Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(65)

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

Created:
6 years ago by rpetterson
Modified:
6 years 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 ago (2014-12-03 00:50:26 UTC) #2
benwells
The two files I was asked to look at lgtm
6 years 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 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 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 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 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 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 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 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 ago (2014-12-03 23:15:26 UTC) #13
commit-bot: I haz the power
Committed patchset #5 (id:80001)
6 years 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 ago (2014-12-04 00:27:56 UTC) #15
Anand Mistry (off Chromium)
6 years 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