551258 - Refactor: remove batch operations from BrowserView

Status: RESOLVED INVALID

(Firefox for Android Graveyard :: General, defect)

Description

[Benjamin Stover (:stechz)]

Attachment: v1

This patch removes batch operations completely from BrowserView.  IMO, batch operations are one of the trickiest things to read in our codebase due to A) it unnecessarily keeps a stack B) has a confusing stacktrace when _viewportChanged is called while a batch operation is happening and C) required a lot of helper functions like forceViewportChange that further increased its complexity.

Another big change is that the tile manager no longer critical paints on a viewport change.  Callers of setZoomLevel now must tell Fennec when to critical paint, which helps to make sure that the critical paint happens only after the scroll position has been changed.  This actually fixes a performance issue with switching tabs where it was possible to critical paint two different areas at once.  Also, the zooming code is less intimidating now.

The replacement for this functionality comes in the form of Coalescer, which gathers dirty rects and size changes and ultimately applies them to BrowserView once every 2 seconds during load.  It is less ambitious than batch operations ever was, so it fits in a neat little object.  Coalescers are per-tab, so loading background tabs don't stop paints for a loaded tab in the foreground.

This refactor has made the purpose of BrowserView clearer in my eyes: browser view is responsible for handling zoom operations and viewport changes to the *current* viewport.  If BrowserView is tackling things that aren't about the current view, it probably shouldn't be.

Some of the utility functions were moved to Util.js, in anticipation of including Util.js in both content and chrome processes in e10s.

I've tried on mobile and watched for noticable regressions in page loading and startup, and things look pretty similar.

Comment 1

[Benjamin Stover (:stechz)]

Comment on attachment 431439 [details] [diff] [review]
v1

Ignore the XXX in util.js--I added that as a reminder to remove redundant getContentScrollOffset and getBrowserDOMWindowUtils, which I did.

Comment 2

[Benjamin Stover (:stechz)]

No longer relevant