61756 2011-05-30 18:44:35 -0700 Flash of black at the end of full screen transition at apple.com product videos 2011-05-31 18:49:05 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jer.noble jer.noble oldest_to_newest 412419 0 jer.noble 2011-05-30 18:44:35 -0700 Flash of black at the end of full screen transition at apple.com product videos 412424 1 95390 jer.noble 2011-05-30 18:55:46 -0700 Created attachment 95390 Patch 412435 2 95390 darin 2011-05-30 20:28:34 -0700 Comment on attachment 95390 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? 412436 3 jer.noble 2011-05-30 20:34:26 -0700 (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. 413071 4 jer.noble 2011-05-31 18:49:05 -0700 Committed r87768: <http://trac.webkit.org/changeset/87768> 95390 2011-05-30 18:55:46 -0700 2011-05-30 20:28:33 -0700 Patch bug-61756-20110530185545.patch text/plain 4866 jer.noble U3VidmVyc2lvbiBSZXZpc2lvbjogODc3MDMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAzZDRkZTRiMDI2YTVhYzQw ZmE5MTNjMDk3YjA5YWUyODMzYzFjMjlhLi40YmZjNTE5NmFmNDZjN2EyODE0OWU4N2RkNjdkZGE3 MmU5MWFkOGQ1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTYgQEAKIDIwMTEtMDUtMzAgIEplciBO b2JsZSAgPGplci5ub2JsZUBhcHBsZS5jb20+CiAKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgRmxhc2ggb2YgYmxhY2sgYXQgdGhlIGVuZCBvZiBmdWxsIHNj cmVlbiB0cmFuc2l0aW9uIGF0IGFwcGxlLmNvbSBwcm9kdWN0IHZpZGVvcworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjE3NTYKKworICAgICAgICBBZGRl ZCB0d28gbmV3IGVudHJpZXMgdG8gdGhlIFdlYkNvcmUgZXhwb3J0cyBsaXN0LgorCisgICAgICAg ICogV2ViQ29yZS5leHAuaW46CisKKzIwMTEtMDUtMzAgIEplciBOb2JsZSAgPGplci5ub2JsZUBh cHBsZS5jb20+CisKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFuIEJlcm5zdGVpbi4KIAogICAgICAg ICBQbGF5L1BhdXNlIGJ1dHRvbiBpbiB3cm9uZyBzdGF0ZSB3aGVuIGVudGVyaW5nIGZ1bGwgc2Ny ZWVuIG9uIFZpbWVvCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9XZWJDb3JlLmV4cC5pbiBi L1NvdXJjZS9XZWJDb3JlL1dlYkNvcmUuZXhwLmluCmluZGV4IDVjZTZjZjUyOThiMjYzMDUyMGRj NmE3MjVlZDgxNjgwZDA4NTdkYTguLjE2Nzk1YjY5ZjYyOTE1YzliZDQ4MzE4MDA3MzA3ZmUxYjQ4 N2RkNDkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL1dlYkNvcmUuZXhwLmluCisrKyBiL1Nv dXJjZS9XZWJDb3JlL1dlYkNvcmUuZXhwLmluCkBAIC05MzAsNiArOTMwLDcgQEAgX19aTjdXZWJD b3JlOUZyYW1lVmlldzE0YWRqdXN0Vmlld1NpemVFdgogX19aTjdXZWJDb3JlOUZyYW1lVmlldzE0 c2V0TWFyZ2luV2lkdGhFaQogX19aTjdXZWJDb3JlOUZyYW1lVmlldzE0c2V0TmVlZHNMYXlvdXRF dgogX19aTjdXZWJDb3JlOUZyYW1lVmlldzE0c2V0VHJhbnNwYXJlbnRFYgorX19aTks3V2ViQ29y ZTlGcmFtZVZpZXcxM2lzVHJhbnNwYXJlbnRFdgogX19aTjdXZWJDb3JlOUZyYW1lVmlldzE1c2V0 TWFyZ2luSGVpZ2h0RWkKIF9fWk43V2ViQ29yZTlGcmFtZVZpZXcxNnNldFBhaW50QmVoYXZpb3JF agogX19aTjdXZWJDb3JlOUZyYW1lVmlldzE3c2V0U2Nyb2xsUG9zaXRpb25FUktOU184SW50UG9p bnRFCkBAIC05MzcsNiArOTM4LDcgQEAgX19aTjdXZWJDb3JlOUZyYW1lVmlldzE4dXBkYXRlQ29u dHJvbFRpbnRzRXYKIF9fWk43V2ViQ29yZTlGcmFtZVZpZXcyMGVudGVyQ29tcG9zaXRpbmdNb2Rl RXYKIF9fWk43V2ViQ29yZTlGcmFtZVZpZXcyMWZsdXNoRGVmZXJyZWRSZXBhaW50c0V2CiBfX1pO N1dlYkNvcmU5RnJhbWVWaWV3MjJzZXRCYXNlQmFja2dyb3VuZENvbG9yRVJLTlNfNUNvbG9yRQor X19aTks3V2ViQ29yZTlGcmFtZVZpZXcxOWJhc2VCYWNrZ3JvdW5kQ29sb3JFdgogX19aTjdXZWJD b3JlOUZyYW1lVmlldzIzdXBkYXRlQ2FuSGF2ZVNjcm9sbGJhcnNFdgogX19aTjdXZWJDb3JlOUZy YW1lVmlldzI0Zm9yY2VMYXlvdXRGb3JQYWdpbmF0aW9uRVJLTlNfOUZsb2F0U2l6ZUVmTlNfMTlB ZGp1c3RWaWV3U2l6ZU9yTm90RQogX19aTjdXZWJDb3JlOUZyYW1lVmlldzI2YWRqdXN0UGFnZUhl aWdodERlcHJlY2F0ZWRFUGZmZmYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxv ZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwppbmRleCBmNDE4OTY4ZmZjY2M0YmQ4NTJhZTli NDdmMzhlMzQyMjM3ZTQ2NWIwLi43ZjE1ZWUxYTA0NThjNTJkZmQ3ODY1MWFlMWVjYTk2MDJkYTAy Njg0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dl YktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTEtMDUtMzAgIEplciBOb2JsZSAg PGplci5ub2JsZUBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgRmxhc2ggb2YgYmxhY2sgYXQgdGhlIGVuZCBvZiBmdWxsIHNjcmVlbiB0 cmFuc2l0aW9uIGF0IGFwcGxlLmNvbSBwcm9kdWN0IHZpZGVvcworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjE3NTYKKworICAgICAgICBUaGUgQCJXZWJL aXRMYXllckhvc3RDaGFuZ2VkIiBub3RpZmljYXRpb24gaXMgY2F1c2luZyB0aGUgUVRNb3ZpZSB0 byB0ZWFyIGRvd24gaXRzIGxheWVyCisgICAgICAgICh3aGljaCBpcyBnb29kKSBhdCB0aGUgd3Jv bmcgdGltZSAod2hpY2ggaXMgYmFkKS4gIFNlbmQgdGhpcyBub3RpZmljYXRpb24gb25seSBhZnRl ciAKKyAgICAgICAgdGhlIGRyYWdJbWFnZSBzbmFwc2hvdCBpcyB0YWtlbi4gIFRoaXMgZXhwb3Nl ZCBhbm90aGVyIHByb2JsZW0sIHRoYXQgdGhlIHNuYXBzaG90IGhhcworICAgICAgICBhIHdoaXRl IGJhY2tncm91bmQuICBTZXQgdGhlIGRlZmF1bHQgYmFja2dyb3VuZCBjb2xvciBhbmQgdHJhbnNw YXJlbmN5IGZvciB0aGUgdmlldyAobWFraW5nCisgICAgICAgIHN1cmUgdG8gcmVzZXQgdGhlbSBh ZnRlcndhcmRzKSB0byB0cmFuc3BhcmVudC4KKworICAgICAgICAqIFdlYlByb2Nlc3MvRnVsbFNj cmVlbi9tYWMvV2ViRnVsbFNjcmVlbk1hbmFnZXJNYWMubW06CisgICAgICAgIChXZWJLaXQ6Oldl YkZ1bGxTY3JlZW5NYW5hZ2VyTWFjOjpzZXRSb290RnVsbFNjcmVlbkxheWVyKToKKwogMjAxMS0w NS0zMCAgTm8nYW0gUm9zZW50aGFsICA8bm9hbS5yb3NlbnRoYWxAbm9raWEuY29tPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IFNpbW9uIEhhdXNtYW5uLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktp dDIvV2ViUHJvY2Vzcy9GdWxsU2NyZWVuL21hYy9XZWJGdWxsU2NyZWVuTWFuYWdlck1hYy5tbSBi L1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvRnVsbFNjcmVlbi9tYWMvV2ViRnVsbFNjcmVlbk1h bmFnZXJNYWMubW0KaW5kZXggYjFhZjMyYWUxMzYyMDAyNzg2NzdmNThjMjNlNTQxYjE1ZDUxN2Yx Zi4uYzhhOWMwYWM2ZWNlZjc0YzU4ZDM3YTFkZWI3MTUwNWViODY5ZjMzNSAxMDA2NDQKLS0tIGEv U291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9GdWxsU2NyZWVuL21hYy9XZWJGdWxsU2NyZWVuTWFu YWdlck1hYy5tbQorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL0Z1bGxTY3JlZW4vbWFj L1dlYkZ1bGxTY3JlZW5NYW5hZ2VyTWFjLm1tCkBAIC0xMzksMTQgKzEzOSwyNSBAQCB2b2lkIFdl YkZ1bGxTY3JlZW5NYW5hZ2VyTWFjOjpzZXRSb290RnVsbFNjcmVlbkxheWVyKFdlYkNvcmU6Okdy YXBoaWNzTGF5ZXIqIGxheQogCiAgICAgaWYgKCFsYXllcikgewogICAgICAgICBtX3BhZ2UtPnNl bmQoTWVzc2FnZXM6OldlYkZ1bGxTY3JlZW5NYW5hZ2VyUHJveHk6OkV4aXRBY2NlbGVyYXRlZENv bXBvc2l0aW5nTW9kZSgpKTsKLSAgICAgICAgW1tOU05vdGlmaWNhdGlvbkNlbnRlciBkZWZhdWx0 Q2VudGVyXSBwb3N0Tm90aWZpY2F0aW9uTmFtZTpAIldlYktpdExheWVySG9zdENoYW5nZWQiIG9i amVjdDptX3Jvb3RMYXllci0+cGxhdGZvcm1MYXllcigpIHVzZXJJbmZvOm5pbF07CiAKICAgICAg ICAgRnJhbWUqIGZyYW1lID0gbV9lbGVtZW50LT5kb2N1bWVudCgpLT5mcmFtZSgpOwotICAgICAg ICBEcmFnSW1hZ2VSZWYgZHJhZ0ltYWdlID0gZnJhbWUgPyBmcmFtZS0+bm9kZUltYWdlKG1fZWxl bWVudC5nZXQoKSkgOiAwOworICAgICAgICBGcmFtZVZpZXcqIHZpZXcgPSBmcmFtZSA/IGZyYW1l LT52aWV3KCkgOiAwOworICAgICAgICBEcmFnSW1hZ2VSZWYgZHJhZ0ltYWdlID0gMDsKKyAgICAg ICAgaWYgKHZpZXcpIHsKKyAgICAgICAgICAgIENvbG9yIHNhdmVkQmFja2dyb3VuZENvbG9yID0g dmlldy0+YmFzZUJhY2tncm91bmRDb2xvcigpOworICAgICAgICAgICAgYm9vbCBzYXZlZElzVHJh bnNwYXJlbnQgPSB2aWV3LT5pc1RyYW5zcGFyZW50KCk7CisgICAgICAgICAgICB2aWV3LT5zZXRC YXNlQmFja2dyb3VuZENvbG9yKENvbG9yOjp0cmFuc3BhcmVudCk7CisgICAgICAgICAgICB2aWV3 LT5zZXRUcmFuc3BhcmVudCh0cnVlKTsKKyAgICAgICAgICAgIGRyYWdJbWFnZSA9IGZyYW1lLT5u b2RlSW1hZ2UobV9lbGVtZW50LmdldCgpKTsKKyAgICAgICAgICAgIHZpZXctPnNldEJhc2VCYWNr Z3JvdW5kQ29sb3Ioc2F2ZWRCYWNrZ3JvdW5kQ29sb3IpOworICAgICAgICAgICAgdmlldy0+c2V0 VHJhbnNwYXJlbnQoc2F2ZWRJc1RyYW5zcGFyZW50KTsKKyAgICAgICAgfQogCiAgICAgICAgIGlm IChtX3Jvb3RMYXllcikgewogICAgICAgICAgICAgW0NBVHJhbnNhY3Rpb24gYmVnaW5dOwogICAg ICAgICAgICAgUGxhdGZvcm1MYXllciogcm9vdFBsYXRmb3JtTGF5ZXIgPSBtX3Jvb3RMYXllci0+ cGxhdGZvcm1MYXllcigpOworICAgICAgICAgICAgW1tOU05vdGlmaWNhdGlvbkNlbnRlciBkZWZh dWx0Q2VudGVyXSBwb3N0Tm90aWZpY2F0aW9uTmFtZTpAIldlYktpdExheWVySG9zdENoYW5nZWQi IG9iamVjdDpyb290UGxhdGZvcm1MYXllciB1c2VySW5mbzpuaWxdOworCiAgICAgICAgICAgICBt X3Jvb3RMYXllci0+cmVtb3ZlQWxsQ2hpbGRyZW4oKTsKICAgICAgICAgICAgIG1fcm9vdExheWVy LT5zeW5jQ29tcG9zaXRpbmdTdGF0ZUZvclRoaXNMYXllck9ubHkoKTsKICAgICAgICAgICAgIG1f cm9vdExheWVyID0gbnVsbHB0cjsK