Closed Bug 330302 Opened 18 years ago Closed 18 years ago

flash of white instead of body background during page load

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: dbaron, Assigned: dbaron)

References

(
URL
)

Details

(Keywords: regression) 

This regressed between Linux Firefox nightlies 2006-03-08-08-trunk and 2006-03-09-05-trunk.

Steps to reproduce:
 1. load http://dbaron.org/home
 2. press reload

Expected results: background remains green during the reload process

Actual results: background turns white at one point during the reload process, when the page paints with everything present except for body's background.

The only checkin that seems remotely related is bug 326645.
I confirmed via local backout that this was caused by bug 326645.
Flags: blocking1.9a1+
Printfs show that the only place GetBodyContent (and GetRootContent) are returning null is when repainting the old document, presumably during paint suppression.

We should probably also figure out why we're painting the old document during paint suppression...
Isn't that the whole point of paint suppression?  That we keep painting the old (now dead) document until the timer fires?

This is also how we end up with things like mContent->GetDocument() == nsnull in various frame painting and reflow code during paint suppression.
(In reply to comment #3)
> Isn't that the whole point of paint suppression?  That we keep painting the old
> (now dead) document until the timer fires?

Yes, but we shouldn't repaint it if nothing has changed that requires a repaint.

So i suspect the issue is the order of things happening in doRemoveChildAt:

1. We first remove the child from the childlist (this will cause GetRootContent
   to return null).
2. We then call ContentRemoved, which is when I suspect that we end up
   repainting and paint a white background.
3. Finally we call UnbindFromTree().

The way things used to behave was that we did exactly the same as above, but we let GetRootContent return the root content until all children had been deleted and all notifications were done.

I would argue that the new behaviour is right. We should let GetRootContent return null as soon as the root content is removed from the document. Especially since mRootContent was a weak pointer and could be dangling at this point.

The problem is why we're repainting at all. The document is being torn down so it seems like a bad idea to still try to paint it. If we indeed want to keep painting the old document, then shouldn't we keep it alive during paint supression?
I'm amused to note that both the Close and Destroy methods of DocumentViewerImpl call mDocument->Destroy(); only the former keeps it around.  I'm wondering how much of this was changed during the fastback work.

We do need UnbindFromTree to happen at Close (IIRC), but I think the content tree should remain attached to the document until Destroy.  Can we do that?
Oh, the repaint is triggered from the containing XUL and has nothing to do with what's inside; ContentRemoved on the frame constructor is not called during this process.
Calling UnbindFromTree but not actually removing the node is somewhat dangerous. A node that is unbound but that the parent still holds on to can be inserted elsewhere in a tree and that way have two parents. This would most likly introduce crashers.
Hmmm...  I'm not sure why we need to unbind at Close()...  Other than perhaps things like event dispatch and focus?  I can see those making assumptions along those lines.
If the only reason we're doing this is to fix ref-cycles, how come they don't appear in DOMParser or XMLHttpRequest documents? Would tearing down surrounding objects such as presshells and/or event managers do the trick?
> how come they don't appear in DOMParser or XMLHttpRequest documents?

Just by inspection, the following things are torn down in ::UnbindFromTree impls that could cause cycles:

1)  XBL bindings (not applied in data documents)
2)  Box objects (probably don't cause cycles, since they mostly hold weak refs)
3)  Access keys (could have the ESM clear that hashtable, if so)
4)  Whatever XTF is doing; no idea what
5)  XUL element controllers (XUL elements in data documents will in fact leak if
    you ever get their controllers... there are other ways to get it to leak
    too; e.g. by getting controllers off of a XUL node that's not in a
    document).  Neil knows more about this.
6)  XUL listener managers?  Not sure.

Of this list I suspect the big problem is #1 -- bindings hold stuff until the destructor, destructor is called via Unbind, etc.


We do need to make scripts stop running, IIRC.  Although perhaps less so with splitwindow.
Unhooking all content won't really stop scripts from running.
I'm closing this for now since the patch that caused it is backed out.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.