[CoordinatedGraphics] invisible webview should not paint the content

[Ryuan Choi]

CoordinatedDrawingArea can suspend or resume the painting when visibility of webpage is changed.
But, CoordinatedDrawingArea does not suspend the painting when invisible webview is created.

[Ryuan Choi]

Created attachment 264296 [details]
Patch

[Darin Adler]

Comment on attachment 264296 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:-72
> -    , m_isPaintingSuspended(!(parameters.viewState & ViewState::IsVisible))

Not sure why we are changing the code to not initialize this. We need to give it some value at this point.

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:79
> +    m_isPaintingSuspended = parameters.viewState & ViewState::IsVisible;

This doesn’t look right for the visible case. It’s going to set m_isPaintingSuspended to true in that case.

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:81
> +    if (!m_isPaintingSuspended)
> +        suspendPainting();

Seems like what we actually want to do is to initialize m_isPaintingSuspended to false, and then write:

    if (!(parameters.viewState & ViewState::IsVisible))
        suspendPainting();

Also would be good to come up with a way to do a regression test.

[Ryuan Choi]

Created attachment 264513 [details]
Patch

[Ryuan Choi]

Comment on attachment 264296 [details]
Patch

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

>> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:-72
>> -    , m_isPaintingSuspended(!(parameters.viewState & ViewState::IsVisible))
> 
> Not sure why we are changing the code to not initialize this. We need to give it some value at this point.

OK, I tried as another approach.

>> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:81
>> +        suspendPainting();
> 
> Seems like what we actually want to do is to initialize m_isPaintingSuspended to false, and then write:
> 
>     if (!(parameters.viewState & ViewState::IsVisible))
>         suspendPainting();
> 
> Also would be good to come up with a way to do a regression test.

I followed like you mentioned.

But, I don't have idea to test this.
This is not to draw anything when view is created without visibility.

[Darin Adler]

Comment on attachment 264513 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:80
> +    if (!(parameters.viewState & ViewState::IsVisible))
> +        suspendPainting();

I suggest initializing m_isPaintingSuspended to false, instead of initializing it based on the IsVisible flag.

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedDrawingArea.cpp:-408
> -    ASSERT(!m_isPaintingSuspended);

I suggest leaving this assertion in.

[Ryuan Choi]

Created attachment 264677 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 264677 [details]
Patch

Clearing flags on attachment: 264677

Committed r191987: <http://trac.webkit.org/changeset/191987>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.