209610 2020-03-26 10:54:08 -0700 Swipe down gestures cause the video layer to stick for a moment before bouncing back into place 2020-03-27 13:57:00 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build iPhone / iPad Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=209680 InRadar P2 Normal --- 1 peng.liu6 peng.liu6 calvaris cdumez darin eric.carlson esprehn+autocc ews-watchlist glenn gyuyoung.kim jer.noble philipj sergio webkit-bug-importer oldest_to_newest 1634429 0 peng.liu6 2020-03-26 10:54:08 -0700 Swipe down gestures cause video layer to stick for a moment before bouncing back into place 1634431 1 peng.liu6 2020-03-26 10:56:03 -0700 <rdar://problem/55814988> 1634438 2 394629 peng.liu6 2020-03-26 11:07:50 -0700 Created attachment 394629 Patch 1634441 3 394629 darin 2020-03-26 11:15:16 -0700 Comment on attachment 394629 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394629&action=review We *must* find a way to test these animations rather than just making code changes and manually trying it out each time. This has turned out to be a difficult area to code properly and seems likely we will continue to break it over and over again in the future. It’s worth lots of extra effort to find a way to test this. Even if we can’t do it right at this moment and need to land this fix first. > Source/WebCore/html/HTMLMediaElement.cpp:6132 > + scheduleEvent(eventNames().webkitendfullscreenEvent); Is this correct in the case where we are about to call *enter*VideoFullscreenForVideoElement because m_videoFullscreenStandby is true? > Source/WebCore/html/HTMLMediaElement.cpp:6138 > else > - document().page()->chrome().client().exitVideoFullscreenForVideoElement(downcast<HTMLVideoElement>(*this)); > - scheduleEvent(eventNames().webkitendfullscreenEvent); > + m_fullscreenTaskQueue.enqueueTask([this] { > + document().page()->chrome().client().exitVideoFullscreenForVideoElement(downcast<HTMLVideoElement>(*this)); > + }); Change log does not make it clear why enqueing this on the task queue will properly sequence with the scheduled event. Is there something that makes sure that the events and the full screen task queue work in sequence? Multiple-line else body gets braces in WebKit coding style. 1634500 4 394629 peng.liu6 2020-03-26 12:51:31 -0700 Comment on attachment 394629 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394629&action=review Completely agree with you that we need tests for the animation. But I need to complete a patch for webkit.org/b/203723 before we can add more tests for video fullscreen and PiP. It is difficult to write reliable tests to test the animation itself, but we definitely can test the configurations related to animations in the WebCore side, e.g., the source/destination rect of animations. For this bug, such kind of tests can prevent regression in the future. >> Source/WebCore/html/HTMLMediaElement.cpp:6132 >> + scheduleEvent(eventNames().webkitendfullscreenEvent); > > Is this correct in the case where we are about to call *enter*VideoFullscreenForVideoElement because m_videoFullscreenStandby is true? When m_videoFullscreenStandby is true, this patch only changes the order to call scheduleEvent() and *enter*VideoFullscreenForVideoElement(). We will enter PiP when m_videoFullscreenStandby is true. It is a kind of exit fullscreen. >> Source/WebCore/html/HTMLMediaElement.cpp:6138 >> + }); > > Change log does not make it clear why enqueing this on the task queue will properly sequence with the scheduled event. Is there something that makes sure that the events and the full screen task queue work in sequence? > > Multiple-line else body gets braces in WebKit coding style. Unfortunately, this patch does not guarantee the things will happen in sequence. Eric proposed a solution: call exitVideoFullscreenForVideoElement() after HTMLElement::dispatchEvent(event) in HTMLMediaElement::dispatchEvent(). I will upload a new patch based on his solution. Sorry, will fix the style issue. 1634511 5 394647 peng.liu6 2020-03-26 13:08:31 -0700 Created attachment 394647 Revised the patch based on comments from Darin and Eric 1634656 6 ews-feeder 2020-03-26 17:13:30 -0700 Committed r259095: <https://trac.webkit.org/changeset/259095> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394647. 394629 2020-03-26 11:07:50 -0700 2020-03-26 13:08:28 -0700 Patch bug-209610-20200326110750.patch text/plain 2514 peng.liu6 U3VidmVyc2lvbiBSZXZpc2lvbjogMjU4NzcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYWNmMTlmNDA0MmYwZjgy YTZlZmNiYzRkNmVhYjg4MTlhZTdlZjVmMC4uYzExN2NhNTQxNzM0YjI2NzYyN2VjYThiZDI0OTg3 NTJhOWQyMWU3OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDIwLTAzLTI2ICBQZW5n IExpdSAgPHBlbmcubGl1NkBhcHBsZS5jb20+CisKKyAgICAgICAgU3dpcGUgZG93biBnZXN0dXJl cyBjYXVzZSB2aWRlbyBsYXllciB0byBzdGljayBmb3IgYSBtb21lbnQgYmVmb3JlIGJvdW5jaW5n IGJhY2sgaW50byBwbGFjZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MjA5NjEwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgRml4IGFuIGV4aXQgZnVsbHNjcmVlbiBhbmltYXRpb24gaXNzdWUgYnkgZmlyaW5n IHRoZSBlbmQgZnVsbHNjcmVlbiBldmVudCBlYXJseQorICAgICAgICB0byBsZXQgdGhlIHBhZ2Ug Y2hhbmdlIHRoZSB2aWRlbyBlbGVtZW50IGJhY2sgdG8gaXRzIG9yaWdpbmFsIHBvc2l0aW9uL3Np emUKKyAgICAgICAgYmVmb3JlIGV4aXRpbmcgZnVsbHNjcmVlbi4KKworICAgICAgICBDb3ZlcmVk IGJ5IGV4aXN0aW5nIHRlc3RzLgorCisgICAgICAgICogaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNw cDoKKyAgICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnQ6OmV4aXRGdWxsc2NyZWVuKToK KwogMjAyMC0wMy0yMCAgVGltIEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAog ICAgICAgICBVcHN0cmVhbSBhIHZhcmlldHkgb2YgQ29jb2EtcGxhdGZvcm0gSEFWRSBhbmQgRU5B QkxFIG1hY3JvcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVt ZW50LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAKaW5kZXgg YmRmNDU5YWU2MzZkMmYxM2I5MDQyZWIzODQ4MWY3ODkwZThjMzdkNC4uNzQ1NzEzZDVjMmYwZTk0 ZGNiNTgxYjUxMDY1YWM1M2M0NjBiN2Y2YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaHRt bC9IVE1MTWVkaWFFbGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRp YUVsZW1lbnQuY3BwCkBAIC02MTI0LDExICs2MTI0LDE4IEBAIHZvaWQgSFRNTE1lZGlhRWxlbWVu dDo6ZXhpdEZ1bGxzY3JlZW4oKQogICAgIGVsc2UKICNlbmRpZgogICAgIGlmIChkb2N1bWVudCgp LnBhZ2UoKS0+Y2hyb21lKCkuY2xpZW50KCkuc3VwcG9ydHNWaWRlb0Z1bGxzY3JlZW4ob2xkVmlk ZW9GdWxsc2NyZWVuTW9kZSkpIHsKKyAgICAgICAgLy8gU29tZSBwYWdlcyBtYXkgY2hhbmdlIHRo ZSBwb3NpdGlvbi9zaXplIG9mIGFuIGlubGluZSB2aWRlbyBlbGVtZW50CisgICAgICAgIC8vIHdo ZW4vYWZ0ZXIgdGhlIHZpZGVvIGVsZW1lbnQgZW50ZXJzIGZ1bGxzY3JlZW4gKHJkYXI6Ly9wcm9i bGVtLzU1ODE0OTg4KS4KKyAgICAgICAgLy8gV2UgbmVlZCB0byBmaXJlIHRoZSBlbmQgZnVsbHNj cmVlbiBldmVudCBlYXJseSB0byBub3RpZnkgdGhlIHBhZ2UKKyAgICAgICAgLy8gdG8gY2hhbmdl IHRoZSBwb3NpdGlvbi9zaXplIGJhY2sgYmVmb3JlIGV4aXRpbmcgZnVsbHNjcmVlbi4KKyAgICAg ICAgLy8gT3RoZXJ3aXNlLCB0aGUgZXhpdCBmdWxsc2NyZWVuIGFuaW1hdGlvbiB3aWxsIGJlIGlu Y29ycmVjdC4KKyAgICAgICAgc2NoZWR1bGVFdmVudChldmVudE5hbWVzKCkud2Via2l0ZW5kZnVs bHNjcmVlbkV2ZW50KTsKICAgICAgICAgaWYgKG1fdmlkZW9GdWxsc2NyZWVuU3RhbmRieSkKICAg ICAgICAgICAgIGRvY3VtZW50KCkucGFnZSgpLT5jaHJvbWUoKS5jbGllbnQoKS5lbnRlclZpZGVv RnVsbHNjcmVlbkZvclZpZGVvRWxlbWVudChkb3duY2FzdDxIVE1MVmlkZW9FbGVtZW50PigqdGhp cyksIG1fdmlkZW9GdWxsc2NyZWVuTW9kZSwgbV92aWRlb0Z1bGxzY3JlZW5TdGFuZGJ5KTsKICAg ICAgICAgZWxzZQotICAgICAgICAgICAgZG9jdW1lbnQoKS5wYWdlKCktPmNocm9tZSgpLmNsaWVu dCgpLmV4aXRWaWRlb0Z1bGxzY3JlZW5Gb3JWaWRlb0VsZW1lbnQoZG93bmNhc3Q8SFRNTFZpZGVv RWxlbWVudD4oKnRoaXMpKTsKLSAgICAgICAgc2NoZWR1bGVFdmVudChldmVudE5hbWVzKCkud2Vi a2l0ZW5kZnVsbHNjcmVlbkV2ZW50KTsKKyAgICAgICAgICAgIG1fZnVsbHNjcmVlblRhc2tRdWV1 ZS5lbnF1ZXVlVGFzayhbdGhpc10geworICAgICAgICAgICAgICAgIGRvY3VtZW50KCkucGFnZSgp LT5jaHJvbWUoKS5jbGllbnQoKS5leGl0VmlkZW9GdWxsc2NyZWVuRm9yVmlkZW9FbGVtZW50KGRv d25jYXN0PEhUTUxWaWRlb0VsZW1lbnQ+KCp0aGlzKSk7CisgICAgICAgICAgICB9KTsKICAgICB9 CiB9CiAK 394647 2020-03-26 13:08:31 -0700 2020-03-26 17:13:31 -0700 Revised the patch based on comments from Darin and Eric bug-209610-20200326130830.patch text/plain 2791 peng.liu6 U3VidmVyc2lvbiBSZXZpc2lvbjogMjU4NzcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYWNmMTlmNDA0MmYwZjgy YTZlZmNiYzRkNmVhYjg4MTlhZTdlZjVmMC4uM2RiZDAxZmMxODI5N2JlODc4ZWQ1MjVmMzBlOWUx OWM3MzVjZmEwMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDIwLTAzLTI2ICBQZW5n IExpdSAgPHBlbmcubGl1NkBhcHBsZS5jb20+CisKKyAgICAgICAgU3dpcGUgZG93biBnZXN0dXJl cyBjYXVzZSB0aGUgdmlkZW8gbGF5ZXIgdG8gc3RpY2sgZm9yIGEgbW9tZW50IGJlZm9yZSBib3Vu Y2luZyBiYWNrIGludG8gcGxhY2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTIwOTYxMAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIEZpeCBhbiBleGl0IGZ1bGxzY3JlZW4gYW5pbWF0aW9uIGlzc3VlIGJ5IGZp cmluZyB0aGUgZW5kIGZ1bGxzY3JlZW4gZXZlbnQKKyAgICAgICAgdG8gbGV0IHRoZSBwYWdlIGNo YW5nZSB0aGUgdmlkZW8gZWxlbWVudCBiYWNrIHRvIGl0cyBvcmlnaW5hbCBwb3NpdGlvbi9zaXpl CisgICAgICAgIGJlZm9yZSBleGl0aW5nIGZ1bGxzY3JlZW4uCisKKyAgICAgICAgQ292ZXJlZCBi eSBleGlzdGluZyB0ZXN0cy4KKworICAgICAgICAqIGh0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFFbGVtZW50OjpkaXNwYXRjaEV2ZW50KToKKyAg ICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnQ6OmV4aXRGdWxsc2NyZWVuKToKKwogMjAy MC0wMy0yMCAgVGltIEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAg ICBVcHN0cmVhbSBhIHZhcmlldHkgb2YgQ29jb2EtcGxhdGZvcm0gSEFWRSBhbmQgRU5BQkxFIG1h Y3JvcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNw cCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAKaW5kZXggYmRmNDU5 YWU2MzZkMmYxM2I5MDQyZWIzODQ4MWY3ODkwZThjMzdkNC4uZWQyNjI4MjMwZDkzMGM4M2FjNjU1 NmFjYjVkZjI1MWU1MDJjN2VhMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1M TWVkaWFFbGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVsZW1l bnQuY3BwCkBAIC01ODc3LDYgKzU4NzcsMTQgQEAgdm9pZCBIVE1MTWVkaWFFbGVtZW50OjpkaXNw YXRjaEV2ZW50KEV2ZW50JiBldmVudCkKICAgICAgICAgZG9jdW1lbnQoKS51c2VyQWN0aXZhdGVk TWVkaWFGaW5pc2hlZFBsYXlpbmcoKTsKIAogICAgIEhUTUxFbGVtZW50OjpkaXNwYXRjaEV2ZW50 KGV2ZW50KTsKKworICAgIC8vIFNvbWUgcGFnZXMgbWF5IGNoYW5nZSB0aGUgcG9zaXRpb24vc2l6 ZSBvZiBhbiBpbmxpbmUgdmlkZW8gZWxlbWVudAorICAgIC8vIHdoZW4vYWZ0ZXIgdGhlIHZpZGVv IGVsZW1lbnQgZW50ZXJzIGZ1bGxzY3JlZW4gKHJkYXI6Ly9wcm9ibGVtLzU1ODE0OTg4KS4KKyAg ICAvLyBXZSBuZWVkIHRvIGZpcmUgdGhlIGVuZCBmdWxsc2NyZWVuIGV2ZW50IHRvIG5vdGlmeSB0 aGUgcGFnZQorICAgIC8vIHRvIGNoYW5nZSB0aGUgcG9zaXRpb24vc2l6ZSBiYWNrICpiZWZvcmUq IGV4aXRpbmcgZnVsbHNjcmVlbi4KKyAgICAvLyBPdGhlcndpc2UsIHRoZSBleGl0IGZ1bGxzY3Jl ZW4gYW5pbWF0aW9uIHdpbGwgYmUgaW5jb3JyZWN0LgorICAgIGlmICghbV92aWRlb0Z1bGxzY3Jl ZW5TdGFuZGJ5ICYmIGV2ZW50LnR5cGUoKSA9PSBldmVudE5hbWVzKCkud2Via2l0ZW5kZnVsbHNj cmVlbkV2ZW50KQorICAgICAgICBkb2N1bWVudCgpLnBhZ2UoKS0+Y2hyb21lKCkuY2xpZW50KCku ZXhpdFZpZGVvRnVsbHNjcmVlbkZvclZpZGVvRWxlbWVudChkb3duY2FzdDxIVE1MVmlkZW9FbGVt ZW50PigqdGhpcykpOwogfQogCiBib29sIEhUTUxNZWRpYUVsZW1lbnQ6OmFkZEV2ZW50TGlzdGVu ZXIoY29uc3QgQXRvbVN0cmluZyYgZXZlbnRUeXBlLCBSZWY8RXZlbnRMaXN0ZW5lcj4mJiBsaXN0 ZW5lciwgY29uc3QgQWRkRXZlbnRMaXN0ZW5lck9wdGlvbnMmIG9wdGlvbnMpCkBAIC02MTI2LDgg KzYxMzQsNyBAQCB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6OmV4aXRGdWxsc2NyZWVuKCkKICAgICBp ZiAoZG9jdW1lbnQoKS5wYWdlKCktPmNocm9tZSgpLmNsaWVudCgpLnN1cHBvcnRzVmlkZW9GdWxs c2NyZWVuKG9sZFZpZGVvRnVsbHNjcmVlbk1vZGUpKSB7CiAgICAgICAgIGlmIChtX3ZpZGVvRnVs bHNjcmVlblN0YW5kYnkpCiAgICAgICAgICAgICBkb2N1bWVudCgpLnBhZ2UoKS0+Y2hyb21lKCku Y2xpZW50KCkuZW50ZXJWaWRlb0Z1bGxzY3JlZW5Gb3JWaWRlb0VsZW1lbnQoZG93bmNhc3Q8SFRN TFZpZGVvRWxlbWVudD4oKnRoaXMpLCBtX3ZpZGVvRnVsbHNjcmVlbk1vZGUsIG1fdmlkZW9GdWxs c2NyZWVuU3RhbmRieSk7Ci0gICAgICAgIGVsc2UKLSAgICAgICAgICAgIGRvY3VtZW50KCkucGFn ZSgpLT5jaHJvbWUoKS5jbGllbnQoKS5leGl0VmlkZW9GdWxsc2NyZWVuRm9yVmlkZW9FbGVtZW50 KGRvd25jYXN0PEhUTUxWaWRlb0VsZW1lbnQ+KCp0aGlzKSk7CisKICAgICAgICAgc2NoZWR1bGVF dmVudChldmVudE5hbWVzKCkud2Via2l0ZW5kZnVsbHNjcmVlbkV2ZW50KTsKICAgICB9CiB9Cg==