Closed Bug 1029142 Opened 10 years ago Closed 10 years ago

Call log deletions don't persist after killing the app

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Dialer
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(tracking-b2g:backlog)

Status:
RESOLVED DUPLICATE of bug 1061614
Milestone:
2.1 S5 (26sep)
Project Flags:
tracking-b2g backlog

People

(Reporter: kats, Assigned: gsvelto)

References

Details

(Whiteboard: [planned-sprint c=3]) 

STR:
1. Load latest master build on a Flame (I'm using a local build, but I did a full flash and reset gaia, so it should be equivalent to a pvtbuild)
2. Install medium reference workload into Gaia
3. Open dialer app, go to call log. Wait for it to populate
4. Click on the button in top-right to enter the selection screen
5. Click "select all" to select all entries, and then un-select the top 7 entries in the list
6. Hit the delete button. This will take you back to the call log in non-select mode.
7. Wait a while, if desired (I was doing other stuff for about a minute)
8. Long-press home button and kill dialer app
9. Restart dialer and go back into call log

Expected: only the 7 non-deleted entries are listed

Actual: many entries are still present. 28 on my device.
blocking-b2g: --- → 2.1?
It would be nice to get a branch check to see if this existed before 2.1.
Keywords: qawanted, regression
This bug repro's on: Flame 2.1 Master, Flame 2.0, Flame 1.4, Flame v122, Flame v121-2, OpenC 2.1 Master, OpenC 2.0, OpenC 1.4, Buri 2.1 Master, Buri 2.0, Buri 1.4

Actual Results: Some calls in the call log are repopulating after deleting some calls, closing the dialer app and reopening the dialer app.


Environmental Variables:
Device: Flame Master
Build ID: 20140623053744
Gaia: bd5065ced020014df5fd45259fba1ac32d65673b
Gecko: 335b6610fe0c
Version: 33.0a1 (Master)
Firmware Version: v122
--------------------------------------------------
Environmental Variables:
Device: Flame 2.0
Build ID: 20140623051745
Gaia: 84ca0fe0a86d039f6d99cb562f52ef55045dee1d
Gecko: cef223bae66b
Version: 32.0a2 (2.0)
Firmware Version: v122
--------------------------------------------------
Environmental Variables:
Device: Flame 1.4
Build ID: 20140623000201
Gaia: 3419a1f68aaf64a0688685bce42d4173b6125597
Gecko: 34ecc9af3560
Version: 30.0 (1.4)
Firmware Version: v122
--------------------------------------------------
Environmental Variables:
Device: Flame 1.3
Build ID: 20140616171114
Gaia: e1b7152715072d27e0880cdc6b637f82fa42bf4e
Gecko: e181a36ebafaa24e5390db9f597313406edfc794
Version: 28.0 (1.3)
Firmware Version: v122
--------------------------------------------------
Environmental Variables:
Device: Flame 1.3
Build ID: 20140610200025
Gaia: e106a3f4a14eb8d4e10348efac7ae6dea2c24657
Gecko: b637b0677e15318dcce703f0358b397e09b018af
Version: 28.0 (1.3)
Firmware Version: v121-2
--------------------------------------------------
Environmental Variables:
Device: Open_C Master
Build ID: 20140623185652
Gaia: 45479c07cb6ba8c733093d6ee32c767c090c9a28
Gecko: e86b84998b18
Version: 33.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
--------------------------------------------------
Environmental Variables:
Device: Open_C 2.0
Build ID: 20140623173149
Gaia: 9d2f7bd16a8dc0c74c97c5a40d2f0731f3dfff4b
Gecko: f118b45daada
Version: 32.0a2 (2.0)
Firmware Version: P821A10V1.0.0B06_LOG_DL
--------------------------------------------------
Environmental Variables:
Device: Open_C 1.4
Build ID: 20140623000201
Gaia: 3419a1f68aaf64a0688685bce42d4173b6125597
Gecko: 34ecc9af3560
Version: 30.0 (1.4)
Firmware Version: P821A10V1.0.0B06_LOG_DL
--------------------------------------------------
Environmental Variables:
Device: Buri Master
Build ID: 20140623185652
Gaia: 45479c07cb6ba8c733093d6ee32c767c090c9a28
Gecko: e86b84998b18
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
--------------------------------------------------
Environmental Variables:
Device: Buri 2.0
Build ID: 20140623173149
Gaia: 9d2f7bd16a8dc0c74c97c5a40d2f0731f3dfff4b
Gecko: f118b45daada
Version: 32.0a2 (2.0)
Firmware Version: v1.2device.cfg
-------------------------------------------------
Environmental Variables:
Device: Buri 1.4
Build ID: 20140623175951
Gaia: 1d52323cd5b6c7d646f444712c81777d34f74e36
Gecko: 4e9d3d4412f9
Version: 30.0 (1.4)
Firmware Version: v1.2device.cfg
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Contact: croesch
issue is not a regression as it is present in all branches
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Keywords: regression
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
UX may want to input?
Flags: needinfo?(cawang)
6. Hit the delete button. This will take you back to the call log in non-select mode.
7. Wait a while, if desired (I was doing other stuff for about a minute)

From the reproduce steps, users have already pressed delete button and waited a while, the results that the deleted items are still presented is really unreasonable. I think it should be fixed. Thanks.
Flags: needinfo?(cawang)
This is a bug but I'm going to tentatively guess that it will take longer than a day to fix. We can revisit this during sprint planning.
Whiteboard: [planned-sprint c=2]
Target Milestone: --- → 2.1 S2 (15aug)
Assignee: nobody → gsvelto
Whiteboard: [planned-sprint c=2]
Target Milestone: 2.1 S2 (15aug) → ---
triage: non-blocking. this is more like a corner case.
blocking-b2g: 2.1? → backlog
This is not a corner case at all. It's what you would do to delete call log entries. It's a basic functionality offered to the user and it is completely broken which IMO is pretty embarassing to ship with. I'd rather remove the option to delete call log entries entirely.
Flags: needinfo?(whuang)
The step 8 is what makes it a bit of a corner case.

My guess here is that we're just doing one big transaction and it is not committed when the app is killed.
If I switched to another app and the system eventually OOM-killed the dialer, would that be different?
[Blocking Requested - why for this release]: There's disagreement over the corner case assessment.

I agree with Kats, this is a really big problem. Not doing things like this correctly ruins user trust and makes them question if other apps and flows have the same problem, even if they don't. Regardless of whether or not this is a blocker, we should really sit down and fix it when we can. I'm sticking it in the next sprint for now.
blocking-b2g: backlog → 2.1?
Target Milestone: --- → 2.1 S4 (12sep)
Sorry, I wrote "next sprint" but I think I'll actually have time to get to this during this sprint if Gabriele doesn't before me. I imagine the concerns here will be similar to bug 1001467 but with less of a focus on performance.
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)
I was a bit terse here. I meant that without step 8, this would be a clear blocker. With step 8, that means users can leave the phone as is for awhile and have this succeed. Not the best experience for sure but given that we've always shipped with this issue, that is taken into account for the blocking status.

We should fix it though. And when we have a fix, we can consider uplifting it based on the riskiness of the patch.
Ok. Also just to be clear I didn't actually check if step 8 is required or not. It's quite possible that this is just broken in all scenarios; it just so happened that when I was reproducing this I needed to do step 8 in order to trigger the other bug I was actually fixing.
Flags: needinfo?(whuang)
Does Step8 look like a "stop deleting the logs"? Anyways, UX has made the expected behavior clear. 
I just feel this shouldn't blocker if Step8 is required to reproduce the bug.
blocking-b2g: 2.1? → backlog
Whiteboard: [planned-sprint c=3]
Some further investigation on this, I can reproduce with the light reference workload and only deleting the first message in the list. The whole issue is somewhat bizarre in that sometimes the deletion seems successful and sometimes it doesn't but it's not dependant on timing fortunately. I've just grabbed the database because I can't delete the first logged call no matter how much I wait so it makes a nice reproducible case.
Status: NEW → ASSIGNED
The issue seems to reside in the CallLogDBManager logic, there seems to be two issues:

- First of all in CallLogDBManager.deleteGroup() the cursor doesn't return anything in my test case so either the groupId is wrong (and thus we're not finding anything to delete) or something else is going amiss within the DB.

- The second issue is minor but still relevant: as in a lot of other IndexedDB code we've attached the successful callback to the successful handler of the last operation in the transaction. This is wrong as the last operation can be successful but the whole transaction fail, the correct way to do it is to wait for the transaction oncomplete event. This is not causing the issue here but should be fixed too.

As a side thought it would be nice if we had a MVC model here whereas the UI changes would only happen in response to actual data changes; I know it's a lot of work to retrofit it in our existing code but it would have made it apparent that the deletions weren't happening here.
- I've seen issues with getting a group because of variable types. What we get from the DOM are strings but we sometimes need ints for the group id. Maybe it's part of the issue here.

For the side note MVC idea, it has to be treated carefully. Tying our visual changes to our data changes can make the UI feel sluggish while we wait for I/O.
(In reply to Anthony Ricaud (:rik) from comment #18)
> - I've seen issues with getting a group because of variable types. What we
> get from the DOM are strings but we sometimes need ints for the group id.
> Maybe it's part of the issue here.

Yes, that could be part of the problem, I'm seeing issues like this. Trying to delete a call the groupId computed from the dataset is:

[1368655200000,"663-911-7347","dialing"]

But the only entry I could find that matches number and status is the following:

[1368050400000,"663-911-7347","dialing"]

There seem to be some kind of disconnect regarding the date, I'm trying to figure out what's causing it.

> For the side note MVC idea, it has to be treated carefully. Tying our visual
> changes to our data changes can make the UI feel sluggish while we wait for
> I/O.

I know, that's a pretty big topic though it's one I'd like to tackle at some stage.
(In reply to Gabriele Svelto [:gsvelto] from comment #19)
> Yes, that could be part of the problem, I'm seeing issues like this. Trying
> to delete a call the groupId computed from the dataset is:
> 
> [1368655200000,"663-911-7347","dialing"]
> 
> But the only entry I could find that matches number and status is the
> following:
> 
> [1368050400000,"663-911-7347","dialing"]
> 
> There seem to be some kind of disconnect regarding the date, I'm trying to
> figure out what's causing it.
Pretty sure this is bug 1061614.
Interesting, I should double-check with real calls.
This seems to be the same issue as bug 1061614. If the id and the lastEntryDate should match then they don't in a lot of entries. This also explains why I could delete some entries but not others. I'll double-check the migration code.
Quick update: the inconsistencies were caused by a database migration bug, see bug 1061614 comment 4. I think I'll dup this against that bug but I'll also open a follow up to behave a little bit better when dealing with errors in the deletion logic.
This was caused by bug 1061614, filed bug 1071653 to address the related issues we've seen here.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.