Closed Bug 1298968 Opened 5 years ago Closed 5 years ago

Add comment explaining why TOPSITES query is handled outside of switch in BrowserProvider

Categories

(Firefox for Android Graveyard :: Data Providers, defect)

defect
Not set
normal

Tracking

(firefox51 fixed)

RESOLVED FIXED
Firefox 51
Tracking Status
firefox51 --- fixed

People

(Reporter: ahunt, Assigned: ahunt)

References

Details

(Whiteboard: [MobileAS])

Attachments

(1 file)

In BrowserProvider.query(), most of the database queries are handled in a large switch(match) statement.

The TOPSITES query is handled separately, at the beginning of this method: the reason for this is that the TOPSITES query requires a writable database (because of the temporary tables), whereas all the other queries are happy with a readable database: we handle TOPSITES specially and let it obtain a writable DB itself, for the other queries we obtain the readable DB within query() and then pass that readable DB to the query methods within the switch.

See:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java?q=BrowserProvider.java&redirect_type=direct#

We should add a comment explaining this directly to BrowserProvider.java.
Assignee: nobody → ahunt
Whiteboard: [MobileAS]
Depends on: 1293790
Removing an erroneously added dependency (that was meant for another bug)/
No longer depends on: 1293790
Blocking on Bug 1293710 so I don't cause unnecessary merge conflicts!
Depends on: 1293710
Comment on attachment 8788537 [details]
Bug 1298968 - Add comment explaining why TOPSITES is handled outside of switch

https://reviewboard.mozilla.org/r/76998/#review75130
Attachment #8788537 - Flags: review?(s.kaspari) → review+
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/243fe9c65a04
Add comment explaining why TOPSITES is handled outside of switch r=sebastian
https://hg.mozilla.org/mozilla-central/rev/243fe9c65a04
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.