Fixes a crash caused for update of webkit notifications.

Fixes a crash caused for update of webkit notifications.

If update happens with different ids, it essentially means a removal
and an add from notification_list point of view. Sending Update() will
confuse the observers.

It is possible to pass both ids for Update(), but handling update of
different ids on each observer doesn't make much sense. It will be better
to send both Removed() and Added() in this case.

BUG=233221
TEST=manually on the reported page in the bug

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

[Dmitry Titov, 2013-04-18 20:06:05]

I'm curious when do you see these ids be different? I think (but not sure) that
current code for both webkitNotifications and rich notifications in practice
ensures that old_id == new_id when MessageCenter::UpdateNotification is called.

Do you see it coming with different ids?

[Jun Mukai, 2013-04-18 20:28:15]

On 2013/04/18 20:06:05, Dmitry Titov wrote:
> I'm curious when do you see these ids be different? I think (but not sure)
that
> current code for both webkitNotifications and rich notifications in practice
> ensures that old_id == new_id when MessageCenter::UpdateNotification is
called.
> 
> Do you see it coming with different ids?

The webkit notifications uses NotificationObjectProxy, which creates the id as
StringPrintf("%d:%d:%d:%d", process_id_, route_id_, notification_id_, worker_);
for the message center.

However, the webkit notification also has 'replaceId' field and it will invoke
update if replaceId is matched. In that case, the old_id and new_id can be
different.

[dewittj, 2013-04-18 22:20:25]

lgtm

[commit-bot: I haz the power, 2013-04-18 22:37:47]

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

[commit-bot: I haz the power, 2013-04-19 09:30:44]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android_dbg, revision is HEAD

[dewittj, 2013-04-19 15:46:32]

On 2013/04/19 09:30:44, I haz the power (commit-bot) wrote:
> The commit queue went berserk retrying too often for a
> seemingly flaky test. Builder is android_dbg, revision is HEAD

This patch may introduce some undesired effects for Webkit notifications -
namely, a new notification with the same replace_id should be treated as an
Update, not as a close/open new notification.  Need this behavior because we
don't have an update function in webkit notifications land.

[dewittj, 2013-04-19 16:03:17]

On 2013/04/19 15:46:32, dewittj wrote:
> On 2013/04/19 09:30:44, I haz the power (commit-bot) wrote:
> > The commit queue went berserk retrying too often for a
> > seemingly flaky test. Builder is android_dbg, revision is HEAD
> 
> This patch may introduce some undesired effects for Webkit notifications -
> namely, a new notification with the same replace_id should be treated as an
> Update, not as a close/open new notification.  Need this behavior because we
> don't have an update function in webkit notifications land.

Might be best to fix this by keeping this patch (since it is two different
notifications as far as the notificationlist is concerned) but also changing
NotificationObjectProxy to generate the notification id based on replaceid.

[Jun Mukai, 2013-04-19 21:55:09]

On 2013/04/19 16:03:17, dewittj wrote:
> On 2013/04/19 15:46:32, dewittj wrote:
> > On 2013/04/19 09:30:44, I haz the power (commit-bot) wrote:
> > > The commit queue went berserk retrying too often for a
> > > seemingly flaky test. Builder is android_dbg, revision is HEAD
> > 
> > This patch may introduce some undesired effects for Webkit notifications -
> > namely, a new notification with the same replace_id should be treated as an
> > Update, not as a close/open new notification.  Need this behavior because we
> > don't have an update function in webkit notifications land.
> 
> Might be best to fix this by keeping this patch (since it is two different
> notifications as far as the notificationlist is concerned) but also changing
> NotificationObjectProxy to generate the notification id based on replaceid.

okay, let me go ahead with this for now.

[commit-bot: I haz the power, 2013-04-19 21:56:35]

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

[commit-bot: I haz the power, 2013-04-20 00:11:46]

Change committed as 195346