10818 - [RFE] Implement batched reflows.

Status: VERIFIED FIXED

(Core :: Layout, enhancement, P3)

Description

[Chris Waterson]

Per our (vidur/nisheeth/waterson/hyatt) discussion today about batching reflows
from RDF and mail/news, I'm logging this bug. For context, see
news://news.mozilla.org/37A0A56F.652A084A%40netscape.com.

Comment 1

[Nisheeth Ranjan]

Accepting bug and setting milestone to M11 for now...

Comment 2

[Nisheeth Ranjan]

Accepting bug and setting milestone to M12...

Comment 3

[Nisheeth Ranjan]

I accidentally made the earlier comment.  It was meant for another bug
that I wanted to fix in M12.  Chris, is this bug a beta stopper?  I know you
worked around the performance problem in the tree widget.  If at all possible,
I'd like to move this out beyond beta.

Comment 4

[Chris Waterson]

yeah, i think that's ok.

Comment 5

[Nisheeth Ranjan]

Setting target milestone to M15...

Comment 6

[Nisheeth Ranjan]

Moving this in to M14.  I'm going to add methods for batching reflows on the 
pres shell.  

Comment 7

[David Hyatt]

You're my hero, stoner. :)

Comment 8

[Nisheeth Ranjan]

Man, if this name sticks, I'm never gonna get a job *anywhere* else!  :)

Comment 9

[Nisheeth Ranjan]

Attaching a patch that adds batching to the PresShell.  Could one of you please 
code review it.  Thanks!

Comment 10

[Nisheeth Ranjan]

Attachment: Patch to batch reflows

Comment 11

[troy]

Hey that's great, Hyatt's calling you stoner now, too!

Comment 12

[troy]

Looks okay, but I have a couple of suggestions:

- maybe we should name them BeginBatchingReflows/EndBatchingReflows instead? I 
think that's the terminology we use elsewhere, e.g., the document observer API
- IsBatchingReflows() might better be GetReflowBatchingStatus(). That would give 
you more flexibility to add additional status down the line

You need to define how the semantics for batching works. In particular, the 
usage model. The way it is now it's meant for a single user at a time and 
the begin/end batching calls should not occur across a flow of control.

Things will get wacky if someone begins batching, leaves the batching open, then 
 a different client (like the HTML content sink) either begins batching or 
starts adding reflow commands thinking they will be processed immediately

At a minimum, for the time being StartBatchingReflows() should probably assert 
that mBatchReflows is FALSE

Comment 13

[Nisheeth Ranjan]

I've incorporated Troy's suggestions into the patch and will check it in today 
once the tree opens.

Comment 14

[Nisheeth Ranjan]

I just checked this in.  Marking fixed.

Comment 15

[Chris Petersen]

Marking verified per last comments.

Comment 16

[Brant Gurganus]

[RFE] is deprecated in favor of severity: enhancement.  They have the same meaning.