997255 - Try to not flush layout after hiding an iframe

Status: NEW

(Core :: Layout, defect, P3)

Description

[Olli Pettay [:smaug][[e-mail]]]

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSubDocumentFrame.cpp?rev=f1822d4ddb96&mark=857-857#851 flushes, and
that shows up in profiles.
There is a case when 'Sheet app 1' during refresh driver tick does
several flushes because http://mxr.mozilla.org/gaia/source/apps/system/js/popup_manager.js
has listener for transitionend which is dispatched during tick and the event listener
removes element from doc.

I don't quite see why we need that explicit flush.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

note, the explicit flush is 9% in the profile I'm looking at.
I assume if that flush was removed, some more flushing would be done later during refresh driver tick.

Comment 2

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

That code is nsSubdocumentFrame trying to distinguish "<iframe> set to display:none (or removed from document)" from "<iframe> is reframed". When an nsSubdocumentFrame is destroyed, it schedules an nsHideViewer as a scriptrunnable. nsHideViewer checks to see if the element has a frame; if it does, then the element was just reframed or moved in the document and we should not tear down the presentation. Otherwise the element has been removed from the document or set to display:none and we should tear down the presentation. Of course, we need to flush out style/frame construction to check whether the element has a frame.

If an <iframe> is being hidden by removing it from the document, we could make nsHideViewer check whether the element is in the document; if it's not, then we know without flushing style that the element won't have a frame and we can tear down the presentation. But if the <iframe> is being hidden by setting it to "display:none", we need to tear down its presentation here.

I suppose we might be able to delay tearing down the presentation until the next style flush, whenever that is triggered, and not trigger one ourselves ever, but that would create weird observable side effects dependent on the timing of style flushes.