Bug 58676 - Views should be made visible before painting in WM_PRINTCLIENT messages
Summary: Views should be made visible before painting in WM_PRINTCLIENT messages
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Brian Weinstein
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2011-04-15 11:17 PDT by Brian Weinstein
Modified: 2011-04-15 12:47 PDT (History)
1 user (show)

See Also:


Attachments
[PATCH] Fix (2.34 KB, patch)
2011-04-15 11:21 PDT, Brian Weinstein
aroben: review+
Details | Formatted Diff | Diff
[PATCH] Fix v2 (3.40 KB, patch)
2011-04-15 12:42 PDT, Brian Weinstein
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Weinstein 2011-04-15 11:17:40 PDT
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>
Comment 1 Brian Weinstein 2011-04-15 11:21:15 PDT
Created attachment 89817 [details]
[PATCH] Fix
Comment 2 Jeff Miller 2011-04-15 11:32:53 PDT
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?
Comment 3 Brian Weinstein 2011-04-15 11:34:03 PDT
(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!
Comment 4 Adam Roben (:aroben) 2011-04-15 12:15:33 PDT
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.
Comment 5 Brian Weinstein 2011-04-15 12:42:10 PDT
Created attachment 89831 [details]
[PATCH] Fix v2
Comment 6 Brian Weinstein 2011-04-15 12:47:31 PDT
Landed in r84019.