157655 2016-05-12 19:39:15 -0700 Don't execute JavaScript within HTMLMediaElement::stop() 2016-07-30 00:12:54 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=160367 InRadar P2 Normal --- 1 adachan adachan cdumez dbates eric.carlson oldest_to_newest 1192588 0 adachan 2016-05-12 19:39:15 -0700 HTMLMediaElement::updateMediaControlsAfterPresentationModeChange() added in r200462 can be called by HTMLMediaElement::stop(). Executing JavaScript after the element has been stopped/suspended is not allowed and we need to fix that. <rdar://problem/26237934> 1192591 1 278802 adachan 2016-05-12 19:43:23 -0700 Created attachment 278802 Patch 1192619 2 278802 eric.carlson 2016-05-12 21:09:01 -0700 Comment on attachment 278802 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278802&action=review > Source/WebCore/html/HTMLMediaElement.cpp:6549 > + if (document().activeDOMObjectsAreSuspended() || document().activeDOMObjectsAreStopped()) You should also return if the controls script hasn't been injected yet, because there can't be a placeholder image of the script isn't running. 1192747 3 eric.carlson 2016-05-13 08:02:34 -0700 (In reply to comment #2) > Comment on attachment 278802 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=278802&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:6549 > > + if (document().activeDOMObjectsAreSuspended() || document().activeDOMObjectsAreStopped()) > > You should also return if the controls script hasn't been injected yet, > because there can't be a placeholder image of the script isn't running. IOW, there is no reason to inject the controls script just to tell it to hide an image that isn't visible. 1193466 4 adachan 2016-05-16 12:29:24 -0700 (In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 278802 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=278802&action=review > > > > > Source/WebCore/html/HTMLMediaElement.cpp:6549 > > > + if (document().activeDOMObjectsAreSuspended() || document().activeDOMObjectsAreStopped()) > > > > You should also return if the controls script hasn't been injected yet, > > because there can't be a placeholder image of the script isn't running. > > IOW, there is no reason to inject the controls script just to tell it to > hide an image that isn't visible. Thanks, I'll also bail early if !m_mediaControlsHost as you suggested. It turns out the suspended check doesn't really work because in ScriptExecutionContext::suspendActiveDOMObjects(), m_activeDOMObjectsAreSuspended is only set to true after suspending all objects. So we won't prevent script from executing while suspending the objects. I've talked to Chris Dumez about this, and he suggested moving the setting of that boolean to true earlier before suspending the objects. I'll be submitting a new patch for EWS. 1193467 5 279035 adachan 2016-05-16 12:30:38 -0700 Created attachment 279035 Patch 1193499 6 279035 cdumez 2016-05-16 13:35:09 -0700 Comment on attachment 279035 Patch r=me as long as Eric is fine with it as well. 1193500 7 cdumez 2016-05-16 13:36:12 -0700 We probably want to rename to boolean at some point to something like areActiveDOMObjectsSuspendingOrSuspended for clarity though. 1193511 8 adachan 2016-05-16 13:50:35 -0700 (In reply to comment #7) > We probably want to rename to boolean at some point to something like > areActiveDOMObjectsSuspendingOrSuspended for clarity though. Yes, Eric has seen the change and he said it looks OK as long as someone familiar with ScriptExecutionContext looks at it, which I guess it's you, Chris. :-) Change has been committed: http://trac.webkit.org/changeset/200965 Agreed with your suggestion on the rename. I think we should probably do the same for the stopping case too. I've filed https://bugs.webkit.org/show_bug.cgi?id=157753. 278802 2016-05-12 19:43:23 -0700 2016-05-16 12:30:34 -0700 Patch bug-157655-20160512194414.patch text/plain 1688 adachan U3VidmVyc2lvbiBSZXZpc2lvbjogMjAwNzk0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGJlYWQ3ZjQxNGU1ZjA1 YjViMDhiY2NmZTEwOGFjZGMzNWJmM2M4MC4uYzRmZTcyNDU1MTdlZTUyNmQ1ZTMzZGNhNDE4NGNh ZWNjNGZmNmIwZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE2IEBACiAyMDE2LTA1LTEyICBBZGEg Q2hhbiAgPGFkYWNoYW5AYXBwbGUuY29tPgogCisgICAgICAgIERvbid0IGV4ZWN1dGUgSmF2YVNj cmlwdCB3aXRoaW4gSFRNTE1lZGlhRWxlbWVudDo6c3RvcCgpCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTc2NTUKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGh0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFFbGVtZW50Ojp1cGRhdGVNZWRpYUNvbnRyb2xz QWZ0ZXJQcmVzZW50YXRpb25Nb2RlQ2hhbmdlKToKKyAgICAgICAgQmFpbCBlYXJseSBpZiB0aGUg RE9NIG9iamVjdCBoYXMgYmVlbiBzdG9wcGVkIG9yIHN1c3BlbmRlZC4KKworMjAxNi0wNS0xMiAg QWRhIENoYW4gIDxhZGFjaGFuQGFwcGxlLmNvbT4KKwogICAgICAgICBQcmV2ZW50IGNsaWNrcyBp biB0aGUgcGxhY2Vob2xkZXIgZnJvbSBwbGF5aW5nIG9yIHBhdXNpbmcgdGhlIHZpZGVvIGluIGEg TWVkaWFEb2N1bWVudC4KICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE1NzYyNQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRp YUVsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcApp bmRleCBkNTExOTA1MzM2ZGM4OTIwODA3NzJiZTkzNWRiNDE2NDBlYjUzMjc0Li40YjM4NGI4ZjA5 NTcxZThlMTQ1YWJlYjFlMWVlNTQwMWRjNDUyZmQzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRN TE1lZGlhRWxlbWVudC5jcHAKQEAgLTY1NDYsNiArNjU0Niw5IEBAIHZvaWQgSFRNTE1lZGlhRWxl bWVudDo6c2V0TWVkaWFDb250cm9sc0RlcGVuZE9uUGFnZVNjYWxlRmFjdG9yKGJvb2wgZGVwZW5k c09uUGFnCiAKIHZvaWQgSFRNTE1lZGlhRWxlbWVudDo6dXBkYXRlTWVkaWFDb250cm9sc0FmdGVy UHJlc2VudGF0aW9uTW9kZUNoYW5nZSgpCiB7CisgICAgaWYgKGRvY3VtZW50KCkuYWN0aXZlRE9N T2JqZWN0c0FyZVN1c3BlbmRlZCgpIHx8IGRvY3VtZW50KCkuYWN0aXZlRE9NT2JqZWN0c0FyZVN0 b3BwZWQoKSkKKyAgICAgICAgcmV0dXJuOworCiAgICAgRE9NV3JhcHBlcldvcmxkJiB3b3JsZCA9 IGVuc3VyZUlzb2xhdGVkV29ybGQoKTsKICAgICBTY3JpcHRDb250cm9sbGVyJiBzY3JpcHRDb250 cm9sbGVyID0gZG9jdW1lbnQoKS5mcmFtZSgpLT5zY3JpcHQoKTsKICAgICBKU0RPTUdsb2JhbE9i amVjdCogZ2xvYmFsT2JqZWN0ID0gSlNDOjpqc0Nhc3Q8SlNET01HbG9iYWxPYmplY3QqPihzY3Jp cHRDb250cm9sbGVyLmdsb2JhbE9iamVjdCh3b3JsZCkpOwo= 279035 2016-05-16 12:30:38 -0700 2016-05-16 13:35:09 -0700 Patch bug-157655-20160516123134.patch text/plain 3037 adachan U3VidmVyc2lvbiBSZXZpc2lvbjogMjAwNzk0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGJlYWQ3ZjQxNGU1ZjA1 YjViMDhiY2NmZTEwOGFjZGMzNWJmM2M4MC4uN2M3MGE4NDQ5OTI2Y2U2MWE3NWE3MmNlM2I0YWUx YjJhYzJjZjdhZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIxIEBACiAyMDE2LTA1LTEyICBBZGEg Q2hhbiAgPGFkYWNoYW5AYXBwbGUuY29tPgogCisgICAgICAgIERvbid0IGV4ZWN1dGUgSmF2YVNj cmlwdCB3aXRoaW4gSFRNTE1lZGlhRWxlbWVudDo6c3RvcCgpCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTc2NTUKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGRvbS9TY3JpcHRFeGVjdXRpb25Db250ZXh0 LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNjcmlwdEV4ZWN1dGlvbkNvbnRleHQ6OnN1c3BlbmRB Y3RpdmVET01PYmplY3RzKToKKyAgICAgICAgTW92ZSB0aGUgc2V0dGluZyBvZiBtX2FjdGl2ZURP TU9iamVjdHNBcmVTdXNwZW5kZWQgdG8gdHJ1ZSBlYXJsaWVyIHNvIHdlIHdvbid0IGV4ZWN1dGUK KyAgICAgICAgYW55IEpTIHdoaWxlIHN1c3BlbmRpbmcgdGhlIG9iamVjdHMuCisgICAgICAgICog aHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVs ZW1lbnQ6OnVwZGF0ZU1lZGlhQ29udHJvbHNBZnRlclByZXNlbnRhdGlvbk1vZGVDaGFuZ2UpOgor ICAgICAgICBCYWlsIGVhcmx5IGlmIHRoZSBjb250cm9scyBzY3JpcHQgaGFzbid0IGJlZW4gaW5q ZWN0ZWQgeWV0IG9yIHRoZSBET00gb2JqZWN0IGhhcyBiZWVuCisgICAgICAgIHN0b3BwZWQgb3Ig c3VzcGVuZGVkLgorCisyMDE2LTA1LTEyICBBZGEgQ2hhbiAgPGFkYWNoYW5AYXBwbGUuY29tPgor CiAgICAgICAgIFByZXZlbnQgY2xpY2tzIGluIHRoZSBwbGFjZWhvbGRlciBmcm9tIHBsYXlpbmcg b3IgcGF1c2luZyB0aGUgdmlkZW8gaW4gYSBNZWRpYURvY3VtZW50LgogICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU3NjI1CiAKZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJDb3JlL2RvbS9TY3JpcHRFeGVjdXRpb25Db250ZXh0LmNwcCBiL1NvdXJjZS9XZWJD b3JlL2RvbS9TY3JpcHRFeGVjdXRpb25Db250ZXh0LmNwcAppbmRleCAyNWRhMmQ2OTcxODczZDU4 NWJiNjA1OThhZWU0MWYyZmI5YzU1MjY3Li4yZTU2OGNhN2MxMmJiMWQ0YjA0OGM5NWYwOWJmNzM0 NWNkYmI4Njk3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vU2NyaXB0RXhlY3V0aW9u Q29udGV4dC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZG9tL1NjcmlwdEV4ZWN1dGlvbkNvbnRl eHQuY3BwCkBAIC0yMjQsNiArMjI0LDggQEAgdm9pZCBTY3JpcHRFeGVjdXRpb25Db250ZXh0Ojpz dXNwZW5kQWN0aXZlRE9NT2JqZWN0cyhBY3RpdmVET01PYmplY3Q6OlJlYXNvbkZvclMKICAgICAg ICAgcmV0dXJuOwogICAgIH0KIAorICAgIG1fYWN0aXZlRE9NT2JqZWN0c0FyZVN1c3BlbmRlZCA9 IHRydWU7CisKICAgICBtX2FjdGl2ZURPTU9iamVjdEFkZGl0aW9uRm9yYmlkZGVuID0gdHJ1ZTsK ICNpZiAhQVNTRVJUX0RJU0FCTEVEIHx8IEVOQUJMRShTRUNVUklUWV9BU1NFUlRJT05TKQogICAg IG1fYWN0aXZlRE9NT2JqZWN0UmVtb3ZhbEZvcmJpZGRlbiA9IHRydWU7CkBAIC0yNDIsNyArMjQ0 LDYgQEAgdm9pZCBTY3JpcHRFeGVjdXRpb25Db250ZXh0OjpzdXNwZW5kQWN0aXZlRE9NT2JqZWN0 cyhBY3RpdmVET01PYmplY3Q6OlJlYXNvbkZvclMKICAgICBtX2FjdGl2ZURPTU9iamVjdFJlbW92 YWxGb3JiaWRkZW4gPSBmYWxzZTsKICNlbmRpZgogCi0gICAgbV9hY3RpdmVET01PYmplY3RzQXJl U3VzcGVuZGVkID0gdHJ1ZTsKICAgICBtX3JlYXNvbkZvclN1c3BlbmRpbmdBY3RpdmVET01PYmpl Y3RzID0gd2h5OwogfQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRp YUVsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcApp bmRleCBkNTExOTA1MzM2ZGM4OTIwODA3NzJiZTkzNWRiNDE2NDBlYjUzMjc0Li41MGRhYTVhOWEw ZDJiMTFkZjgwZWU2NDI3MGRhZmRmOTdlNTBjMGEwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRN TE1lZGlhRWxlbWVudC5jcHAKQEAgLTY1NDYsNiArNjU0NiwxMSBAQCB2b2lkIEhUTUxNZWRpYUVs ZW1lbnQ6OnNldE1lZGlhQ29udHJvbHNEZXBlbmRPblBhZ2VTY2FsZUZhY3Rvcihib29sIGRlcGVu ZHNPblBhZwogCiB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6OnVwZGF0ZU1lZGlhQ29udHJvbHNBZnRl clByZXNlbnRhdGlvbk1vZGVDaGFuZ2UoKQogeworICAgIC8vIERvbid0IGV4ZWN1dGUgc2NyaXB0 IGlmIHRoZSBjb250cm9scyBzY3JpcHQgaGFzbid0IGJlZW4gaW5qZWN0ZWQgeWV0LCBvciB3ZSBo YXZlCisgICAgLy8gc3RvcHBlZC9zdXNwZW5kZWQgdGhlIG9iamVjdC4KKyAgICBpZiAoIW1fbWVk aWFDb250cm9sc0hvc3QgfHwgZG9jdW1lbnQoKS5hY3RpdmVET01PYmplY3RzQXJlU3VzcGVuZGVk KCkgfHwgZG9jdW1lbnQoKS5hY3RpdmVET01PYmplY3RzQXJlU3RvcHBlZCgpKQorICAgICAgICBy ZXR1cm47CisKICAgICBET01XcmFwcGVyV29ybGQmIHdvcmxkID0gZW5zdXJlSXNvbGF0ZWRXb3Js ZCgpOwogICAgIFNjcmlwdENvbnRyb2xsZXImIHNjcmlwdENvbnRyb2xsZXIgPSBkb2N1bWVudCgp LmZyYW1lKCktPnNjcmlwdCgpOwogICAgIEpTRE9NR2xvYmFsT2JqZWN0KiBnbG9iYWxPYmplY3Qg PSBKU0M6OmpzQ2FzdDxKU0RPTUdsb2JhbE9iamVjdCo+KHNjcmlwdENvbnRyb2xsZXIuZ2xvYmFs T2JqZWN0KHdvcmxkKSk7Cg==