arc: OnClosedFromAndroid removes notifications as not by_user

arc: OnClosedFromAndroid removes notifications as not by_user

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149, 649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.

Committed: https://crrev.com/5ac2951337b278b6caaed5304f95db151a411983
Cr-Commit-Position: refs/heads/master@{#423565}

[xiyuan, 2016-10-05 17:35:49]

xiyuan@chromium.org changed reviewers:
+ yoshiki@chromium.org

[xiyuan, 2016-10-05 17:35:58]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 17:36:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 18:07:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 18:08:00]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2016-10-05 21:46:58]

Description was changed from

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

to

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

[yoshiki, 2016-10-06 00:24:04]

lgtm

[yoshiki, 2016-10-06 06:37:14]

Still lgtm but have one request.

https://codereview.chromium.org/2394483004/diff/1/ui/arc/notification/arc_not...
File ui/arc/notification/arc_notification_manager.cc (right):

https://codereview.chromium.org/2394483004/diff/1/ui/arc/notification/arc_not...
ui/arc/notification/arc_notification_manager.cc:104:
item->OnClosedFromAndroid(false /* by_user */);
Could you remove the "by_user" argument from
ArcNotificationItem::OnClosedFromAndroid() and to pass "false" directly from
inside of ArcNotificationItem::OnClosedFromAndroid()?

I think that's better than passing false from here.

[xiyuan, 2016-10-06 15:55:32]

Description was changed from

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

to

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379,b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

[xiyuan, 2016-10-06 15:56:35]

Description was changed from

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379,b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

to

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

[xiyuan, 2016-10-06 16:03:14]

Description was changed from

==========
arc: Set |by_user| to false in OnNotificationRemoved

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

to

==========
arc: OnClosedFromAndroid removes notifications as not by_user

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

[xiyuan, 2016-10-06 16:15:41]

https://codereview.chromium.org/2394483004/diff/1/ui/arc/notification/arc_not...
File ui/arc/notification/arc_notification_manager.cc (right):

https://codereview.chromium.org/2394483004/diff/1/ui/arc/notification/arc_not...
ui/arc/notification/arc_notification_manager.cc:104:
item->OnClosedFromAndroid(false /* by_user */);
On 2016/10/06 06:37:13, yoshiki wrote:
> Could you remove the "by_user" argument from
> ArcNotificationItem::OnClosedFromAndroid() and to pass "false" directly from
> inside of ArcNotificationItem::OnClosedFromAndroid()?
> 
> I think that's better than passing false from here.

Sounds good. Done.

[xiyuan, 2016-10-06 16:15:49]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 16:16:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 16:48:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-06 16:48:29]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2016-10-06 16:52:05]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2016-10-06 16:52:05]

The patchset sent to the CQ was uploaded after l-g-t-m from yoshiki@chromium.org
Link to the patchset: https://codereview.chromium.org/2394483004/#ps20001
(title: "remove by_user from OnClosedFromAndroid")

[commit-bot: I haz the power, 2016-10-06 16:52:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 16:59:13]

Description was changed from

==========
arc: OnClosedFromAndroid removes notifications as not by_user

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

to

==========
arc: OnClosedFromAndroid removes notifications as not by_user

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

[commit-bot: I haz the power, 2016-10-06 16:59:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-06 17:02:39]

Description was changed from

==========
arc: OnClosedFromAndroid removes notifications as not by_user

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.
==========

to

==========
arc: OnClosedFromAndroid removes notifications as not by_user

Otherwise, a pinned notification remains in message center while
the underlying ArcNotificationItem is destroyed. This would cause
a crash later when message center attempts to create a view for
the notification.

BUG=647149,649379
BUG=b/31905643
TEST=Install any app from play store. After install finishes, bring
    up the message center and observe no crash.

Committed: https://crrev.com/5ac2951337b278b6caaed5304f95db151a411983
Cr-Commit-Position: refs/heads/master@{#423565}
==========

[commit-bot: I haz the power, 2016-10-06 17:02:41]

Patchset 2 (id:??) landed as
https://crrev.com/5ac2951337b278b6caaed5304f95db151a411983
Cr-Commit-Position: refs/heads/master@{#423565}