1202020 - Remove dependencies on async pan/zoom information from Java code

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Toolbar, defect, P5)

Description

[Kartikaya Gupta (email:[e-mail])]

Until now the Java code has had access to the ImmutableViewportMetrics which provides it with information like the current zoom and scroll position. There are various bits of code that use this information to find out the screen position of content elements, in order to sync Java UI widgets with page content. As far as I can tell the bits of code that rely on this behaviour are: (1) the FormAssistPopup, (2) the TextSelectionHandle, and (3) the ZoomedView.

(1) and (2) both use a combination of browser.js and Java code to compute the final screen position - browser.js computes a document-relative CSS position, and then the Java side uses the ImmutableViewportMetrics to convert that into a screen position. See code at [1] and [2] for an example of this works.

This half-and-half combination allows the code to work in the face of async panning/zooming and also with subdocuments. browser.js code will resolve the scroll positions of iframes/divs, and the Java code will take care of the top-level document's pan/zoom position. However, as we move to the C++ APZ, which has async subdocument scrolling, this will no longer be possible. The browser.js code will have out-of-date scroll positions for iframes because the scroll position can be changed in the compositor.

This means that either the Java code needs to know the async scroll metrics for everything, or (preferably) we need to make it work without having to know the async scroll metrics at all.

Already the FormAssistPopup automatically hides and shows so that it is only visible while the user is not actively panning/zooming. This is great because it can allow us to eliminate the dependency on scroll metrics. The text selection handles can be handled similarly, or if/when we switch to the Gecko selection carets the problem should be taken care of automatically since Java won't need to do anything.

The ZoomedView case I haven't looked into in as much detail but conceptually I see no reason it should need to know the async scroll/zoom if it routes input events through the APZ code.

This bug tracks the work needed to make all this happen.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=6605bf7174c8#5737
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/FormAssistPopup.java?rev=32f284b4aeac#291

Comment 1

[Randall Barker [:rbarker]]

Now that Bug 1335895 has landed I'm not sure how much more can be removed? It might be possible to remove even the steady state updates of the FrameMetrics information?

Comment 2

[Kartikaya Gupta (email:[e-mail])]

There's still a bunch of stuff that can be removed - for one thing there's a now a bunch of fields in the ImmutableViewportMetrics that aren't used at all, so those can all go. Based on my quick look at the code there's only 4 fields that are still used. There's also some JNI goop related to the IVM that's not needed any more, and we can rip that out.

Comment 3

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

ImmutableViewportMetrics isn't a thing any more.