Started looking through rjesup & shaver's reflow tree hack. I proposed in the bug that we ought to just do away with the reflow queue and replace it with a `dirty tree' that maintains paths to all the dirty frames. Thinking about that some more (after I posted it) made me realize that there may be some problems. First of all, a reflow may destructively alter the frame tree, invalidating reflow paths. That potentially makes it dangerous to maintain a path across reflows, but presumably that ought not happen (because all reflow events will be processed in a single pass -- unless we start to talk about interruptible reflow). Oh, one case where it may be a problem is in the case of continuations -- but no, I think I fixed that problem before I left with the reflow path fixup crap in the block frame. Other issues could arise where one target frame is the ancestor of another target frame in the reflow tree; however, by making the reflow tree a `global' structure (e.g., accessible from the pres context), we could mitigate those (and fix some hairy bugs in the process).
Need to take a look at kin's fix for some block-in-inline issues.
Started looking at implementing reflow roots.
Filed a
bug
on implementing reflow roots. I'll need to go back and check typing
performance. Also, I'm wondering if I can get rid of the hack
buster put in for a
text invalidation problem.
I've got a hacky version of reflow roots working (where I just mank
up nsHTMLReflowCommand. I'd rather put the smarts into
the nsGfxScrollFrame, so that it targets a reflow at
itself. This will mean that incremental reflows will never (?) end
up in the ViewportFrame, so I'll need to check
elements with fixed positioning.
Okay, so it turns out that if I target incremental reflows at the
nsGfxScrollFrame, we don't properly size the view for
the nsGfxTextControlFrame2. If I target incremental
reflows at the nsGfxTextControlFrame2, we crash
because we can't find a reflow state for the text widget's
containing block. Maybe I can synthesize one?
Boioioing. So it turns out that the real problem was that
I was using the presentation shell's width and height as the
availableSpace in which to do the retargeted
reflow. Fixing that by using the dimensions of the `reflow root'
frame fixed the problem. Put a patch in the
bug.
Fixed a
minor performance problem
with the RDFServiceImpl where it was trying to create
URI objects from `rdf:' specs.
Spent some time thrashing around a topcrash in the rdfliner, but to no avail. I suspect that it may have something to do with the rdfliner's inability to support more than one row with the same URI, but after banging around bookmarks for a half-hour or so, couldn't get it to crash. (Although I did get it to nuke a few bookmarks.) Spent a few hours minimizing a test case for a layout topcrash bug.
Spent some more time staring at the reflow tree hack.
Spent the day banging on the reflow tree hack. Got it to hang together, although not well.
It's too bad this isn't as funny as blake's blog.
I've got my reworked version of rjesup & shaver's
reflow tree hack
working with a few glitches. I just realized that I blew the
maintenance of the
mNumDescendant[Timeout]ReflowsPending member in
nsTableFrame. Trying to figure out how to fix
that...perhaps I can just walk the reflow tree and discount
all of them?
Realized that the
reflow tree stuff
should supercede and obviate the need for the timeout reflow stuff,
so I just ripped it all out. Cut a branch on Friday
(REFLOW_20020412_BRANCH), and made some fixes there
today. Specifically, re-wrote nsBoxReflowState::Unwind
(again!) to properly deal with reflows targeted at nested
boxes. Also, fixed a problem with text fields and initial values:
had to deal with the case where a reflow command gets enqueued
during a reflow (which the old code used to do, but I'd
over-zealoously removed). Fixed leaks of nsReflowPath
objects. Caught up on bugmail and super-reviews and stuff.
One problem I haven't dealt with reflow tree is how to handle mutliple reflows targeted at the same frame.
I think for now the last sounds simplest and safest; we can improve on it if the multiple-reflows-to-the-same-frame problem becomes commonplace.
Fixed a problem in nsViewportFrame::ReflowFixedFrames
where an incremental reflow targeted at the viewport was being
passed to children without modifying the reflow reason. Fixed the
missing resize-reflow problem: we needed to prune the incremental
reflow path in nsBoxToBlockAdaptor after doing the
reflow so that subsequent reflows would be treated as
resize-reflows.
kin and I debugged a reflow root regression: wasn't keeping the frame notification stuff straight, so the table timeout stuff got confused.
dbaron
pointed out a problem
with the block changes for the reflow tree. It turns out that we
need to
fix
nsBlockFrame::RememberReflowDamage so that it accounts
for horizontal changes to floaters.
Spent most of the day yesterday trying to track down a table
incremental reflow bug that appeared with the
reflow tree hack.
It ended up that the
nsTableRowGroupFrame::RecoverState method assumed it
would only be called once per incremental reflow. Also, fixed a
dumb crasher that occured when a fixed frame was the target of an
incremental reflow. dbaron
pointed out
a possible flaw with my evil schemes for recording horizontal
floater damage. I'll need to look at that today.
Looked into a crash bug that ended up being a crappy block-in-inline situation.
Shaver mailed me an overview of the TiVo architecture that I ought to look over sometime. Also a paper about why one ought to use open source software. Possibly interesting with regards to the discussion of JS2 is JANET, a project to do a JS front-end to .NET.
Proposed an
alternative
to RememberFloaterDamage
for dealing with
horizonal expansion of floats.
dbaron pointed out some dumb mistakes with my
patch
to
remove RememberFloaterDamage.
I think I've fixed them. Cleaned up some loose ends for the
reflow tree
and started bugging people about code review. Started to
organize
my various test cases.
So I'm trying to figure out if this reflow retargeting is really
necessary (viz.
nsBlockFrame::RetargetInlineIncrementalReflow).
I'd originally put this code in there to deal with the case where
an incremental reflow is targeted at a nested continuation frames
inside a table cell. Because the table cell asks the block to
compute its max-width, nsBlockFrame::ReflowLine will
do a two-pass reflow of the line, which will destroy the
continuations which were targeted for reflow.
In this situation, RetargetInlineIncrementalReflow
would retarget the reflow to the primary frames along the reflow
path, allowing the incremental reflow to safely arrive at its
target during the first-pass unconstrained reflow. Failure to do
this would cause the incremental reflow to `miss' the target,
typically arriving at the target frame as a resize (or dirty?)
reflow.
This led to bizarre bugs. In the case of text edit frames, the box
code would detect that the size had not changed, and that the box
itself was not dirty, so the incremental reflow would be optimized
away. Hence the every-other-character appearing phenomena: the
first reflow would mark the anonymous <div>
dirty and be optimized away; the second reflow would be rolled up
to the parent box (because the <div> was still
dirty) and so the box would be marked as dirty and propagate the
reflow. This problem is no longer an issue now that text edit
frames are `reflow roots'.
In a world where inlines can only contain inlines or reflow roots, I think the answer is `we don't need this mechanism anymore'. In the former case, things will `come out in the wash' because of the way the reflow descends through all of the inline frames in each line. In the latter case, it's a reflow root, so the problem is defined away. This is the world we're in now (I think).
But this may be a problem again if we implement
display: inline-block, or if we end up with
inline box frames that are not reflow roots, possibly due to XBL
form controls. For example, a <select> will
change size if you dynamically add an <option>
that is wide enough, hinting that these _can't_ be reflow roots
(because the growth of the <select> affects the
flow around it).
So I think I've managed to convince myself that this code is necessary, although not immediately so. (So I may not get around to upgrading it right away since there is no urgent need.)
dbaron pointed out that, with the reflow tree, it ought to be possible to `mask' a style change reflow by scheduling a dirty reflow at an ancestor frame. Here's a test case that tries to do just that: oddly, it works. Why?
Well, it turns out that with a dirty incremental reflow, stuff just
works: the dirty incremental reflow leaves the reflow reason
untouched in nsBlockReflowContext::ReflowBlock, so the
reflow continues to proceed through the frame hierarchy as
incremental. I've managed to concoct another
test case
that demonstrates how to break stuff with a incremental style
changed reflow, though. In this case, we target two style changed
reflows at the frame hierarchy: one at the outer
<div>, and one at a frame nested two levels
down. (This is the test case that dbaron was originally trying to
get at, I think.)
After the reflow tree has landed, I ought to:
ContentChanged incremental reflow type. The
only code that responds to this is the block frame (which does a
PrepareResizeReflow): we could (and probably should)
handle this by just marking all of the lines dirty in
nsBlockFrame::ContentChanged.
UserDefined incremental reflow
type. The only code that responds to this is the
ViewportFrame: it reflows its fixed frames.
eReflowReason_Dirty reflow reason. I
think this can be subsumed by
eReflowReason_Incremental and a check to the
nsHTMLReflowState's path.
Other stuff:
WONTFIX-ed a
float ``bug''
that has to do with over-estimating the max-element-size.
<div>. This is the
second time
I've seen this happen...do we need a quirk?
nsSpaceManager objects.
text-indent is not included in the MES computation.
It's not straight-forward to fix correctly, but it's probably not
too hard to fix with a jack-hammer: just add the text-indent to
the MES for the first line -- overestimates it a bit, but...close
enough for government work.