Allow compiling Android with notifications=1, add stubbed UI manager

Allow compiling Android with notifications=1, add stubbed UI manager

This patch makes it possible to compile Android's chrome_shell_apk target
whilst setting notifications=1 in the GYP_DEFINES. It also adds a stubbed
version of the NotificationUIManagerAndroid, which will be responsible
for displaying notifcations (through the Android framework).

Chrome for Android will not be using the new Message Center, because
displaying notifications will be done by the platform directly.

BUG=90795
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267041

[Peter Beverloo, 2014-04-22 18:58:18]

There are no buildbots currently compiling with notifications=1, but once the
implementation got some more body we will be able to (a) enable support, and (b)
expose it as part of experimental Web Platform features in about:flags.

[dewittj, 2014-04-22 19:03:17]

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
File chrome/browser/notifications/notification_ui_manager.cc (right):

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
chrome/browser/notifications/notification_ui_manager.cc:9: #include
"chrome/browser/notifications/message_center_notification_manager.h"
do you need to exclude these lines if Android is defined?

[Peter Beverloo, 2014-04-22 19:24:09]

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
File chrome/browser/notifications/notification_ui_manager.cc (right):

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
chrome/browser/notifications/notification_ui_manager.cc:9: #include
"chrome/browser/notifications/message_center_notification_manager.h"
On 2014/04/22 19:03:18, dewittj wrote:
> do you need to exclude these lines if Android is defined?

It would be cleaner, but since it'll add a lot of noise (three include blocks
rather than two), the file is very small and the code is #ifdef'd out anyway, I
don't think this does a lot of harm.

Happy to move them out if you prefer!

[dewittj, 2014-04-22 19:53:09]

lgtm, no need to move them out if it's okay with respect to Chromium's history. 
It seems like we should ensure a builder in the CQ and on the waterfall ASAP so
this doesn't break again (since people are not used to #ifdef ENABLE_EXTENSIONS)
as far as I can see.

[stevenjb, 2014-04-22 20:26:39]

lgtm

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
File chrome/browser/notifications/notification_ui_manager.cc (right):

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
chrome/browser/notifications/notification_ui_manager.cc:9: #include
"chrome/browser/notifications/message_center_notification_manager.h"
On 2014/04/22 19:24:10, Peter Beverloo wrote:
> On 2014/04/22 19:03:18, dewittj wrote:
> > do you need to exclude these lines if Android is defined?
> 
> It would be cleaner, but since it'll add a lot of noise (three include blocks
> rather than two), the file is very small and the code is #ifdef'd out anyway,
I
> don't think this does a lot of harm.
> 
> Happy to move them out if you prefer!

I think this is fine as-is, but if we do change it, rather than creating another
#ifdef block, I would simply move Create() to notification_ui_manager_android.cc
and eliminate this file from the android build entirely (and maybe rename it
notification_ui_manager_desktop.cc).

[Peter Beverloo, 2014-04-29 15:46:19]

+bauerb for chrome/browser/prefs/browser_prefs.cc

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
File chrome/browser/notifications/notification_ui_manager.cc (right):

https://codereview.chromium.org/247943003/diff/1/chrome/browser/notifications...
chrome/browser/notifications/notification_ui_manager.cc:9: #include
"chrome/browser/notifications/message_center_notification_manager.h"
On 2014/04/22 20:26:39, stevenjb wrote:
> On 2014/04/22 19:24:10, Peter Beverloo wrote:
> > On 2014/04/22 19:03:18, dewittj wrote:
> > > do you need to exclude these lines if Android is defined?
> > 
> > It would be cleaner, but since it'll add a lot of noise (three include
blocks
> > rather than two), the file is very small and the code is #ifdef'd out
anyway,
> I
> > don't think this does a lot of harm.
> > 
> > Happy to move them out if you prefer!
> 
> I think this is fine as-is, but if we do change it, rather than creating
another
> #ifdef block, I would simply move Create() to
notification_ui_manager_android.cc
> and eliminate this file from the android build entirely (and maybe rename it
> notification_ui_manager_desktop.cc).
> 

Good idea, thanks! Done.

[Bernhard Bauer, 2014-04-29 15:50:14]

c/b/p LGTM.

[Peter Beverloo, 2014-04-29 17:04:58]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2014-04-29 17:05:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/peter@chromium.org/247943003/20001

[commit-bot: I haz the power, 2014-04-30 02:00:58]

Change committed as 267041