642977 - Iframe contents flicker while resizing browser

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Alice0775 White]

Build Identifier:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110318 Firefox/4.0b13pre ID:20110318030416
and
Mozilla/5.0 (X11; Linux i686; rv:2.0b13pre) Gecko/20110318 Firefox/4.0b13pre ID:20110318071829

I encountered this problem when I test Bug 642800.

Iframe contents flicker while resizing browser.

This does not happen on Firefox3.6.15, Chrome12.0.706.0 canary build and Opera11.01.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open URL or http://www.dentaku.com/demo/firefox4rc/iframe-resize/
3. Resize window

Actual Results:
 Iframe contents flicker while resizing browser

Expected Results:
 Should not flicker

Regression window(flockering while resize browser):
Works:
http://hg.mozilla.org/mozilla-central/rev/09872e2e2130
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100901 Firefox/4.0b6pre ID:20100901135147
Fails:
http://hg.mozilla.org/mozilla-central/rev/d35f97d3fe5c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100901 Firefox/4.0b6pre ID:20100901150524
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=09872e2e2130&tochange=d35f97d3fe5c
Triggered by:
a7e7578e3ba7	Timothy Nikkel — Bug 591435. Need to honour paint suppression in subdocuments. r=roc a=blocking beta6+

Comment 1

[Timothy Nikkel (:tnikkel)]

We have a resize event handler that sets the display of the iframe element to none, then sets its height based on document.documentElement.clientHeight (which flushes), and then sets its display to block. So we have to reconstruct the iframe and do an initial reflow of its document, which makes painting suppressed for some period of time.

Comment 2

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

We should only suppress painting for documents that are loading. If a document is fully loaded and we create a presshell for it, PresShell::InitialReflow should not enter paint suppression.

Comment 3

[Timothy Nikkel (:tnikkel)]

So I guess what we want is a general way of knowing when LoadComplete is not going to be called after InitialReflow (error pages also suffer from this, and there is a bug on it somewhere) so we can skip paint suppression.

Comment 4

[Timothy Nikkel (:tnikkel)]

A quick look at the docshell code makes me think that a method on the docshell called something like ShouldExpectLoadEvent that checks if mEODForCurrentDocument is false on the docshell and mIsLoadingDocument is true on its docloader would do the trick. We could call that from InitialReflow and decide to suppress or not suppress painting there.

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to comment #3)
> So I guess what we want is a general way of knowing when LoadComplete is not
> going to be called after InitialReflow (error pages also suffer from this, and
> there is a bug on it somewhere) so we can skip paint suppression.

The error pages bug is bug 568844.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Another option is to only do paint suppression for initialreflow called from the sinks.  In most other cases, I'd think we don't want to suppress it.

And even for the sinks the current logic is semi-bogus in some cases (e.g. the ones where we have already delayed initialreflow waiting on stylesheets)...

If we do want to check "loaded" state, we can check whether DOMContentLoaded has fired on the document, right?  Or in other words, check the readyState of the document; if it's INTERACTIVE or COMPLETE, then we're done parsing.  Or do we want some other definition of "loaded"?

mEODForCurrentDocument corresponds to onload, for what it's worth.  If that's what you want, you can just directly ask the document for that information.

Comment 7

[Timothy Nikkel (:tnikkel)]

It's not that we want to do something when the load event happens, it's that we want to know in advance if a load event is coming at all. This is because DocumentViewerImpl::LoadComplete usually unsuppresses painting (if the timer isn't the one that does it). The problem in this bug is that LoadComplete unsuppresses painting, but a paint happens before that (and after InitialReflow). The problem in bug 568844 is that LoadComplete is never called. So we can check the ready state of the document and that fixes this bug, but that doesn't fix bug 568844 because the readystate is loading for the error page when InitialReflow is called. If there is a better way to do this I'm open to it of course.

Comment 8

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

Error pages are just broken.  There is no good way to know that they're not going to fire onload "sometime".....

Comment 10

[Boris Zbarsky [:bzbarsky]]

And we need to fix that, btw; we have bugs on that.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 520835 [details] [diff] [review]
patch

I really do think I'd prefer checking the document's ready state and fixing error pages in some sane way....

Comment 12

[Timothy Nikkel (:tnikkel)]

Ok, I can do it that way.

Comment 13

[Timothy Nikkel (:tnikkel)]

Attachment: alternate patch

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 520847 [details] [diff] [review]
alternate patch

r=me, assuming we do want to turn off paint suppression once we have DOMContentLoaded.

Comment 15

[Timothy Nikkel (:tnikkel)]

Attachment: alternate patch v2

Your comment made me have second thoughts. I'm going to go with the more conservative fix. We can be more aggressive in the future if we want.

Comment 16

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/f4faa30ceddc

Comment 17

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/f4faa30ceddc