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

Issue 134103005: [Hotword] Putting preferences under search for hotword service. Putting behind a flag. (Closed)

Created:
6 years, 11 months ago by rpetterson
Modified:
6 years, 11 months ago
Reviewers:
samarth, James Hawkins
CC:
chromium-reviews, dbeam+watch-options_chromium.org, skanuj+watch_chromium.org, melevin+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, dominich, jfweitz+watch_chromium.org, David Black, arv+watch_chromium.org, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

[Hotword] Putting preferences under search for hotword service. Putting behind a flag. In order to test/see the new settings run chrome with "--force-fieldtrials=VoiceTrigger/abc/ BUG=325439, 289023 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=246500

Patch Set 1 #

Patch Set 2 : add flag and cleanup #

Total comments: 10

Patch Set 3 : fixing comments and call order #

Total comments: 5

Patch Set 4 : fixing margin and color #

Total comments: 2

Patch Set 5 : convert hex to lowercase #

Patch Set 6 : rebase #

Patch Set 7 : remove extra line #

Patch Set 8 : fix error #

Patch Set 9 : trying rebase again #

Patch Set 10 : not sure why it's not patching. trying a different order. #

Patch Set 11 : trying another reorder for mysterious patching issues #

Patch Set 12 : trying again #

Patch Set 13 : rebasing one more time. #

Patch Set 14 : somehow lost pref_names.cc change #

Patch Set 15 : trying another rebase #

Patch Set 16 : trying . . . #

Patch Set 17 : adding a line. maybe it will help? #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -0 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +15 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/browser_options.css View 1 2 3 4 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/browser_options.html View 1 2 3 1 chunk +55 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/browser_options.js View 1 2 3 3 chunks +31 lines, -0 lines 0 comments Download
M chrome/browser/search/hotword_service_factory.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/browser_options_handler.h View 1 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/browser_options_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +25 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/settings_app_browsertest.js View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/url_constants.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/url_constants.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
rpetterson
samarth@chromium.org: Please review changes in search -- and general hotword consistency jhawkins@chromium.org: Please review changes ...
6 years, 11 months ago (2014-01-14 00:20:59 UTC) #1
James Hawkins
https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css File chrome/browser/resources/options/browser_options.css (right): https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css#newcode160 chrome/browser/resources/options/browser_options.css:160: margin-left: 22px; RTL https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.js File chrome/browser/resources/options/browser_options.js (right): https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.js#newcode131 chrome/browser/resources/options/browser_options.js:131: ...
6 years, 11 months ago (2014-01-14 00:53:49 UTC) #2
rpetterson
https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css File chrome/browser/resources/options/browser_options.css (right): https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css#newcode160 chrome/browser/resources/options/browser_options.css:160: margin-left: 22px; On 2014/01/14 00:53:50, James Hawkins wrote: > ...
6 years, 11 months ago (2014-01-14 21:54:17 UTC) #3
samarth
search lgtm https://codereview.chromium.org/134103005/diff/120001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/134103005/diff/120001/chrome/app/generated_resources.grd#newcode6912 chrome/app/generated_resources.grd:6912: + Enable "Ok Google" to start a ...
6 years, 11 months ago (2014-01-17 01:26:40 UTC) #4
James Hawkins
On 2014/01/17 01:26:40, samarth wrote: > search lgtm > > https://codereview.chromium.org/134103005/diff/120001/chrome/app/generated_resources.grd > File chrome/app/generated_resources.grd (right): ...
6 years, 11 months ago (2014-01-17 17:39:11 UTC) #5
James Hawkins
https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css File chrome/browser/resources/options/browser_options.css (right): https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css#newcode160 chrome/browser/resources/options/browser_options.css:160: margin-left: 22px; On 2014/01/14 21:54:17, rpetterson wrote: > On ...
6 years, 11 months ago (2014-01-17 17:39:18 UTC) #6
rpetterson
https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css File chrome/browser/resources/options/browser_options.css (right): https://codereview.chromium.org/134103005/diff/50001/chrome/browser/resources/options/browser_options.css#newcode160 chrome/browser/resources/options/browser_options.css:160: margin-left: 22px; On 2014/01/17 17:39:18, James Hawkins wrote: > ...
6 years, 11 months ago (2014-01-17 23:53:23 UTC) #7
James Hawkins
LGTM with nit. https://codereview.chromium.org/134103005/diff/200001/chrome/browser/resources/options/browser_options.css File chrome/browser/resources/options/browser_options.css (right): https://codereview.chromium.org/134103005/diff/200001/chrome/browser/resources/options/browser_options.css#newcode164 chrome/browser/resources/options/browser_options.css:164: color: #9B9B9B; nit: Lower-case letters in ...
6 years, 11 months ago (2014-01-21 18:18:02 UTC) #8
rpetterson
https://codereview.chromium.org/134103005/diff/200001/chrome/browser/resources/options/browser_options.css File chrome/browser/resources/options/browser_options.css (right): https://codereview.chromium.org/134103005/diff/200001/chrome/browser/resources/options/browser_options.css#newcode164 chrome/browser/resources/options/browser_options.css:164: color: #9B9B9B; On 2014/01/21 18:18:03, James Hawkins wrote: > ...
6 years, 11 months ago (2014-01-21 18:24:19 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rlp@chromium.org/134103005/260001
6 years, 11 months ago (2014-01-21 21:07:44 UTC) #10
commit-bot: I haz the power
Failed to apply patch for chrome/common/pref_names.h: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
6 years, 11 months ago (2014-01-21 21:07:50 UTC) #11
rpetterson
On 2014/01/21 21:07:50, I haz the power (commit-bot) wrote: > Failed to apply patch for ...
6 years, 11 months ago (2014-01-22 18:47:00 UTC) #12
James Hawkins
On 2014/01/22 18:47:00, rpetterson wrote: > On 2014/01/21 21:07:50, I haz the power (commit-bot) wrote: ...
6 years, 11 months ago (2014-01-22 18:48:34 UTC) #13
rpetterson
On 2014/01/22 18:48:34, James Hawkins wrote: > On 2014/01/22 18:47:00, rpetterson wrote: > > On ...
6 years, 11 months ago (2014-01-22 18:49:55 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rlp@chromium.org/134103005/820001
6 years, 11 months ago (2014-01-23 00:10:08 UTC) #15
commit-bot: I haz the power
6 years, 11 months ago (2014-01-23 03:06:25 UTC) #16
Message was sent while issue was closed.
Change committed as 246500

Powered by Google App Engine
This is Rietveld 408576698