Flash of black at the end of full screen transition at apple.com product videos
Created attachment 95390 [details] Patch
Comment on attachment 95390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95390&action=review OK. > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:156 > if (m_rootLayer) { The null check for m_rootLayer seems a little strange given that the code you are moving was already dereferencing m_rootLayer before this. I’m guessing this null check is not needed (but not saying you should take it out right now). > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:159 > [CATransaction begin]; > PlatformLayer* rootPlatformLayer = m_rootLayer->platformLayer(); > + [[NSNotificationCenter defaultCenter] postNotificationName:@"WebKitLayerHostChanged" object:rootPlatformLayer userInfo:nil]; Is it really a good idea to send the notification after beginning the CA transaction? Is there any harm to making the CATransaction method call afterward?
(In reply to comment #2) > (From update of attachment 95390 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95390&action=review > > OK. > > > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:156 > > if (m_rootLayer) { > > The null check for m_rootLayer seems a little strange given that the code you are moving was already dereferencing m_rootLayer before this. I’m guessing this null check is not needed (but not saying you should take it out right now). I know, this could probably safely turn into an ASSERT or be removed altogether. > > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:159 > > [CATransaction begin]; > > PlatformLayer* rootPlatformLayer = m_rootLayer->platformLayer(); > > + [[NSNotificationCenter defaultCenter] postNotificationName:@"WebKitLayerHostChanged" object:rootPlatformLayer userInfo:nil]; > > Is it really a good idea to send the notification after beginning the CA transaction? Is there any harm to making the CATransaction method call afterward? The only possible harm would be to re-introduce some flashing during the transition. Wrapping these in a transaction should guarantee that the operations aren't flushed until the transaction completes. We have to send the notification while the listeners are still ancestors of rootPlatformLayer. And we should remove those children inside this transaction, to avoid flashing. So the notification should occur within the transaction, could occur before the transaction, and can't occur afterwards. That said, I don't know what the danger would be of this notification being handled within the transaction.
Committed r87768: <http://trac.webkit.org/changeset/87768>