1397239 - nsCSSFrameConstructor::mTempFrameTreeState can leak.

Status: NEW

(Core :: Layout, enhancement, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

Right now we record the frame tree state when reconstructing frames for an element, even when they're reconstructed asynchronously.

The problem is that after that, but before we actually go ahead and reconstruct the frames, some DOM content may be gone from the document, and we indeterminately store the frame tree state there.

Should we clear the frame tree state after a flush with FlushType >= FlushType::Frames?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Testcase

When we remove the <div id="test"> we reconstruct the <span> (and the parent <div> fwiw).

We then remove the text control from the DOM, so nothing will retrieve the state back when we reconstruct, and there's nothing clearing it afterwards.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

(I plan to look at this more in depth when I got the chance).

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Should we clear the frame tree state after a flush with FlushType >= FlushType::Frames?

That seems fairly plausible to me, but worth checking what other UAs do with testcases that send something display:none, then flush, then send it back to display:block and flush and what happens to the scroll position of things in there, including both text controls and overflow:scroll divs.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #3)
> > Should we clear the frame tree state after a flush with FlushType >= FlushType::Frames?
> 
> That seems fairly plausible to me, but worth checking what other UAs do with
> testcases that send something display:none, then flush, then send it back to
> display:block and flush and what happens to the scroll position of things in
> there, including both text controls and overflow:scroll divs.

So bad news is that they (Blink / WebKit) preserve overflow:scroll state across flushes, so this is not going to work :(.

They didn't seem to preserve any kind of text control scroll state though.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I haven't looked into more ways to fix it. I guess we can remove the key from UnbindFromTree or what not... I don't plan to look at this particularly soon, I'm swamped already :)