Bug 88374 – WebPage::windowIsVisible is incorrect during resumePainting

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

[Tim Horton]

A few methods in WKView do something like this:

_data->_page->viewStateDidChange(WebPageProxy::ViewIsVisible);
[self _updateWindowVisibility];

viewStateDidChange can end up calling resumePainting, which depends on the window visibility (windowIsVisible) being correctly set in order to unpause the requestAnimationFrame controller.

The only other thing that depends on windowIsVisible is PluginView and friends, which seem unlikely to mind it being updated slightly earlier. The behavior as it stands now seems wrong, and will likely only serve to confuse future users of windowIsVisible.

<rdar://problem/11587039>

[Tim Horton]

I've tried and failed continuously to make a test for this. The mixture of fullscreen and rAF and WebKit2 and the different DrawingArea implementations is making it very hard, and potentially not possible (I can't cause the problem to reproduce in WKTR at all).

[Tim Horton]

One effect of this is that Bing Maps stops updating if you full-screen it (http://binged.it/K9uLcw).

[Tim Horton]

Created attachment 145897 [details]
patch

went a bit overboard with the changelog

[Simon Fraser (smfr)]

Comment on attachment 145897 [details]
patch

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

r=me as long as jer and/or anders is OK with this.

> Source/WebKit2/ChangeLog:24
> +        Will cause the second and third rAF calls to have no effect. Even after cancelFullScreen() is called,

lowercase w

[Jer Noble]

I'm fine with this.

[Tim Horton]

Anders said it was OK on IRC before lunch yesterday, too.

http://trac.webkit.org/changeset/119570

[Tim Horton]

(In reply to comment #4)
> (From update of attachment 145897 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145897&action=review
> 
> r=me as long as jer and/or anders is OK with this.
> 
> > Source/WebKit2/ChangeLog:24
> > +        Will cause the second and third rAF calls to have no effect. Even after cancelFullScreen() is called,
> 
> lowercase w

Aack, I missed this comment. I don't really see a follow-up commit to fix the ChangeLog, though?