app_list: Refactor mixer groups to avoid hard-coded groups in Mixer.

app_list: Refactor mixer groups to avoid hard-coded groups in Mixer.

Now, Mixer is not opinionated about what groups it has; it provides an
interface for its client to provide any number of arbitrary groups.
(Except that there is a special case for the omnibox group, which should
be fixed in the future.)

BUG=

Committed: https://crrev.com/36a4e7d48e672fc36cf235a7cd42b15da97dfa65
Cr-Commit-Position: refs/heads/master@{#324595}

[Matt Giuca, 2015-02-09 04:59:35]

mgiuca@chromium.org changed reviewers:
+ calamity@chromium.org

[Matt Giuca, 2015-02-09 04:59:35]

OK, so this refactor basically takes all logic of *specific* groups out of the
Mixer, *except* the omnibox, which is special currently.

I'm hoping to make Omnibox not special in the future, or at least, make this
specialness customizable by the client (SearchBoxController), but I think it
will be easier to think about after this refactor.

If you don't like it, I can play with the algorithm first and then refactor
later.

[Matt Giuca, 2015-02-12 03:10:00]

Patchset #3 (id:40001) has been deleted

[calamity, 2015-04-10 03:34:18]

https://codereview.chromium.org/882463004/diff/70001/ui/app_list/search/mixer.h
File ui/app_list/search/mixer.h (right):

https://codereview.chromium.org/882463004/diff/70001/ui/app_list/search/mixer...
ui/app_list/search/mixer.h:38: static const size_t OMNIBOX_GROUP;
Can we make an omnibox_group_ field that gets set by an AddOmniboxGroup method?

That would remove the need for DCHECKing against a runtime index we're deciding
on at compile time.

[Matt Giuca, 2015-04-10 03:49:19]

Good suggestion.

https://codereview.chromium.org/882463004/diff/70001/ui/app_list/search/mixer.h
File ui/app_list/search/mixer.h (right):

https://codereview.chromium.org/882463004/diff/70001/ui/app_list/search/mixer...
ui/app_list/search/mixer.h:38: static const size_t OMNIBOX_GROUP;
On 2015/04/10 03:34:18, calamity wrote:
> Can we make an omnibox_group_ field that gets set by an AddOmniboxGroup
method?
> 
> That would remove the need for DCHECKing against a runtime index we're
deciding
> on at compile time.

Done.

[calamity, 2015-04-10 05:28:05]

lgtm

https://codereview.chromium.org/882463004/diff/110001/ui/app_list/search/mixe...
File ui/app_list/search/mixer.cc (right):

https://codereview.chromium.org/882463004/diff/110001/ui/app_list/search/mixe...
ui/app_list/search/mixer.cc:119: : ui_results_(ui_results),
has_omnibox_group_(false) {
Initialize omnibox_group_.

In fact, you can do it in the class definition now!

[Matt Giuca, 2015-04-10 05:33:36]

In-class!!!

https://codereview.chromium.org/882463004/diff/110001/ui/app_list/search/mixe...
File ui/app_list/search/mixer.cc (right):

https://codereview.chromium.org/882463004/diff/110001/ui/app_list/search/mixe...
ui/app_list/search/mixer.cc:119: : ui_results_(ui_results),
has_omnibox_group_(false) {
On 2015/04/10 05:28:05, calamity wrote:
> Initialize omnibox_group_.
> 
> In fact, you can do it in the class definition now!

Done.

[Matt Giuca, 2015-04-10 05:33:43]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2015-04-10 05:33:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
calamity@chromium.org
Link to the patchset: https://codereview.chromium.org/882463004/#ps130001
(title: "Init omnibox_group_ and use in-class initialization.")

[commit-bot: I haz the power, 2015-04-10 05:34:30]

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

[commit-bot: I haz the power, 2015-04-10 06:27:55]

Committed patchset #7 (id:130001)

[commit-bot: I haz the power, 2015-04-10 06:28:47]

Patchset 7 (id:??) landed as
https://crrev.com/36a4e7d48e672fc36cf235a7cd42b15da97dfa65
Cr-Commit-Position: refs/heads/master@{#324595}