[Eraser] First pass at removing the notification center panel.

[Eraser] First pass at removing the notification center panel.

This just rips out the center and settings component, and does not
refactor any of the delegate/interface classes to match the new reality
(that is to be done later).

Also TODO: removing the entry from the OSX main menu (didn't want
to do xib stuff here), so for now the entry is just disabled.

R=peter,rsesek,oshima,bauerb
TBR=atwilson

BUG=530376
Committed: https://crrev.com/751821773c1ac7189331bdf02af7d3d06bd11d00
Cr-Commit-Position: refs/heads/master@{#349425}

[dewittj, 2015-09-15 21:54:15]

dewittj@chromium.org changed reviewers:
+ peter@chromium.org, rsesek@chromium.org

[dewittj, 2015-09-15 21:55:11]

PTAL

[Robert Sesek, 2015-09-15 22:07:08]

Please also wrap the CL description so that it shows up better in `git log`.

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/app_con...
File chrome/browser/app_controller_mac.mm (right):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/app_con...
chrome/browser/app_controller_mac.mm:1002: enable = NO;
Leave a TODO and point to the bug # ?

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/app_con...
chrome/browser/app_controller_mac.mm:1326: -
(void)updateDisplayMessageCenterPrefMenuItem:(NSMenuItem*)item {
You can remove this. Only the -toggle… method is referenced by the xib.

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/notific...
File chrome/browser/notifications/message_center_notification_manager.cc
(right):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/notific...
chrome/browser/notifications/message_center_notification_manager.cc:278: 
nit: remove extra blank line

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.h (right):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.h:54:
scoped_ptr<message_center::MessageCenterTray> tray_;
I assume we still need |tray_| per the CL description, in that the interfaces
need to be refactored and then this can be removed?

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm (left):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm:168: //
TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465858
WontFix this bug ?

https://codereview.chromium.org/1334363002/diff/100001/ui/message_center/coco...
File ui/message_center/cocoa/status_item_view.mm (left):

https://codereview.chromium.org/1334363002/diff/100001/ui/message_center/coco...
ui/message_center/cocoa/status_item_view.mm:165: { IDR_TRAY_EMPTY,
IDR_TRAY_EMPTY_PRESSED },
Can we remove these resources too?

[dewittj, 2015-09-15 22:52:23]

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/app_con...
File chrome/browser/app_controller_mac.mm (right):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/app_con...
chrome/browser/app_controller_mac.mm:1002: enable = NO;
On 2015/09/15 22:07:08, Robert Sesek wrote:
> Leave a TODO and point to the bug # ?

Done.

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/app_con...
chrome/browser/app_controller_mac.mm:1326: -
(void)updateDisplayMessageCenterPrefMenuItem:(NSMenuItem*)item {
On 2015/09/15 22:07:08, Robert Sesek wrote:
> You can remove this. Only the -toggle… method is referenced by the xib.

Done.

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/notific...
File chrome/browser/notifications/message_center_notification_manager.cc
(right):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/notific...
chrome/browser/notifications/message_center_notification_manager.cc:278: 
On 2015/09/15 22:07:08, Robert Sesek wrote:
> nit: remove extra blank line

Done.

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.h (right):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.h:54:
scoped_ptr<message_center::MessageCenterTray> tray_;
On 2015/09/15 22:07:08, Robert Sesek wrote:
> I assume we still need |tray_| per the CL description, in that the interfaces
> need to be refactored and then this can be removed?

I was assuming so, but now that I look it's only used in one place so I'll just
modify that place and then we can remove this member var entirely.

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm (left):

https://codereview.chromium.org/1334363002/diff/100001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm:168: //
TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465858
On 2015/09/15 22:07:08, Robert Sesek wrote:
> WontFix this bug ?

Done.

https://codereview.chromium.org/1334363002/diff/100001/ui/message_center/coco...
File ui/message_center/cocoa/status_item_view.mm (left):

https://codereview.chromium.org/1334363002/diff/100001/ui/message_center/coco...
ui/message_center/cocoa/status_item_view.mm:165: { IDR_TRAY_EMPTY,
IDR_TRAY_EMPTY_PRESSED },
On 2015/09/15 22:07:08, Robert Sesek wrote:
> Can we remove these resources too?

Yes!

Done.

[Robert Sesek, 2015-09-15 23:06:19]

LGTM

[dewittj, 2015-09-15 23:07:36]

looks like tray_ is still required after all for now.  I'll look into removing
that (basically useless) interface next.

[Robert Sesek, 2015-09-15 23:08:27]

On 2015/09/15 23:07:36, dewittj wrote:
> looks like tray_ is still required after all for now.  I'll look into removing
> that (basically useless) interface next.

OK. +135 lines, -4208 lines is still pretty \o/

[Peter Beverloo, 2015-09-16 10:34:10]

On 2015/09/15 23:08:27, Robert Sesek wrote:
> On 2015/09/15 23:07:36, dewittj wrote:
> > looks like tray_ is still required after all for now.  I'll look into
removing
> > that (basically useless) interface next.
> 
> OK. +135 lines, -4208 lines is still pretty \o/

Whoohoo! lgtm, thank you so much!

[Peter Beverloo, 2015-09-16 10:34:20]

https://codereview.chromium.org/1334363002/diff/140001/chrome/browser/notific...
File chrome/browser/notifications/notification_ui_manager_desktop.cc (left):

https://codereview.chromium.org/1334363002/diff/140001/chrome/browser/notific...
chrome/browser/notifications/notification_ui_manager_desktop.cc:16:
NotificationUIManager* NotificationUIManager::Create(PrefService* local_state) {
nit: maybe add a TODO to remove the argument? Android does not use it either.

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/fake...
File ui/message_center/fake_message_center_tray_delegate.h (right):

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/fake...
ui/message_center/fake_message_center_tray_delegate.h:21:
FakeMessageCenterTrayDelegate(MessageCenter* message_center);
nit: explicit

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
File ui/message_center/message_center_tray.cc (right):

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
ui/message_center/message_center_tray.cc:24: #ifdef OS_CHROMEOS
nit: #if defined(OS_CHROMEOS)

(896 vs. 4 occurrences)

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
File ui/message_center/message_center_tray_unittest.cc (right):

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
ui/message_center/message_center_tray_unittest.cc:303: NotifierId
notifier_id(GURL("www.test.org"));
micro nit: protocol?

[dewittj, 2015-09-16 18:23:49]

https://codereview.chromium.org/1334363002/diff/140001/chrome/browser/notific...
File chrome/browser/notifications/notification_ui_manager_desktop.cc (left):

https://codereview.chromium.org/1334363002/diff/140001/chrome/browser/notific...
chrome/browser/notifications/notification_ui_manager_desktop.cc:16:
NotificationUIManager* NotificationUIManager::Create(PrefService* local_state) {
On 2015/09/16 10:34:20, Peter Beverloo wrote:
> nit: maybe add a TODO to remove the argument? Android does not use it either.

Done in notification_ui_manager.h

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/fake...
File ui/message_center/fake_message_center_tray_delegate.h (right):

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/fake...
ui/message_center/fake_message_center_tray_delegate.h:21:
FakeMessageCenterTrayDelegate(MessageCenter* message_center);
On 2015/09/16 10:34:20, Peter Beverloo wrote:
> nit: explicit

Done.

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
File ui/message_center/message_center_tray.cc (right):

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
ui/message_center/message_center_tray.cc:24: #ifdef OS_CHROMEOS
On 2015/09/16 10:34:20, Peter Beverloo wrote:
> nit: #if defined(OS_CHROMEOS)
> 
> (896 vs. 4 occurrences)

Done.

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
File ui/message_center/message_center_tray_unittest.cc (right):

https://codereview.chromium.org/1334363002/diff/140001/ui/message_center/mess...
ui/message_center/message_center_tray_unittest.cc:303: NotifierId
notifier_id(GURL("www.test.org"));
On 2015/09/16 10:34:20, Peter Beverloo wrote:
> micro nit: protocol?

Done.

[dewittj, 2015-09-16 18:39:48]

dewittj@chromium.org changed reviewers:
+ atwilson@chromium.org, bauerb@chromium.org, oshima@chromium.org

[dewittj, 2015-09-16 18:44:29]

Hi additional reviewers, please take a look at the changes in your areas of
ownership:

oshima: resources
atwilson: background
bauerb: browser_prefs

Thanks!

[oshima, 2015-09-16 19:46:11]

resources lgtm

[Bernhard Bauer, 2015-09-17 09:09:19]

browser_prefs LGTM

[dewittj, 2015-09-17 16:20:23]

atwilson -> TBR for trivial change to a background test.

[dewittj, 2015-09-17 16:20:29]

The CQ bit was checked by dewittj@chromium.org

[dewittj, 2015-09-17 16:20:30]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
peter@chromium.org
Link to the patchset: https://codereview.chromium.org/1334363002/#ps160001
(title: "peter comments")

[commit-bot: I haz the power, 2015-09-17 16:40:06]

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

[commit-bot: I haz the power, 2015-09-17 16:46:41]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-09-17 16:47:19]

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