Fixes possible crashes for ProfileNotification is not found.

Fixes possible crashes for ProfileNotification is not found.

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

[dewittj1, 2013-04-17 20:02:04]

+dimich

[Dmitry Titov, 2013-04-17 20:13:51]

Hmm. The only reason for the notificaiton not to be there is if it was removed
by MessageCenterNotificationManager::OnNotificationRemoved() before
OnNotificaitonClicked was called. I wonder how that can happen, because it may
be an indication of a real bug, which this fix could cover up...

[Dmitry Titov, 2013-04-17 21:58:52]

LGTM with a note. I also put a version of how this can happen into the related
bug. It seems we must assume the set of views/widgets may not mirror the set of
notifications at all times.

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

https://codereview.chromium.org/14205012/diff/1/chrome/browser/notifications/...
chrome/browser/notifications/message_center_notification_manager.cc:428: if
(iter == profile_notifications_.end() || !(*iter).second)
I think we don't need the second part of the check. At this point the
notification is most likely simply removed. In fact, I'd remove the LOG as well
(it's expected condition, sorta), so the whoel this if() can go.

[dewittj, 2013-04-18 00:06:12]

lgtm agree with dimich.

[Jun Mukai, 2013-04-18 03:08:09]

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

https://codereview.chromium.org/14205012/diff/1/chrome/browser/notifications/...
chrome/browser/notifications/message_center_notification_manager.cc:428: if
(iter == profile_notifications_.end() || !(*iter).second)
On 2013/04/17 21:58:52, Dmitry Titov wrote:
> I think we don't need the second part of the check. At this point the
> notification is most likely simply removed. In fact, I'd remove the LOG as
well
> (it's expected condition, sorta), so the whoel this if() can go.

removed. thanks!

[dewittj, 2013-04-18 17:37:10]

On 2013/04/18 03:08:09, Jun Mukai wrote:
>
https://codereview.chromium.org/14205012/diff/1/chrome/browser/notifications/...
> File chrome/browser/notifications/message_center_notification_manager.cc
> (right):
> 
>
https://codereview.chromium.org/14205012/diff/1/chrome/browser/notifications/...
> chrome/browser/notifications/message_center_notification_manager.cc:428: if
> (iter == profile_notifications_.end() || !(*iter).second)
> On 2013/04/17 21:58:52, Dmitry Titov wrote:
> > I think we don't need the second part of the check. At this point the
> > notification is most likely simply removed. In fact, I'd remove the LOG as
> well
> > (it's expected condition, sorta), so the whoel this if() can go.
> 
> removed. thanks!

lgtm++

[commit-bot: I haz the power, 2013-04-18 19:27:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/14205012/6001

[commit-bot: I haz the power, 2013-04-18 23:25:06]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-04-19 03:38:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/14205012/6001

[commit-bot: I haz the power, 2013-04-19 08:39:39]

Change committed as 195104