1456260 - Support concurrent GeckoSession.saveState() calls

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P1)

Description

[Dylan Roeh (:droeh) (he/him)]

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.

Comment 1

[Dylan Roeh (:droeh) (he/him)]

Attachment: Support concurrent saveState() calls

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

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".

Comment 3

[Dylan Roeh (:droeh) (he/him)]

(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?

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

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.

Comment 5

[Dylan Roeh (:droeh) (he/him)]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62eb4b67db48
Support concurrent GeckoSession.saveState() calls. r=jchen

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7289b97cdf1
Fix eslint failure. r=me

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/62eb4b67db48
https://hg.mozilla.org/mozilla-central/rev/f7289b97cdf1