Bug 58676 – Views should be made visible before painting in WM_PRINTCLIENT messages

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brian Weinstein]

If a view that is in the background (from a background tab) tries to paint to an HDC in response to a WM_PRINTCLIENT message, painting is suspended from the drawing area, so the paint will fail.

We should tell the view that it is temporarily visible, so it is allowed to paint.

<rdar://problem/9279211>

[Brian Weinstein]

Created attachment 89817 [details]
[PATCH] Fix

[Jeff Miller]

Comment on attachment 89817 [details]
[PATCH] Fix

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

> Source/WebKit2/UIProcess/win/WebView.cpp:693
> +    bool isVisible = isViewVisible();

Maybe this should be wasVisible?

[Brian Weinstein]

(In reply to comment #2)
> (From update of attachment 89817 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89817&action=review
> 
> > Source/WebKit2/UIProcess/win/WebView.cpp:693
> > +    bool isVisible = isViewVisible();
> 
> Maybe this should be wasVisible?

Yeah, I was waffling on a name here. wasVisible is good. Thanks!

[Adam Roben (:aroben)]

Comment on attachment 89817 [details]
[PATCH] Fix

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

> Source/WebKit2/UIProcess/win/WebView.cpp:700
> +    if (!isVisible)
> +        setIsVisible(true);
> +
>      paint(hdc, winRect);
>  
> +    if (!isVisible)
> +        setIsVisible(false);

I think it's worth adding a comment explaining why we do this.

It's a little unfortunate that we don't have more fine-grained control of this. Today, the visible state of the view only affects whether painting happens. In the future it could affect more things which we don't want to touch here. Maybe a FIXME saying that it would be good to have a more-direct way of telling the web process to draw even if we're invisible would be enough here.

> Source/WebKit2/UIProcess/win/WebView.cpp:1482
> +void WebView::setIsVisible(bool isVisible)
> +{
> +    m_isVisible = isVisible;
> +    m_page->viewStateDidChange(WebPageProxy::ViewIsVisible);
> +}

Seems like onShowWindowEvent should call this function.

[Brian Weinstein]

Created attachment 89831 [details]
[PATCH] Fix v2

[Brian Weinstein]

Landed in r84019.