1827329 - Fix layout/base/tests/test_zoom_restore_bfcache.html on Android Fission

Status: RESOLVED FIXED

(GeckoView Graveyard :: Sandboxing, task, P1)

Description

[[:owlish] 🦉 PST]

Mochitest-plain variant. All builds.

e.g. https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=da4dc488078970e3e60ef6964e275f46ba9c456e&selectedTaskRun=B4-vs9PpSnindYlnA43SAw.0

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: logcat log

I've attached a snippet of the logcat log for the run of this test. It appears the failure is caused by some issue with zoom not being correctly restored during BFCache navigations on android. IIRC the zoom infrastructure is somewhat controlled by the frontend right now, so there's a chance that we need to make some changes on the Android side to align with the changes we did on Desktop.

I believe this check is the one which is failing: https://searchfox.org/mozilla-central/rev/11dbac7f64f509b78037465cbb4427ed71f8b565/layout/base/tests/test_zoom_restore_bfcache.html#54

Leaving a ni? for :emilio to potentially look into this, as he's more familiar than I am with the zooming logic.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1827329 - Keep full/text zoom across bfcached navigations. r=nika,smaug

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Does something like the attached patch help? I think in desktop this works because of this code.

Comment 4

[[:owlish] 🦉 PST]

I tried the change in the patch locally - I still get the same error when I run the test with Fission.

Comment 5

[[:owlish] 🦉 PST]

Apologies, looked closer - the initial error was FAIL Should preserve zoom when restored - got 1, expected 2, but with the change in the patch I get UNEXPECTED-PASS Should preserve zoom on frames too - 2 should equal 2

Comment 6

[Chris Peterson [:cpeterson]]

(In reply to [:owlish] 🦉 PST from comment #5)

Apologies, looked closer - the initial error was FAIL Should preserve zoom when restored - got 1, expected 2, but with the change in the patch I get UNEXPECTED-PASS Should preserve zoom on frames too - 2 should equal 2

Irene, that sounds like Emilio's patch fixed the bug and we just need to update the test expectation in the patch.

If you change todo_is to is in the test, do the rest of the tests pass? And do the tests still pass on desktop with Emilio's patch?

https://searchfox.org/mozilla-central/rev/32c74afbb24dce4b5dd6b33be71197e615631d71/layout/base/tests/test_zoom_restore_bfcache.html#66

todo_is(msg.frameDevicePixelRatio, originalDPR * 2, "Should preserve zoom on frames too");

Comment 7

[[:owlish] 🦉 PST]

Oh, yes - when I change that line the way you said, the test passes! Excellent!

Emilio, can we have a non-WIP patch please?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cc4558be019
Keep full/text zoom across bfcached navigations. r=smaug

Comment 9

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/0cc4558be019