Bug 61756 – Flash of black at the end of full screen transition at apple.com product videos

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jer Noble]

Flash of black at the end of full screen transition at apple.com product videos

[Jer Noble]

Created attachment 95390 [details]
Patch

[Darin Adler]

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?

[Jer Noble]

(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.

[Jer Noble]

Committed r87768: <http://trac.webkit.org/changeset/87768>