Bug 119075 – With frame flattening on, too many resize events fired if document is resized in onresize handler.

• Component: Frames
• Status: RESOLVED FIXED

[Yongjun Zhang]

This is exposed by http://trac.webkit.org/changeset/149382.  When frame flattening is on, if we change an iframe's document size in its resize handler, we could get excessive number of resize events.

[zalan]

I think http://trac.webkit.org/changeset/149287 made the major logic change by moving 'resize event' sending from performPostLayout() to setFrameRect() and making it sync, while http://trac.webkit.org/changeset/149382 tried to ease on that by checking against isInLayout() and sending the resize event off async.
What I think what happens is that http://trac.webkit.org/changeset/150011 copied the resize event sending back to the performPostLayout() (now we are sending it off at 2 places), but since peformPostLayout() can be invoke by a timer, the isInLayout() guarding does not help there anymore and we are sending the resize events sync.

Do you have a test case on this?

[Yongjun Zhang]

(In reply to comment #1)
> I think http://trac.webkit.org/changeset/149287 made the major logic change by moving 'resize event' sending from performPostLayout() to setFrameRect() and making it sync, while http://trac.webkit.org/changeset/149382 tried to ease on that by checking against isInLayout() and sending the resize event off async.
> What I think what happens is that http://trac.webkit.org/changeset/150011 copied the resize event sending back to the performPostLayout() (now we are sending it off at 2 places), but since peformPostLayout() can be invoke by a timer, the isInLayout() guarding does not help there anymore and we are sending the resize events sync.
>
The problems is actually not caused by sync sending resize events.  Rather, it is caused by queueing extra resize events in DocumentEventQueue when it is still dispatching the previous resize event in DocumentEventQueue::pendingEventTimerFired, and this makes a loop in the same runloop.  Before 149287, we were dispatching the resize event in performPostLayout() in another runloop.

[Yongjun Zhang]

Created attachment 207557 [details]
Don't send resize event when widget frame is being changed in RenderFrameBase::layoutWithFlattening.

[Simon Fraser (smfr)]

Comment on attachment 207557 [details]
Don't send resize event when widget frame is being changed in RenderFrameBase::layoutWithFlattening. 

View in context: https://bugs.webkit.org/attachment.cgi?id=207557&action=review

> Source/WebCore/ChangeLog:8
> +        With http://trac.webkit.org/changeset/149287, WebCore also send resize event in FrameView::setFrameRect.  When

sends

> Source/WebCore/ChangeLog:12
> +        is done layout.

is done laying out

> Source/WebCore/page/FrameView.h:441
> +    void setResizeEventAllowed(bool resizeEventAllowed) { m_resizeEventAllowed = resizeEventAllowed; }

Might as well add a const inline setter for this too.

This currently doesn't allow nesting. Does it need to (using a counter)?

> Source/WebCore/rendering/RenderFrameBase.cpp:68
> +    childFrameView->setResizeEventAllowed(false);

Could TemporaryChange<> help you here?

[Yongjun Zhang]

(In reply to comment #4)
> (From update of attachment 207557 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207557&action=review

> > Source/WebCore/page/FrameView.h:441
> > +    void setResizeEventAllowed(bool resizeEventAllowed) { m_resizeEventAllowed = resizeEventAllowed; }
> 
> Might as well add a const inline setter for this too.

Did you mean getter?  I will add it.

> 
> This currently doesn't allow nesting. Does it need to (using a counter)?
> 
> > Source/WebCore/rendering/RenderFrameBase.cpp:68
> > +    childFrameView->setResizeEventAllowed(false);
> 
> Could TemporaryChange<> help you here?
Seems like if the method and member variable is in the same class, TemporaryChange<> would work.  In this patch, the method and member variable are from different classes.

thanks for the review.

[Yongjun Zhang]

Created attachment 207567 [details]
Address review comments for commit.

[WebKit Commit Bot]

Comment on attachment 207567 [details]
Address review comments for commit.

Clearing flags on attachment: 207567

Committed r153397: <http://trac.webkit.org/changeset/153397>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.