300030 - (reflow-refactor) Refactor intrinsic width computation out of nsIFrame::Reflow

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug describes a design change that I've been talking about for a
while (see various posts to mozilla-layout).  Many bugs likely to be
fixed by this change have "[reflow-refactor]" in the Status Whiteboard.
I've been working on the changes on various REFLOW_{YYYYMMDD}_BRANCH
branches.

The basic idea to make two closely related changes at the same time:
 1. Change intrinsic width computation so that it happens in separate
    methods on nsIFrame.  (Currently preferred with calculation using
    the nsIFrame API can happen in two different ways as well:  having
    only one codepath reduces incremental layout ("{inc}") bugs.)
 2. Change the way we handle incremental layout to use dirty bits on the
    frames rather than the reflow tree (which is essentially out-of-line
    dirty bits) and a complicated system of reflow reasons that often
    leads to weird incremental layout ("{inc}") bugs.

I hope to add more details of the new design here in the near future.
Much of it is still open to change; at this point the branch is close to
compiling and probably a ways from working, so the cost of changing
things is still relatively low.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What have you decided to do about the intrinsic width of scrollframes?
Currently, we do a full reflow of the scrolled content to see if we need a
horizontal scrollbar, which helps determine the intrinsic width.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://groups.google.com/group/netscape.public.mozilla.layout/msg/c80282ffff8047c3
has comment 0 in a tad more detail; I should really write more sometime.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://wiki.mozilla.org/Gecko:Reflow_Refactoring has been around for a while; it's been in the URL field but I should mention it in a comment.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

For the record, I've discussed landing plans with bzbarsky and roc, and they're comfortable with this landing without complete review.  I know bzbarsky has been looking at it a good bit (including having written almost all of the form control changes).

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch as landed

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

diffstat says:
 201 files changed, 8726 insertions(+), 18253 deletions(-)

tinderbox numbers from Firefox tinderbox:

Linux argo-vm Dep Nightly:
  Z:14.08MB, Zdiff:-53276 (+57545/-110821)
balsa-trunk:
  RLk and Lk unchanged
  MH down from 13.1MB to 13.0MB
  A down from 486K to 485K
Linux bl-bldlnx01 Dep argo-vm test perf:
  Tp: 3 test mean fell from 704.3333 to 682.6667, 3%, p-value = 0.01854
  Tp2: 3 test mean fell from 530.8917 to 507.2333, 4.5%, p-value = 0.01977
  Tdhtml: 3 test mean fell from 1300.667 to 1290.667, 0.75%, p-value = 0.00748

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: some detailed pageload test data

This was some detailed pageload test data that I did on Windows a few days before the branch landed.  (I should probably try to get the equivalent out of the tinderbox logs.)  It showed that the pageload improvements varied significantly between pages.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since I occasionally want to find it and it takes me a few minutes to dig up, I figure it's worth noting that the in gecko-dev, the landing of this branch was 31f18988