Automatically close the default push notification when a real one appears.

Automatically close the default push notification when a real one appears.

When the developer shows a notification themselves in response to
an incoming push message, and a default notification still happens
to stick around from a previous message where they forgot to do so,
the default notification should be closed automagically.

BUG=495175
Committed: https://crrev.com/ed53734f33844f3d1c182dc18cf6d808d0221e60
Cr-Commit-Position: refs/heads/master@{#332387}

[Miguel Garcia, 2015-06-02 13:25:45]

miguelg@chromium.org changed reviewers:
+ miguelg@chromium.org

[Miguel Garcia, 2015-06-02 13:25:45]

lgtm

[Peter Beverloo, 2015-06-02 13:28:30]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2015-06-02 13:28:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149243005/1

[johnme, 2015-06-02 13:35:00]

The CQ bit was unchecked by johnme@chromium.org

[johnme, 2015-06-02 13:43:51]

johnme@chromium.org changed reviewers:
+ johnme@chromium.org

[johnme, 2015-06-02 13:43:52]

lgtm with nits

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_browsertest.cc (right):

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_browsertest.cc:577: EXPECT_EQ(1u,
notification_manager()->GetNotificationCount());
It would be nice to check that the right notification is showing.

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_notification_manager.cc
(left):

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:146: #if
defined(OS_ANDROID)
Please don't remove this. The double closing brackets avoid breaking various
editors' syntax highlighters (including IIRC sublime text). Perhaps there should
be a comment explaining that.

[Peter Beverloo, 2015-06-02 13:58:08]

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_browsertest.cc (right):

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_browsertest.cc:577: EXPECT_EQ(1u,
notification_manager()->GetNotificationCount());
On 2015/06/02 13:43:52, johnme wrote:
> It would be nice to check that the right notification is showing.

Done.

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_notification_manager.cc
(left):

https://codereview.chromium.org/1149243005/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:146: #if
defined(OS_ANDROID)
On 2015/06/02 13:43:52, johnme wrote:
> Please don't remove this. The double closing brackets avoid breaking various
> editors' syntax highlighters (including IIRC sublime text). Perhaps there
should
> be a comment explaining that.

I'm having a very hard time accepting that reasoning John. This code is not
cool.

I've reverted it for the sake of this patch since we want to merge it, but I
really want to see this go.

[Peter Beverloo, 2015-06-02 13:59:08]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2015-06-02 13:59:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
miguelg@chromium.org, johnme@chromium.org
Link to the patchset: https://codereview.chromium.org/1149243005/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-06-02 13:59:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149243005/40001

[commit-bot: I haz the power, 2015-06-02 15:11:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-02 15:11:46]

Patchset 3 (id:??) landed as
https://crrev.com/ed53734f33844f3d1c182dc18cf6d808d0221e60
Cr-Commit-Position: refs/heads/master@{#332387}