Bug 92848

Summary: Attempt to fix flakiness of color-suggestion-picker-appearance.html
Product: WebKit Reporter: Keishi Hattori <keishi>
Component: FormsAssignee: Keishi Hattori <keishi>
Status: RESOLVED FIXED    
Severity: Normal CC: tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Keishi Hattori 2012-08-01 00:59:18 PDT 
We observed console.log bleeding into the next test.
Comment 1 Keishi Hattori 2012-08-01 01:12:30 PDT 
Created attachment 155745 [details]
Patch
Comment 2 Kent Tamura 2012-08-01 01:20:18 PDT 
Comment on attachment 155745 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        (ColorPicker.prototype._layout): We put the width and height into
> +        variables so we don't call window.onresize twice.

Why window.onresize is called twice with the current code?
Comment 3 Keishi Hattori 2012-08-01 01:34:35 PDT 
(In reply to comment #2)
> (From update of attachment 155745 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=155745&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        (ColorPicker.prototype._layout): We put the width and height into
> > +        variables so we don't call window.onresize twice.
> 
> Why window.onresize is called twice with the current code?

My theory is that this._element.offsetHeight is causing a layout, which asks the window to resize immediately.
Comment 4 Kent Tamura 2012-08-01 01:43:18 PDT 
Comment on attachment 155745 [details]
Patch

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

>>> Source/WebCore/ChangeLog:10
>>> +        variables so we don't call window.onresize twice.
>> 
>> Why window.onresize is called twice with the current code?
> 
> My theory is that this._element.offsetHeight is causing a layout, which asks the window to resize immediately.

I don't think so. Layout doesn't change the window size basically.
Did you reproduce the problem locally, and confirmed the fix by this change?
Comment 5 Keishi Hattori 2012-08-01 03:31:22 PDT 
(In reply to comment #4)
> > My theory is that this._element.offsetHeight is causing a layout, which asks the window to resize immediately.
> 
> I don't think so. Layout doesn't change the window size basically.
> Did you reproduce the problem locally, and confirmed the fix by this change?

Here is the stack trace. It seems to be true.

WebCore::EventHandler::sendResizeEvent()
WebCore::FrameView::performPostLayoutTasks()
WebCore::FrameView::layout(bool)
WebCore::RenderWidget::updateWidgetPosition()
WebCore::RenderView::updateWidgetPositions()
WebCore::FrameView::performPostLayoutTasks()
WebCore::FrameView::layout(bool)
WebCore::Document::updateLayout()
WebCore::Document::updateLayout()
WebCore::Document::updateLayoutIgnorePendingStylesheets()
WebCore::Element::offsetHeight()
...
Comment 6 Kent Tamura 2012-08-01 04:35:25 PDT 
So, did you reproduce the problem locally, and confirmed the fix?
Comment 7 Keishi Hattori 2012-08-01 04:36:18 PDT 
(In reply to comment #6)
> So, did you reproduce the problem locally, and confirmed the fix?

Yes, I did.
Comment 8 Kent Tamura 2012-08-01 04:40:32 PDT 
Comment on attachment 155745 [details]
Patch

ok
Comment 9 WebKit Review Bot 2012-08-01 05:03:27 PDT 
Comment on attachment 155745 [details]
Patch

Clearing flags on attachment: 155745

Committed r124326: <http://trac.webkit.org/changeset/124326>
Comment 10 WebKit Review Bot 2012-08-01 05:03:30 PDT 
All reviewed patches have been landed.  Closing bug.