TabManager.storeChanges called twice during restore

RESOLVED FIXED

Status

()

Firefox for iOS
Data Storage
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: rnewman, Assigned: fluffyemily, Mentored)

Tracking

({perf})

unspecified
All
Unspecified

Firefox Tracking Flags

(fxios+)

Details

(Whiteboard: [good second bug][lang=swift])

Attachments

(1 attachment)

47 bytes, text/x-github-pull-request
sleroux
: review+
sleroux
: feedback+
Details | Review | Splinter Review
(Reporter)

Description

3 years ago
This ultimately hits the DB twice as much as necessary.
(Reporter)

Comment 1

3 years ago
Ah. It's actually called once per entry in history during restore!

    func webView(webView: WKWebView, didFinishNavigation navigation: WKNavigation!) {
        hideNetworkActivitySpinner()
        storeChanges()
    }

And even then it's wrong, seemingly missing history.
Culprit of slow startup? Ought to track, no?
tracking-fxios: --- → ?
Keywords: perf
(Reporter)

Updated

3 years ago
tracking-fxios: ? → +
(Reporter)

Comment 3

3 years ago
Triage explanation: plus for this because it affects startup. Not plus for Bug 1187176, even though we'd love a fix.
(Assignee)

Updated

3 years ago
Assignee: nobody → etoop
(Assignee)

Comment 4

3 years ago
Created attachment 8642940 [details] [review]
Pull request
Attachment #8642940 - Flags: review?(sleroux)
Comment on attachment 8642940 [details] [review]
Pull request

Fixes look good but left a comment on resolving the issue pointed out in this bug.
Attachment #8642940 - Flags: feedback+
Attachment #8642940 - Flags: review?(sleroux) → review+
(Assignee)

Comment 6

3 years ago
I think the issue with storeChanges() getting called too often overall and the fact that storeChanges() is pretty heavy in it's implementation (delete all tabs from the db, create them all again AND write tabs to disk) will all come under https://bugzilla.mozilla.org/show_bug.cgi?id=1187176 with this one being purely about slow startup performance
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.