Defer construction of NotificationUIManager to fix notification initialization.

Defer construction of NotificationUIManager to fix notification initialization.

This is instead of http://codereview.chromium.org/8566054/ and should allow the refactoring code to proceed.

BUG=chromium:103427
TEST=See issue


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110853

[stevenjb, 2011-11-19 01:06:52]

Once more with feeling. I wish I'd done this the first time.

John identified what the actual cause of failure was. It seems that when I
changed DesktopNotificationServiceFactory::BuildServiceInstanceFor to pass NULL
to DesktopNotificationService(), that allowed some other CL(s) to avoid early UI
dependencies also, so by reverting that change I exposed a new dependency.

So, this CL will avoid passing a lazy-initialized singleton in the constructor,
and instead will set/construct it when it actually uses it.

[jam, 2011-11-19 01:18:31]

http://codereview.chromium.org/8550003/diff/1/chrome/browser/notifications/de...
File chrome/browser/notifications/desktop_notification_service.cc (right):

http://codereview.chromium.org/8550003/diff/1/chrome/browser/notifications/de...
chrome/browser/notifications/desktop_notification_service.cc:217:
DesktopNotificationService::DesktopNotificationService(Profile* profile,
can we make this take a single argument now?

http://codereview.chromium.org/8550003/diff/1/chrome/browser/notifications/de...
chrome/browser/notifications/desktop_notification_service.cc:429: if
(!ui_manager_)
this member variable, and even this function, seem unnecessary

[stevenjb, 2011-11-19 01:24:20]

http://codereview.chromium.org/8550003/diff/1/chrome/browser/notifications/de...
File chrome/browser/notifications/desktop_notification_service.cc (right):

http://codereview.chromium.org/8550003/diff/1/chrome/browser/notifications/de...
chrome/browser/notifications/desktop_notification_service.cc:217:
DesktopNotificationService::DesktopNotificationService(Profile* profile,
On 2011/11/19 01:18:32, John Abd-El-Malek wrote:
> can we make this take a single argument now?

Unfortunately no. We do construct locally a NotificationUIManager and pass it in
for tests.

http://codereview.chromium.org/8550003/diff/1/chrome/browser/notifications/de...
chrome/browser/notifications/desktop_notification_service.cc:429: if
(!ui_manager_)
On 2011/11/19 01:18:32, John Abd-El-Malek wrote:
> this member variable, and even this function, seem unnecessary
Tests might set this in which case we do not want to override it.

[jam, 2011-11-19 01:28:24]

i see. it's more self-documenting in these scenarios if we have a XForTesting
method to set a member, and then it's clear for everyone looking at it what's
happening. but doesn't need to happen now.

lgtm

[commit-bot: I haz the power, 2011-11-19 17:17:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/8550003/3001

[commit-bot: I haz the power, 2011-11-19 18:09:03]

Try job failure for 8550003-3001 (retry) on linux_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2011-11-20 00:57:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/8550003/3001

[commit-bot: I haz the power, 2011-11-20 02:11:55]

Change committed as 110853