946569 - [Wap push] When send multiple messages to device, some message will be expired even thought there is no SI expire properties in those messages.

Status: RESOLVED DUPLICATE of bug 942080

(Firefox OS Graveyard :: Gaia, defect)

Description

[Enpei]

Attachment: time stamp: 23:24, 23:26

When there are multiple messages received on DUT, some of it might be expired even though no SI expire date has been given in those messages. It should be 100% reproducible when DUT receives messages in suspend mode. Sometimes I can still reproduce it when DUT is in home page.

* Build Number                
Buri
Gaia      8d762f3376318fd6be390432db750ae4904c9ab6
Gecko     http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/758f3fb32dda
BuildID   20131204004003
Version   26.0

* Reproduce Steps
Please check log buri_oneexpire.txt
1. Send 3 WAP push messages while device is suspend(short press power key)
Time stamp: 23:24, message text is wap8suspend, and 23:26, wap10 and 11 messages are all expired.

log buri_canget.txt
1. Send 5 WAP push messages while device is in home page
Time stamp: 12:34, one of message text wap 1 to wap 5, I don't remember which one is.

2. Send 5 WAP push messages while device is suspend(short press power key)
Time stamp: 12:37, message text wap 14 suspend.

Comment 1

[Enpei]

Hi Gabriele,

Could you help to check this bug? It's a regression and I need sometime to find out when is it break.

Comment 2

[Enpei]

Attachment: time stamp: 12:34 and 12:37.

Comment 3

[Enpei]

1127 build still has the bug, might be nothing to do with bug 932209.

1121 build still has the problem, too. But when setting first 8 message I didn't find it, until sending 12 messages then I met it. It's the build after bug 929895 is fixed.
Gaia:     ce276842c9ac1746073271fb736dfdb626a89240
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/36c4c667b9f2
BuildID   20131121004002
Version   26.0

Comment 4

[Enpei]

(In reply to Enpei from comment #3)
> 1127 build still has the bug, might be nothing to do with bug 932209.
> 
> 1121 build still has the problem, too. But when setting first 8 message I
> didn't find it, until sending 12 messages then I met it. It's the build
> after bug 929895 is fixed.
Because there are other blocking issues before 1121 that causes unable to reproduce this bug, so probably it's a regression after 929895.
> Gaia:     ce276842c9ac1746073271fb736dfdb626a89240
> Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/36c4c667b9f2
> BuildID   20131121004002
> Version   26.0

Comment 5

[Bruce Huang [:bhuang] (inactive)]

koi+ on this one as it's part of the standard use cases we should be supporting for WAP Push.

Comment 6

[Gabriele Svelto [:gsvelto]]

I'll try to reproduce this today, taking it.

Comment 7

[Kai-Chih Hu [:khu]]

Because 1.2 is going to be finished, should we mark this bug from koi+ to 1.3+? Thanks.

Comment 8

[Gabriele Svelto [:gsvelto]]

(In reply to Kevin Hu [:khu] from comment #7)
> Because 1.2 is going to be finished, should we mark this bug from koi+ to
> 1.3+? Thanks.

It depends on when is the 1.2 deadline. I've managed to reproduce this today and I will be working on a patch to fix the problem. With a bit of luck this should land between tomorrow and Friday.

Comment 9

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH] Prevent messages from being expired when clicking on a notification

This patch fixes an issue which I've seen cause non-expired messages to be marked as expired. In practice what was happening was that clicking on the notification would cause two onclick events to be delivered to the app, the first one retrieved the message and displayed it and the second one tried to retrieve the same message - now deleted from the db - and displayed the "expired message" text as it couldn't find the message. Since this happens in rapid sequence it's usually impossible to glimpse the message being displayed before the "expired message" screen shows up.

Enpei can you apply this to your build and see if the problem goes away?

Comment 10

[Enpei]

Hi Gabriele,

I still got 4 out of 20 messages which has the problem with following build and your patch, could you check the log again?

Gaia      3452fbdb5e1bed0cd27cc6173136537a03e8072f
Gaia      332497cc58aa2dd28da591e4d4c086bdec443e7c
Gecko     89f16b34c24987c1415038b232d0d87f99bde4c5
BuildID   20131212182210
Version   26.0
ro.build.version.incremental=291
ro.build.date=Wed Dec  4 09:48:04 CST 2013

Comment 11

[Enpei]

Attachment: time stamp: 19:17, 19:18, message text are wap 15, 18, 19, 20

Comment 12

[Enpei]

The way I apply patch is not via git am or git apply which all failed(correct line is 192, not 171, not sure if this is the reason). So I manually add following codes in wappush.js
              if (!message.clicked) {
                message.clicked = true;
                app.launch();
                this.displayWapPushMessage(message.timestamp);
              }

and marked // on both app.launch(); and this.displayWapPushMessage(message.timestamp);

Let me know if there is any problem to do so.

Comment 13

[Gabriele Svelto [:gsvelto]]

(In reply to Enpei from comment #12)
> Let me know if there is any problem to do so.

It's OK, then what I fixed is only part of the problem. I will have to disable the auto-close behavior of the application for getting it completely right which is something I hoped I wouldn't have to do. I'll post a new patch shortly. Thank you for your help.

Comment 14

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH v2] Prevent messages from being accidentally dropped

This patch also removes auto-close behavior when in the background in addition to handling double notification events. Enpei can you test this and see if the problem goes away? I also wanted to ask you exactly how are you testing this: are you sending a message, opening it, then sending another one and so on? Or are you sending all the messages in one go and then you open all of them after they've all been received?

Comment 15

[Enpei]

(In reply to Gabriele Svelto [:gsvelto] from comment #14)
> Created attachment 8347104 [details] [diff] [review]
> [PATCH v2] Prevent messages from being accidentally dropped
> 
> This patch also removes auto-close behavior when in the background in
> addition to handling double notification events. Enpei can you test this and
> see if the problem goes away? I also wanted to ask you exactly how are you
> testing this: are you sending a message, opening it, then sending another
> one and so on? Or are you sending all the messages in one go and then you
> open all of them after they've all been received?

Hi Gabriele, I may verify your patch again this weekend or next Monday. For the question you asked, I send all the messages first then open it one by one. Hope this can clarify your question.

Comment 16

[Gabriele Svelto [:gsvelto]]

(In reply to Enpei from comment #15)
> Hi Gabriele, I may verify your patch again this weekend or next Monday. For
> the question you asked, I send all the messages first then open it one by
> one. Hope this can clarify your question.

OK, don't bother re-testing then, I know what's going on :-) This is intended behavior for 1.2 as the messages you see as expired have the same SI-ID [1] of previous messages. This causes the previous messages to be removed and since we've not yet implemented bug 938540 then we're not removing their notifications. Instead when you click on them they show us as expired because we've received a message which we consider 'newer' than them. Because of this I'm closing this as a duplicate of bug 942080.

[1] Those messages do not have an explicit SI-ID but WAP-167-ServiceInd-20010731-a section 5.2.1 mandates that if a message has an HREF attribute but not a SI-ID one then the HREF is used as the SI-ID. So in your test all the messages with the same HREF end up being considered messages with the same SI-ID.

Comment 17

[Enpei]

(In reply to Enpei from comment #15)
> (In reply to Gabriele Svelto [:gsvelto] from comment #14)
> > Created attachment 8347104 [details] [diff] [review]
> > [PATCH v2] Prevent messages from being accidentally dropped
> > 
> > This patch also removes auto-close behavior when in the background in
> > addition to handling double notification events. Enpei can you test this and
> > see if the problem goes away? I also wanted to ask you exactly how are you
> > testing this: are you sending a message, opening it, then sending another
> > one and so on? Or are you sending all the messages in one go and then you
> > open all of them after they've all been received?
> 
> Hi Gabriele, I may verify your patch again this weekend or next Monday. For
> the question you asked, I send all the messages first then open it one by
> one. Hope this can clarify your question.

Thank you for pointing out this, Gabriele. And sorry for not think of this at first place and let you spent so much time debugging it. I will double check if test cases has clear instruction to avoid this problem.