1060865 - SMS filtering with startDate/endDate broken

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[:gerard-majax]

After bug 878533 landed, my patch (bug 887198 attachment 8475080 [details] [diff] [review]) for limiting the amount of requested messages to only the last month broke.

As far as I could analyze, on Gecko side in MobileMessageDB I'm getting invalid/shifted values for the filter content.

If I specify both startDate and endDate:
> 08-30 23:38:03.995  1862  1862 I Gecko   : MobileMessageDB: Creating a message cursor. Filters: startDate: 1409434683453 endDate: 52614631488 delivery: null numbers: (null) read: (null) threadId: 3 reverse: true

If I specify only startDate:
> 08-30 23:37:23.643  1862  1862 I Gecko   : MobileMessageDB: Creating a message cursor. Filters: startDate: 0 endDate: (null) delivery: null numbers: (null) read: (null) threadId: 3 reverse: true

Please note the behavior of startDate and endDate in those logs:
 - when we specify both, the startDate values is in fact the current timestamp, and endDate just makes no sense
 - when we specify only startDate, its value is 0 and the endDate stays null

Comment 1

[:gerard-majax]

Log from MobileMessageManager.cpp, just before the dbService->CreateMessageCursor() call:
> 08-31 00:14:36.841  4476  4476 I Gecko   : Calling: CreateMessageCursor: hasStartDate=1
> 08-31 00:14:36.841  4476  4476 I Gecko   : Calling: CreateMessageCursor: startDate=1407017676782
> 08-31 00:14:36.841  4476  4476 I Gecko   : Calling: CreateMessageCursor: hasEndDate=1
> 08-31 00:14:36.841  4476  4476 I Gecko   : Calling: CreateMessageCursor: endDate=1409436876782

Log from MobileMessageDB:
> 08-31 00:14:36.849  4187  4187 I Gecko   : MobileMessageDB: Creating a message cursor. Filters: startDate: 1409436876782 endDate: 13730751496340730000 delivery: null numbers: (null) read: (null) threadId: 3 reverse: true

Comment 2

[:gerard-majax]

Attachment: Tracking SmsFilterData effect

With this patch applied, I can see the following:

> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: CreateMessageCursor: hasStartDate=1
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: CreateMessageCursor: startDate=1407019168633
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: CreateMessageCursor: hasEndDate=1
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: CreateMessageCursor: endDate=1409438368633
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: IPCCreateMessageCursor: aHasStartDate=1
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: IPCCreateMessageCursor: aStartDate=1407019168633
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: IPCCreateMessageCursor: aHasEndDate=1
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: IPCCreateMessageCursor: aEndDate=1409438368633
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: SmsFilterData CreateMessageCursor: hasStartDate=1
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: SmsFilterData CreateMessageCursor: startDate=1409438368633
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: SmsFilterData CreateMessageCursor: hasEndDate=1
> 08-31 00:39:28.695  4918  4918 I Gecko   : Calling: SmsFilterData CreateMessageCursor: endDate=4615148482524188380

So it would look like the assignments at http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/ipc/SmsIPCService.cpp#238 are triggering the issue: values passed to the method are okay.

Comment 3

[:gerard-majax]

Oh god. That was SO obvious:

> data.startDate() = aStartDate;
http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/ipc/SmsIPCService.cpp#239

And then:
> data.startDate() = aEndDate;
http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/ipc/SmsIPCService.cpp#241

Comment 4

[:gerard-majax]

[Blocking Requested - why for this release]: API broken

Comment 5

[:gerard-majax]

Attachment: Fix end date filling in SmsFilter

Copying the endDate value in the startDate field won't make the filter
working properly ; in fact, it will even crash the Messages app
sometimes because of the uninitialized value of the endDate which is
invalid and triggers a DataError within the MobileMessageDB.

Comment 6

[:gerard-majax]

Comment on attachment 8481902 [details] [diff] [review]
Fix end date filling in SmsFilter

Vicamo, please find attached a patch that fixes the issue. I ran a try run against B2G Emulator ICS with Marionette, https://tbpl.mozilla.org/?tree=Try&rev=fd9affb28c5a&showall=1 it seems to be green but I don't see any marionette tests being ran.

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8481902 [details] [diff] [review]
Fix end date filling in SmsFilter

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

Sorry for that mistake :(

Comment 8

[:gerard-majax]

Attachment: Fix end date filling in SmsFilter

Copying the endDate value in the startDate field won't make the filter
working properly ; in fact, it will even crash the Messages app
sometimes because of the uninitialized value of the endDate which is
invalid and triggers a DataError within the MobileMessageDB.

Comment 9

[:gerard-majax]

I had to rebase this patch because bug 1058101 landed and moved all the files ...

Comment 10

[:gerard-majax]

Comment on attachment 8482151 [details] [diff] [review]
Fix end date filling in SmsFilter

Carrying r+ from vicamo

Comment 11

[:gerard-majax]

Comment on attachment 8481902 [details] [diff] [review]
Fix end date filling in SmsFilter

Obsolete because of bug 1058101

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fb36560d80c

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7fb36560d80c