1528509 - GeckoView state saving is under-tested

Status: NEW

(GeckoView :: General, task, P3)

Description

[Jan Henning [:JanH]]

At the moment, there is a single basic sanity test, however significant parts of Fennec's session store tests (test_session_form_data.html, test_session_scroll_position.html and test_session_scroll_visual_viewport.html) cover additional functionality that would still be relevant even for GeckoView, e.g. testing with frames, exercising saving and restoring the back-forward session history, especially in conjunction with the scroll position, saving and restoring the pinch-zoom, specifically testing the visual viewport as opposed to the layout viewport, etc.
While the above tests are written to be Fennec-specific and aren't e10s-ready, either and therefore couldn't be re-used 1:1, the general logic of what they test should probably still be applicable to GeckoView's state saving/restoring, too, so it would be good if they could in some way be ported to cover GeckoView, too.

If bug 1463878 gains traction, it would also be good to have additional tests that ensure that the planned session store data cache is correctly updated after any relevant actions (GeckoSession creation/destruction, navigation, form data input, scrolling, pinch-zooming, etc.), and having those extended GeckoView tests would therefore be good for that, too.

Comment 1

[Jan Henning [:JanH]]

Granted, some of these things will be covered by equivalent desktop tests, too, but especially pinch-zoom and the visual viewport for example aren't.

Comment 2

[Chris Peterson [:cpeterson]]

Assigning to Dylan because he said he would add more state-saving tests as part of bug 1463878.

Comment 3

[Chris Peterson [:cpeterson]]

saveState API bug 1463878 is scheduled for Fenix M3, so let's schedule these saveState tests for M3, too.

Comment 4

[Chris Peterson [:cpeterson]]

67=wontfix. Fenix MVP will use GeckoView 68, so we don't need to uplift this fix to 67 Beta.

Comment 5

[Chris Peterson [:cpeterson]]

Dylan says this doesn't need to be an M3 bug. We have basic tests already.

Comment 6

[Jan Henning [:JanH]]

The planned-for Fennec removal is getting ever so closer, which means we'd be left without proper test coverage for significant parts of the back-end of our state saving infrastructure.

Comment 7

[Chris Peterson [:cpeterson]]

I'm editing a bunch of GeckoView bugs. If you'd like to filter all this bugmail, search and destroy emails containing this UUID:

e88a5094-0fc0-4b7c-b7c5-aef00a11dbc9

Comment 8

[[ex-Mozilla] Agi Sferro | :agi]

Dylan do you think this should stay as P2?

Comment 9

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

(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #8)

Dylan do you think this should stay as P2?

We should certainly improve it at some point, but I don't have the cycles to work on it this sprint. Maybe flag it for the next sprint and I can take a look (hopefully after getting us to use SessionStateListener etc.)?

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:amoya, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 11

[Chris Peterson [:cpeterson]]

Is GeckoView state saving still under-tested?

Comment 12

[Chris Peterson [:cpeterson]]

Nice to have more tests. This would be a good intern project.