1052435 - Old SMS notifications reappear after reboot

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Anthony Ricaud (:rik)]

STR:
1. Send one SMS to a FxOS device: "Foo"
2. Wait for the FxOs device to receive it
3. Send a second SMS to the FxOS device: "Bar"
4. Open the thread in SMS (by opening the SMS app, not taping the notification)
5. Reboot

Expected: No notifications appear

Actual: The notification for "Foo" appears.


It looks like all notifications that have been replaced will replay at reboot.

Comment 1

[:gerard-majax]

[Blocking Requested - why for this release]:

Comment 2

[:gerard-majax]

Thanks. It looks like there is something bad happening with tags.

Comment 3

[:gerard-majax]

STR:
 0. Send one SMS to n1
 1. Send one SMS to n1
 2. Open SMS app

Expected:
 notificationstore.json at step (2) should be empty

Actual
 notificationstore.json at step (2) contains the first sms notification from step (0)

Comment 4

[:gerard-majax]

Looks like we were missing some testcases, in fact.

Comment 5

[:gerard-majax]

This is actually tested, and the test is green for a nice reason: at http://dxr.mozilla.org/mozilla-central/source/dom/src/notification/test/unit/test_notificationdb.js#158 we create two notifications from the system app, with the same tag and two different IDs.

The fun part begins when one notices that the final systemNotification1 and systemNotification2 objects are shareing the same id. This makes the NotificationDB code thinking it's the same notification, hence hiding the bug.

Comment 6

[:gerard-majax]

Attachment: Fix handling of Notification with tag

This commit fixes two bugs. When sending two notifications with the same
tag, the NotificationDB this.byTag member was not properly updated. This
resulted in multiple notifications with the same tag being saved.
NotificationStorage's cache may hide this during the running session,
but on B2G the resend of notifications at reboot would expose the issue.
The second bug fixed is the test that makes sure we properly overwrite
notifications with the same tag in the database: the way we handled fake
notification object made them sharing the same ID. This, NotificationDB
would not even consider the tags and this lead to hiding the bug.

Comment 7

[:gerard-majax]

Comment on attachment 8471699 [details] [diff] [review]
Fix handling of Notification with tag

Michael, do you mind to review this?

Comment 8

[:gerard-majax]

https://tbpl.mozilla.org/?tree=Try&rev=4cd91af9ae05
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4cd91af9ae05

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8471699 [details] [diff] [review]
Fix handling of Notification with tag

Review of attachment 8471699 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 10

[:gerard-majax]

Thanks. Try is all green, tree is closed :(

Comment 11

[:gerard-majax]

Beatriz, you may want to know about this one.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/3e71c9bb48e4

Comment 13

[Dietrich Ayala (:dietrich)]

Malfunctioning new feature for 2.0, blocking+ from your friendly neighborhood triage team.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3e71c9bb48e4

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/997180f6c66b

Comment 17

[Beatriz Rodríguez [:brg]]

(In reply to Alexandre LISSY :gerard-majax from comment #11)
> Beatriz, you may want to know about this one.
Thanks Alexandre!
Testing in 2.0 Flame today build(Gecko 1dc3556, Gaia 8d051e3) the issue is not reproduced in our side anymore. Sms notifications of read messages are not displayed after rebooting.

Comment 18

[sarsenyev]

Verified the issue is fixed on 2.1 and 2.0 Flame

No old notifications appears after rebooting

"Flame 2.1 

Device: Flame 2.1 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141120001207
Gaia: f8d3bf44029e0afc0124600a4bb34dba8fc1ad21
Gecko: f70a67a7f846
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0"

"Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141120000206
Gaia: 1ede2666f1e6c1b3fd3b282011caf0cbc59544b0
Gecko: 54f1b0ee07a6
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0"