Make chrome/browser/ui/webui not depend on ash when using Athena

Make chrome/browser/ui/webui not depend on ash when using Athena

This CL:
- Adds !defined(USE_ATHENA) ifdefs where Ash is used.
- Stops compiling KeyboardOverlayUI in Athena because it is not necessary
- Stops compiling in Athena for simplicity:
   DisplayOptionsHandler, PowerHandler, FirstRunUI

BUG=426561
TEST=None
TBR=oshima,estade,stevenjb

Committed: https://crrev.com/9a50e09098a21b4484df6793e6660b8fde3d1481
Cr-Commit-Position: refs/heads/master@{#302376}

[pkotwicz, 2014-10-29 23:49:34]

pkotwicz@chromium.org changed reviewers:
+ oshima@chromium.org

[pkotwicz, 2014-10-29 23:49:34]

Oshima, PTAL

[oshima, 2014-10-30 18:08:41]

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc (right):

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc:20: #if
defined(USE_ASH)

First run impl for athena will be completely different.
Is it hard to exclude this first run ui on athena?

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc (right):

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc:393: const
gfx::Size& size = ash::Shell::GetScreen()->GetPrimaryDisplay().size();
Can you just use

gfx::Screen::GetNativeScren()->GetPrimaryDisplay()

instead?

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:27: #include
"ash/audio/sounds.h"
ash/audio should be easy to componentize. can you or one of WAT folks work on
it?

[pkotwicz, 2014-10-30 21:31:41]

Oshima, can you please take another look?

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc (right):

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc:20: #if
defined(USE_ASH)
On 2014/10/30 18:08:41, oshima wrote:
> 
> First run impl for athena will be completely different.
> Is it hard to exclude this first run ui on athena?

Done.

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc (right):

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc:393: const
gfx::Size& size = ash::Shell::GetScreen()->GetPrimaryDisplay().size();
On 2014/10/30 18:08:41, oshima wrote:
> Can you just use
> 
> gfx::Screen::GetNativeScren()->GetPrimaryDisplay()
> 
> instead?

Done.

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://codereview.chromium.org/684143003/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:27: #include
"ash/audio/sounds.h"
I have kicked off the process by sending an email about removing
--ash-enable-system-sounds (I have CCed you on the email)

I do not think that componentization should block this CL

[oshima, 2014-10-30 22:51:48]

lgtm

[pkotwicz, 2014-10-31 14:28:08]

pkotwicz@chromium.org changed reviewers:
+ estade@chromium.org

[pkotwicz, 2014-10-31 14:28:09]

estade@ for chrome/browser/ui/webui/options OWNERS

[pkotwicz, 2014-10-31 17:50:20]

pkotwicz@chromium.org changed reviewers:
+ jhawkins@chromium.org

[pkotwicz, 2014-10-31 17:50:59]

estade@ seems to be OOO

jhawkins@ can you please take a look at the changes in
chrome/browser/ui/webui/options

[Evan Stade, 2014-10-31 18:27:12]

https://codereview.chromium.org/684143003/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/684143003/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:1709:
ash::Shell::GetInstance()->user_wallpaper_delegate()->OpenSetWallpaperPage();
#else notimplemented? notreached?

https://codereview.chromium.org/684143003/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
(right):

https://codereview.chromium.org/684143003/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:34:
#if !defined(USE_ATHENA)
why not USE_ASH?

[pkotwicz, 2014-10-31 19:05:49]

Patchset #3 (id:40001) has been deleted

[pkotwicz, 2014-10-31 19:07:59]

Evan, can you please take another look?

https://codereview.chromium.org/684143003/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/684143003/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:1709:
ash::Shell::GetInstance()->user_wallpaper_delegate()->OpenSetWallpaperPage();
Added NOTIMPLEMENTED()

We will need to grep the codebase for USE_ATHENA ifdefs. Each ifdef is a bug. We
have added USE_ATHENA ifdefs in random places to make Chrome not crash.

I have changed all of the ifdefs in this CL to be !defined(USE_ATHENA) instead
of USE_ASH. Because we are still compiling Athena with Ash, using USE_ATHENA
instead of USE_ASH ifdefs "does something". When I make Athena no longer compile
Ash, I will write a CL to convert all of the USE_ATHENA ifdefs to USE_ASH

[stevenjb, 2014-10-31 19:20:34]

stevenjb@chromium.org changed reviewers:
+ stevenjb@chromium.org

[stevenjb, 2014-10-31 19:20:34]

https://codereview.chromium.org/684143003/diff/40002/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/set_time_ui.cc (right):

https://codereview.chromium.org/684143003/diff/40002/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/set_time_ui.cc:130: ash::user::LOGGED_IN_NONE)
{
Use LoginState::Get()->IsUserLoggedIn() instead of disabling.

https://codereview.chromium.org/684143003/diff/40002/chrome/chrome_browser_ui...
File chrome/chrome_browser_ui.gypi (left):

https://codereview.chromium.org/684143003/diff/40002/chrome/chrome_browser_ui...
chrome/chrome_browser_ui.gypi:1679:
'browser/ui/webui/options/chromeos/internet_options_handler_strings.h',
Is there an issue tracking what the plan is for Settings?
internet_options_handler* should no longer have any Ash dependencies; I'm pretty
sure we are going to need it for Athena, one way or another.

[pkotwicz, 2014-10-31 20:43:03]

stevenjb@ and estade@ can you please take another look?

Yes, there is crbug.com/380213 which is tracking the plan for settings. I
believe that the bug is assigned to stevenjb@

It turns out that internet_options_handler_strings does not need to include
grit/ash_strings. This CL now includes internet_options_handler_strings.h

[pkotwicz, 2014-10-31 20:44:39]

https://codereview.chromium.org/684143003/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://codereview.chromium.org/684143003/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:21: #include
"media/audio/sounds/sounds_manager.h"
Previously, we were relying on ash/audio/sounds.h to include this file for us

[stevenjb, 2014-10-31 20:57:02]

https://codereview.chromium.org/684143003/diff/40002/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/set_time_ui.cc (right):

https://codereview.chromium.org/684143003/diff/40002/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/set_time_ui.cc:130: ash::user::LOGGED_IN_NONE)
{
On 2014/10/31 19:20:34, stevenjb wrote:
> Use LoginState::Get()->IsUserLoggedIn() instead of disabling.

LoginState?

[Evan Stade, 2014-10-31 20:58:55]

lgtm

[pkotwicz, 2014-10-31 22:22:32]

stevenjb@ can you please take another look?

[stevenjb, 2014-10-31 22:34:23]

lgtm

[pkotwicz, 2014-10-31 23:01:09]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-10-31 23:02:44]

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

[commit-bot: I haz the power, 2014-10-31 23:09:52]

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

[commit-bot: I haz the power, 2014-10-31 23:09:54]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pkotwicz, 2014-11-01 01:01:16]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-01 01:01:50]

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

[pkotwicz, 2014-11-01 01:02:04]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2014-11-01 01:03:03]

Patchset #7 (id:120001) has been deleted

[pkotwicz, 2014-11-01 01:03:30]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-01 01:04:42]

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

[commit-bot: I haz the power, 2014-11-01 01:53:40]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2014-11-01 01:54:11]

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