Closed Bug 1456260 Opened Last year Closed Last year

Support concurrent GeckoSession.saveState() calls

Categories

(GeckoView :: General, enhancement, P1)

All
Android
enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: droeh, Assigned: droeh)

Details

(Whiteboard: [geckoview:klar])

Attachments

(1 file)

Currently we log a warning and return a null saved state if GeckoSession.saveState() is called while we are already in the middle of saving state; it would be better to fully support this case.
Assignee: nobody → droeh
Priority: -- → P1
Whiteboard: [geckoview+klar]
Whiteboard: [geckoview+klar] → [geckoview:klar]
Comment on attachment 8971618 [details] [diff] [review]
Support concurrent saveState() calls

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

This is okay, though we should really only send one "GeckoView:SaveState" message at a time, and return the same state to all pending `saveState()` calls.

Please file a follow-up bug about cleaning up callbacks if the child disconnects unexpectedly, in which case we won't receive "GeckoView:SaveStateFinish".
Attachment #8971618 - Flags: review?(nchen) → review+
(In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> This is okay, though we should really only send one "GeckoView:SaveState"
> message at a time, and return the same state to all pending `saveState()`
> calls.

I thought about going this route, but isn't it possible for the state to have changed between when the two GeckoSession.saveState() calls are made?
It's all asynchronous so it doesn't really matter if the state changes in the meantime. Even in the current case, the state could have changed between the child sending the message and the parent receiving the response.
Fair enough. I'll address that in the follow up, as I'm on PTO next week and don't want to leave this hanging over a minor issue.
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62eb4b67db48
Support concurrent GeckoSession.saveState() calls. r=jchen
https://hg.mozilla.org/mozilla-central/rev/62eb4b67db48
https://hg.mozilla.org/mozilla-central/rev/f7289b97cdf1
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 61 → mozilla61
You need to log in before you can comment on or make changes to this bug.