Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Fennec with C++ APZ sometimes get stuck in a state with scrollbars in the top-left and zooming not working properly

RESOLVED FIXED in Firefox 45

Status

()

Firefox for Android
Graphics, Panning and Zooming
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
Firefox 45
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(2 attachments)

While testing with C++ APZ I'm able to reproduce (somewhat intermittently) a scenario where upon zooming in, the page remains low-res. If I scroll at this point so that the dynamic toolbar goes on or off, the page immediately zooms back out. Also the scrollbars end up positioned on a fictitious viewport area in the top-left corner of the screen. We saw this on nytimes.com as well a few times last week.

What I'm seeing when it gets into this state is that the presShell resolution has been reset to 1.0 somehow (not yet sure how) and so when APZCCH gets a repaint request from the APZ, it doesn't update the resolution because there's a resolution mismatch [1]. The root cause here is the resolution getting set to 1.0 out of band, not yet sure why that's happening.

[1] https://dxr.mozilla.org/mozilla-central/rev/3f5afaf4e6b72c4b1a20749b4ce7d945add5299f/gfx/layers/apz/util/APZCCallbackHelper.cpp#219
It looks like the resolution is being read from the root scrollframe rather than the presShell [1], and the root scrollframe's resolution is 1.0. It actually gets set to the correct resolution initially but then we throw away that scrollframe and create about 40 new ones in quick succession which all have a 1.0 resolution. I think we might need to have the root scrollframe pick up the resolution from the presShell on creation. Still digging.

[1] https://dxr.mozilla.org/mozilla-central/rev/3f5afaf4e6b72c4b1a20749b4ce7d945add5299f/layout/base/nsLayoutUtils.cpp#8450
So yeah, picking up the resolution from the presShell in the ScrollFrameHelper constructor fixes the problem. However storing the resolution in two different places seems unnecessary and redundant. IIRC this was added for fennec to be able to restore the zoom when going back (so that the zoom is saved along with the scroll position in the history) but maybe we can find a better way to do this so it doesn't duplicate state which can get out of sync.
Created attachment 8690471 [details] [diff] [review]
Part 1 - Don't keep a copy of the resolution in the root scrollframe

This stuff was added back in bug 611556 to be able to save the resolution when navigating back to a page. However, having two copies of the resolution means that they can get out of sync, which is what was causing the bug here. We can either get rid of the resolution from the presshell (which is what was discussed in that bug) or get rid of it from the scrollframe (which is what I'm doing here). The reason I chose to do this was because our current architecture is more consistent with the resolution being a property of the presshell; additional resolution-related state is also currently on the presshell. Also I'm not sure there's much value is supporting zoom on arbitrary scrollable elements; it just feels cleaner to only allow it per-presshell.
Attachment #8690471 - Flags: review?(roc)
Created attachment 8690472 [details] [diff] [review]
Part 2 - Remove nsLayoutUtils wrappers
Attachment #8690472 - Flags: review?(roc)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> It
> actually gets set to the correct resolution initially but then we throw away
> that scrollframe and create about 40 new ones in quick succession which all
> have a 1.0 resolution.

Just to clarify the "40 new ones" wasn't accurate - we only re-created the root scrollframe once, the rest of the scrollframes I was seeing were not root scrollframes. So the problem here was that:
- the presShell and root scrollframe were created (with 1.0 resolution)
- a 0.39 resolution was set on the presshell and root scrollframe
- the root scrollframe was destroyed a new one was created, with a 1.0 resolution
- no saved state was found to restore onto the new root scrollframe

This left things in a state where the presShell had resolution 0.39 and the root scrollframe had resolution 1.0 which screwed up a lot of stuff.
Try push is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bcae751db8e
Attachment #8690471 - Flags: review?(roc) → review+
Attachment #8690472 - Flags: review?(roc) → review+

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/35ed8d9de74e
https://hg.mozilla.org/integration/mozilla-inbound/rev/4347df8d3bcf

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/35ed8d9de74e
https://hg.mozilla.org/mozilla-central/rev/4347df8d3bcf
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Depends on: 1228357
You need to log in before you can comment on or make changes to this bug.