Bug 142750 – WebKit1 Clients Are Not Reliably Repainted

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Brent Fulgham]

In Bug 135514 I corrected a problem where paint operations were being performed during layout. An unintended side-effect of this was that WebKit clients that performed a programmatic scroll of web content (outside of WebKit's control) would no longer receive a "free" paint operation.

To avoid breaking clients that relied on this misfeature, we need to notify AppKit that the view needs to be repainted once layout completes.

[Brent Fulgham]

Created attachment 248751 [details]
Patch

[Brent Fulgham]

<rdar://problem/20042453>

[Simon Fraser (smfr)]

Comment on attachment 248751 [details]
Patch

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

> Source/WebCore/page/FrameView.h:116
> +    WEBCORE_EXPORT bool isInViewSizeAdjust() { return m_layoutPhase == InViewSizeAdjust; }

I think this would be better called something like inPaintableState(), and it should test the inverse.

> Source/WebKit/mac/WebView/WebClipView.mm:118
> +    if (paintable)
> +        [[self window] _disableDelayedWindowDisplay];

Is this still required?

> Source/WebKit/mac/WebView/WebClipView.mm:123
> +    if (paintable)
> +        [[self window] _enableDelayedWindowDisplay];

And this?

[Brent Fulgham]

Created attachment 248755 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 248755 [details]
Patch

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

> Source/WebCore/page/FrameView.h:116
> +    WEBCORE_EXPORT bool inPaintableState() { return m_layoutPhase != InViewSizeAdjust && !isInLayout(); }

I don't think this is tight enough. FrameView::layout() sets the phase to InPostLayout before we've done updateLayerPositionsAfterLayout(), and it's certainly bad to paint before we've updated layers. I would prohibit painting with InLayout, InViewSizeAdjust and InPostLayout.

> Source/WebKit/mac/WebView/WebClipView.mm:119
> +                    [self setNeedsDisplay: YES];

No space after the :
I think this deserves a comment describing the situation under which this can occur.

[Brent Fulgham]

Committed r181587: <http://trac.webkit.org/changeset/181587>