Prepare for building with enable_app_list=0 on Desktop

Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms. The code will be required as soon as enable_app_list is
flipped to 0. E.g. calls to --show-app-list will just show chrome://apps
instead.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com

Committed: https://crrev.com/d13e0cdf6fc5fddb580968adb2a5b9c5379a4404
Cr-Commit-Position: refs/heads/master@{#393474}

[tapted, 2016-05-02 00:03:02]

Patchset #1 (id:1) has been deleted

[tapted, 2016-05-02 00:03:14]

Patchset #1 (id:20001) has been deleted

[tapted, 2016-05-02 00:03:49]

Patchset #3 (id:80001) has been deleted

[tapted, 2016-05-02 00:04:00]

Patchset #3 (id:100001) has been deleted

[tapted, 2016-05-02 00:04:24]

Patchset #4 (id:140001) has been deleted

[tapted, 2016-05-02 00:04:41]

Patchset #5 (id:180001) has been deleted

[tapted, 2016-05-02 00:04:54]

Patchset #5 (id:200001) has been deleted

[tapted, 2016-05-02 02:53:22]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

BUG=600915
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables. Only "real" change should be on Mac - the new
`chrome_browser_ui_app_list_views_sources`gyp variable already isn't
needed on Mac, so don't bother adding it.

BUG=600915
==========

[tapted, 2016-05-02 02:53:22]

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

[tapted, 2016-05-02 02:57:34]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables. Only "real" change should be on Mac - the new
`chrome_browser_ui_app_list_views_sources`gyp variable already isn't
needed on Mac, so don't bother adding it.

BUG=600915
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables. Only "real" change should be on Mac - the new
`chrome_browser_ui_app_list_views_sources` gyp variable already isn't
needed on Mac, so don't bother adding it. (The files should already be
dead-code stripped by the linker)

BUG=600915
==========

[tapted, 2016-05-02 02:58:27]

Hiya Matt, please take a look

[Matt Giuca, 2016-05-03 07:31:16]

Some minor Qs.

Also, this seems to be at least in part derived from
https://codereview.chromium.org/1747773002/ (is that correct, or did you redo it
from scratch)? Particularly refactoring AppInfoDialog. Either way, you should
credit wierichs@ in the CL description for starting the work on this.

https://codereview.chromium.org/1861233003/diff/280001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1861233003/diff/280001/build/common.gypi#newc...
build/common.gypi:262: ['OS!="ios" and OS!="android" and chromecast==0', {
Delightful!

Can we just change this to a positive set of platforms?

['chromeos==1 or OS=="win" or OS=="linux" or OS=="mac"' ..

(in fact the same condition as the flag immediately above.) Then later we can
remove the non-chromeos ones.

https://codereview.chromium.org/1861233003/diff/280001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1861233003/diff/280001/build/config/features....
build/config/features.gni:60: enable_app_list = !is_ios && !is_android &&
!is_chromecast
Same here as in GYP.

https://codereview.chromium.org/1861233003/diff/280001/chrome/browser/ui/BUIL...
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/1861233003/diff/280001/chrome/browser/ui/BUIL...
chrome/browser/ui/BUILD.gn:519: if (is_desktop_linux) {
Can you explain what this is doing?

https://codereview.chromium.org/1861233003/diff/280001/ui/app_list/app_list_s...
File ui/app_list/app_list_switches.cc (right):

https://codereview.chromium.org/1861233003/diff/280001/ui/app_list/app_list_s...
ui/app_list/app_list_switches.cc:58: return false;
Removing the enable-mac-views-app-list switch is a nice piece of functionality
that could be split out of this CL into a small CL by itself.

I don't feel strongly about doing that but if you want to you could.

[tapted, 2016-05-04 05:48:23]

Patchset #11 (id:340001) has been deleted

[tapted, 2016-05-04 06:00:57]

Patchset #11 (id:360001) has been deleted

[tapted, 2016-05-04 06:51:23]

Patchset #11 (id:380001) has been deleted

[tapted, 2016-05-04 09:54:42]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-04 09:54:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1861233003/400001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1861233003/400001

[tapted, 2016-05-04 09:55:43]

https://codereview.chromium.org/1861233003/diff/280001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1861233003/diff/280001/build/common.gypi#newc...
build/common.gypi:262: ['OS!="ios" and OS!="android" and chromecast==0', {
On 2016/05/03 07:31:15, Matt Giuca wrote:
> Delightful!
> 
> Can we just change this to a positive set of platforms?
> 
> ['chromeos==1 or OS=="win" or OS=="linux" or OS=="mac"' ..
> 
> (in fact the same condition as the flag immediately above.) Then later we can
> remove the non-chromeos ones.

That becomes tricky since chromecast \implies{} OS="linux". I've updated
all.gyp, gn_migration.gyp, and src/BUILD.gn to carry more of the conditions that
the targets previously had so that this isn't needed. Once it's chromeos-only we
will need to do a simplification pass, but there were TODOs for that already.

https://codereview.chromium.org/1861233003/diff/280001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1861233003/diff/280001/build/config/features....
build/config/features.gni:60: enable_app_list = !is_ios && !is_android &&
!is_chromecast
On 2016/05/03 07:31:16, Matt Giuca wrote:
> Same here as in GYP.

Done.

https://codereview.chromium.org/1861233003/diff/280001/chrome/browser/ui/BUIL...
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/1861233003/diff/280001/chrome/browser/ui/BUIL...
chrome/browser/ui/BUILD.gn:519: if (is_desktop_linux) {
On 2016/05/03 07:31:16, Matt Giuca wrote:
> Can you explain what this is doing?

This is adding the app_list_linux_sources - i.e. those 4 files from
chrome_browser_ui.gypi for desktop linux but not ChromeOS --
app_list_{service_,}linux.{cc,h} -- which were previously in 

# Desktop Linux. Assume aura/ash/views/x11.
'chrome_browser_ui_desktop_linux_sources': [
  /* here */


I think this fits the common pattern, but holler if you think it needs a comment
in the .gn file

https://codereview.chromium.org/1861233003/diff/280001/ui/app_list/app_list_s...
File ui/app_list/app_list_switches.cc (right):

https://codereview.chromium.org/1861233003/diff/280001/ui/app_list/app_list_s...
ui/app_list/app_list_switches.cc:58: return false;
On 2016/05/03 07:31:16, Matt Giuca wrote:
> Removing the enable-mac-views-app-list switch is a nice piece of functionality
> that could be split out of this CL into a small CL by itself.
> 
> I don't feel strongly about doing that but if you want to you could.

OK you twisted my arm :p

[commit-bot: I haz the power, 2016-05-04 11:34:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-04 11:34:55]

Dry run: This issue passed the CQ dry run.

[Matt Giuca, 2016-05-05 00:26:30]

Please respond:

> Also, this seems to be at least in part derived from
> https://codereview.chromium.org/1747773002/ (is that correct, or did you redo
it
> from scratch)? Particularly refactoring AppInfoDialog. Either way, you should
> credit wierichs@ in the CL description for starting the work on this.

https://codereview.chromium.org/1861233003/diff/280001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1861233003/diff/280001/build/common.gypi#newc...
build/common.gypi:262: ['OS!="ios" and OS!="android" and chromecast==0', {
On 2016/05/04 09:55:43, tapted wrote:
> On 2016/05/03 07:31:15, Matt Giuca wrote:
> > Delightful!
> > 
> > Can we just change this to a positive set of platforms?
> > 
> > ['chromeos==1 or OS=="win" or OS=="linux" or OS=="mac"' ..
> > 
> > (in fact the same condition as the flag immediately above.) Then later we
can
> > remove the non-chromeos ones.
> 
> That becomes tricky since chromecast \implies{} OS="linux". I've updated
> all.gyp, gn_migration.gyp, and src/BUILD.gn to carry more of the conditions
that
> the targets previously had so that this isn't needed. Once it's chromeos-only
we
> will need to do a simplification pass, but there were TODOs for that already.

Acknowledged.

https://codereview.chromium.org/1861233003/diff/280001/chrome/browser/ui/BUIL...
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/1861233003/diff/280001/chrome/browser/ui/BUIL...
chrome/browser/ui/BUILD.gn:519: if (is_desktop_linux) {
On 2016/05/04 09:55:43, tapted wrote:
> On 2016/05/03 07:31:16, Matt Giuca wrote:
> > Can you explain what this is doing?
> 
> This is adding the app_list_linux_sources - i.e. those 4 files from
> chrome_browser_ui.gypi for desktop linux but not ChromeOS --
> app_list_{service_,}linux.{cc,h} -- which were previously in 
> 
> # Desktop Linux. Assume aura/ash/views/x11.
> 'chrome_browser_ui_desktop_linux_sources': [
>   /* here */
> 
> 
> I think this fits the common pattern, but holler if you think it needs a
comment
> in the .gn file

Acknowledged.

https://codereview.chromium.org/1861233003/diff/280001/ui/app_list/app_list_s...
File ui/app_list/app_list_switches.cc (right):

https://codereview.chromium.org/1861233003/diff/280001/ui/app_list/app_list_s...
ui/app_list/app_list_switches.cc:58: return false;
On 2016/05/04 09:55:43, tapted wrote:
> On 2016/05/03 07:31:16, Matt Giuca wrote:
> > Removing the enable-mac-views-app-list switch is a nice piece of
functionality
> > that could be split out of this CL into a small CL by itself.
> > 
> > I don't feel strongly about doing that but if you want to you could.
> 
> OK you twisted my arm :p

Ever so slightly...

[tapted, 2016-05-05 00:28:49]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables. Only "real" change should be on Mac - the new
`chrome_browser_ui_app_list_views_sources` gyp variable already isn't
needed on Mac, so don't bother adding it. (The files should already be
dead-code stripped by the linker)

BUG=600915
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables. Only "real" change should be on Mac - the new
`chrome_browser_ui_app_list_views_sources` gyp variable already isn't
needed on Mac, so don't bother adding it. (The files should already be
dead-code stripped by the linker)

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

[tapted, 2016-05-05 00:30:11]

On 2016/05/05 00:26:30, Matt Giuca wrote:
> Please respond:
> 
> > Also, this seems to be at least in part derived from
> > https://codereview.chromium.org/1747773002/ (is that correct, or did you
redo
> it
> > from scratch)? Particularly refactoring AppInfoDialog. Either way, you
should
> > credit wierichs@ in the CL description for starting the work on this.

Whoops meant to do this. Updated the CL description.

The answer to your question is "both" :). Patchset 1 started before I knew about
it, then Patchset 2 converged some of the ideas.

[Matt Giuca, 2016-05-05 00:32:21]

lgtm

[tapted, 2016-05-05 04:01:37]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables. Only "real" change should be on Mac - the new
`chrome_browser_ui_app_list_views_sources` gyp variable already isn't
needed on Mac, so don't bother adding it. (The files should already be
dead-code stripped by the linker)

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test the code needed on previously supported
platforms that will be used once enable_app_list is flipped to 0.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

[tapted, 2016-05-05 04:02:23]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test the code needed on previously supported
platforms that will be used once enable_app_list is flipped to 0.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms, which will be required as soon as enable_app_list is flipped
to 0.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

[tapted, 2016-05-05 04:03:53]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms, which will be required as soon as enable_app_list is flipped
to 0.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms. The code will be required as soon as enable_app_list is
flipped to 0. E.g. calls to --show-app-list will just show chrome://apps
instead.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

[tapted, 2016-05-05 07:16:16]

tapted@chromium.org changed reviewers:
+ dpranke@chromium.org, rdevlin.cronin@chromium.org, sky@chromium.org

[tapted, 2016-05-05 07:17:29]

+dpranke, rdevlin, sky for OWNERS

dpranke: src/BUILD.gn and src/build/
sky: chrome/browser/ui/{BUILD.gn,startup/} and chrome/browser/prefs/
rdevlin.cronin: chrome/common/extensions

Thanks!

[Devlin, 2016-05-05 16:01:36]

my gyp/gn fu isn't too strong, so these might be silly questions.

https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn#newcode270
BUILD.gn:270: if (enable_app_list && !is_chromecast) {
What's the link between app list and chromecast?

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/apps...
File chrome/browser/ui/apps/app_info_dialog.h (right):

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/apps...
chrome/browser/ui/apps/app_info_dialog.h:26: // Used for UMA to track where the
App Info dialog is launched from.
DANGER!  These enums are used for UMA.  #if defing one out is bad.  Rename it,
or add a comment.

https://codereview.chromium.org/1861233003/diff/420001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1861233003/diff/420001/chrome/chrome_tests.gy...
chrome/chrome_tests.gypi:2655: # This assumes the AppList is views-based.
I'm confused - shouldn't this be in a toolkit views condition rather than an OS
mac condition then?

https://codereview.chromium.org/1861233003/diff/420001/chrome/chrome_tests.gy...
chrome/chrome_tests.gypi:2656:
'browser/ui/app_list/app_list_service_views_browsertest.cc',
Also, if this browsertest depends on views, shouldn't it be in ui/views?

[Dirk Pranke, 2016-05-05 17:13:25]

dpranke@chromium.org changed reviewers:
+ slan@chromium.org

[Dirk Pranke, 2016-05-05 17:13:26]

lgtm w/ the possible option to fix the chromecast stuff.

+slan for that ...

https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn#newcode270
BUILD.gn:270: if (enable_app_list && !is_chromecast) {
On 2016/05/05 16:01:36, Devlin wrote:
> What's the link between app list and chromecast?

It seems unlikely to me that cast actually wants or uses app list, so maybe we
should update the default in build/config/features.gni .

[sky, 2016-05-05 17:24:38]

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/star...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/star...
chrome/browser/ui/startup/startup_browser_creator.cc:53: #include
"chrome/browser/ui/app_list/app_list_service.h"
Only include if ENABLE_APP_LIST?

[slan, 2016-05-05 18:04:10]

On 2016/05/05 17:13:26, Dirk Pranke wrote:
> lgtm w/ the possible option to fix the chromecast stuff.
> 
> +slan for that ...
> 
> https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn#newcode270
> BUILD.gn:270: if (enable_app_list && !is_chromecast) {
> On 2016/05/05 16:01:36, Devlin wrote:
> > What's the link between app list and chromecast?
> 
> It seems unlikely to me that cast actually wants or uses app list, so maybe we
> should update the default in build/config/features.gni .

+1, would be much cleaner to update this line:

https://code.google.com/p/chromium/codesearch#chromium/src/build/config/featu...

to:

enable_app_list = !is_ios && !is_android && !is_chromecast

[tapted, 2016-05-06 02:36:56]

On 2016/05/05 18:04:10, slan wrote:
> On 2016/05/05 17:13:26, Dirk Pranke wrote:
> > lgtm w/ the possible option to fix the chromecast stuff.
> > 
> > +slan for that ...
> > 
> > https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn
> > File BUILD.gn (right):
> > 
> > https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn#newcode270
> > BUILD.gn:270: if (enable_app_list && !is_chromecast) {
> > On 2016/05/05 16:01:36, Devlin wrote:
> > > What's the link between app list and chromecast?
> > 
> > It seems unlikely to me that cast actually wants or uses app list, so maybe
we
> > should update the default in build/config/features.gni .
> 
> +1, would be much cleaner to update this line:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/featu...
> 
> to:
> 
> enable_app_list = !is_ios && !is_android && !is_chromecast

Done.

https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1861233003/diff/420001/BUILD.gn#newcode270
BUILD.gn:270: if (enable_app_list && !is_chromecast) {
On 2016/05/05 17:13:26, Dirk Pranke wrote:
> On 2016/05/05 16:01:36, Devlin wrote:
> > What's the link between app list and chromecast?
> 
> It seems unlikely to me that cast actually wants or uses app list, so maybe we
> should update the default in build/config/features.gni .

Done.

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/apps...
File chrome/browser/ui/apps/app_info_dialog.h (right):

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/apps...
chrome/browser/ui/apps/app_info_dialog.h:26: // Used for UMA to track where the
App Info dialog is launched from.
On 2016/05/05 16:01:36, Devlin wrote:
> DANGER!  These enums are used for UMA.  #if defing one out is bad.  Rename it,
> or add a comment.

Eep - thanks for catching this. Removed the #ifdefs around this, since it's
still used for ChromeOS. (added a "ChromeOS" comment).

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/star...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/1861233003/diff/420001/chrome/browser/ui/star...
chrome/browser/ui/startup/startup_browser_creator.cc:53: #include
"chrome/browser/ui/app_list/app_list_service.h"
On 2016/05/05 17:24:38, sky wrote:
> Only include if ENABLE_APP_LIST?

Done.

https://codereview.chromium.org/1861233003/diff/420001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1861233003/diff/420001/chrome/chrome_tests.gy...
chrome/chrome_tests.gypi:2655: # This assumes the AppList is views-based.
On 2016/05/05 16:01:36, Devlin wrote:
> I'm confused - shouldn't this be in a toolkit views condition rather than an
OS
> mac condition then?

Clarified the comment:

                 # This assumes the AppList is views-based, but Mac only links
                 # browser parts for the Cocoa implementation.

(i.e. the tests will fail since the AppListService alward returns the Cocoa app
list).

https://codereview.chromium.org/1861233003/diff/420001/chrome/chrome_tests.gy...
chrome/chrome_tests.gypi:2656:
'browser/ui/app_list/app_list_service_views_browsertest.cc',
On 2016/05/05 16:01:36, Devlin wrote:
> Also, if this browsertest depends on views, shouldn't it be in ui/views?

now enable_app_list \implies{} toolkit_views==1. Even on Mac - toolkit_views
exists there, but it's not used for all UI. (before the latest changes
chromecast complicated things, relying on a separate toolkit_views check in some
places, but that's now simplified).

[Devlin, 2016-05-06 17:13:39]

extensions lgtm

[slan, 2016-05-06 17:47:13]

Updated enable_app_list default value l-g-t-m

[Dirk Pranke, 2016-05-06 22:35:48]

lgtm.

[tapted, 2016-05-09 22:58:45]

sky: (ping) PTAL for  chrome/browser/ui/{BUILD.gn,startup/} and
chrome/browser/prefs/

Thanks!

[sky, 2016-05-12 15:13:18]

LGTM

[tapted, 2016-05-13 07:19:31]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-05-13 07:19:32]

The patchset sent to the CQ was uploaded after l-g-t-m from mgiuca@chromium.org,
dpranke@chromium.org, rdevlin.cronin@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1861233003/#ps460001
(title: "Rebase for r392580 - app_list_shower_views_unittest deleted by that
rev")

[commit-bot: I haz the power, 2016-05-13 07:19:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1861233003/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1861233003/460001

[commit-bot: I haz the power, 2016-05-13 08:27:06]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms. The code will be required as soon as enable_app_list is
flipped to 0. E.g. calls to --show-app-list will just show chrome://apps
instead.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms. The code will be required as soon as enable_app_list is
flipped to 0. E.g. calls to --show-app-list will just show chrome://apps
instead.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

[commit-bot: I haz the power, 2016-05-13 08:27:08]

Committed patchset #14 (id:460001)

[commit-bot: I haz the power, 2016-05-13 08:28:03]

Description was changed from

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms. The code will be required as soon as enable_app_list is
flipped to 0. E.g. calls to --show-app-list will just show chrome://apps
instead.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com
==========

to

==========
Prepare for building with enable_app_list=0 on Desktop

Created by flipping enable_app_list to '0' everywhere except ChromeOS,
ensuring it passes CQ, then flipping it back.

Mostly just splitting app_list source files out to their own gyp
variables.

This will allow us to test new code needed for previously supported
platforms. The code will be required as soon as enable_app_list is
flipped to 0. E.g. calls to --show-app-list will just show chrome://apps
instead.

Builds upon initial work in https://codereview.chromium.org/1747773002/

BUG=600915
COLLABORATOR=wierichs@google.com

Committed: https://crrev.com/d13e0cdf6fc5fddb580968adb2a5b9c5379a4404
Cr-Commit-Position: refs/heads/master@{#393474}
==========

[commit-bot: I haz the power, 2016-05-13 08:28:05]

Patchset 14 (id:??) landed as
https://crrev.com/d13e0cdf6fc5fddb580968adb2a5b9c5379a4404
Cr-Commit-Position: refs/heads/master@{#393474}