[email/POP3] Make FolderStorage.sliceOpenMostRecent not trigger connection establishment for POP3 folders other than the inbox; clean up tests

RESOLVED FIXED

Status

Firefox OS
Gaia::E-Mail
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: asuth, Assigned: mcav)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
For bug 945544 I am implementing emptying the trash for POP3.  While writing the unit test I noticed that we are attempting to establish a connection when opening the trash folder.  This occurs as a result of Pop3FolderSyncer.sync using the lazyWithConnection wrapper.  Even though inside the actual sync implementation it knows to do nothing with the server, it's too late by that point.

Trivial fixes will be insufficient because the sync method does the horrible _TEST_pendingAdds and _perofrmTestDeletions logic in there which are essential for many of the tests shoe-horned to work with POP3.

I think there may be some bad efficiency issues since lazyWithConnection for sync passes getNew=true which means we will get the connection just to discard it in another folder.

Note that this bug does not affect localdrafts because it is specially handled by FolderStorage._sliceOpenMostRecent.  Stop-gap solutions may be possible by just making that method more aware of POP3's special-ness for the purposes of the trash folder.

I'm proposing we mark this as a 1.3 blocker since having the trash folder request a connection could interfere with the correct operation of pending inbox syncs, etc.  :mcav would be most qualified to make a determination of how serious a problem this is, though.
(Assignee)

Comment 1

5 years ago
I think this can be addressed without too much fuss. I'll whip up a patch and we can take a look to make sure it addresses this fully.
Assignee: nobody → mcav
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 8341864 [details]
GELAM Pull Request

This patch would force lazyWithConnection to only establish a connection if the folder is the inbox. The title of this bug, though, seems to indicate that you thought a more involved fix was necessary -- so it's possible I'm missing something.
Attachment #8341864 - Flags: review?(bugmail)
(Reporter)

Updated

5 years ago
Attachment #8341864 - Flags: review?(bugmail) → review+
(Reporter)

Comment 4

5 years ago
(landed in 1.3 as non-feature bugfix prior to branching, no further triage required)
blocking-b2g: 1.3? → ---
You need to log in before you can comment on or make changes to this bug.