74279 2011-12-12 02:47:03 -0800 [GTK] fullscreen/full-screen-iframe-legacy.html is flaky 2012-03-06 09:47:45 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 pnormand pnormand eric.carlson mrobinson oldest_to_newest 519087 0 pnormand 2011-12-12 02:47:03 -0800 [GTK] fullscreen/full-screen-iframe-legacy.html is flaky 519088 1 118759 pnormand 2011-12-12 02:49:02 -0800 Created attachment 118759 Patch 571839 2 118759 eric.carlson 2012-03-06 07:24:40 -0800 Comment on attachment 118759 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118759&action=review > LayoutTests/ChangeLog:8 > + * fullscreen/full-screen-iframe-legacy.html: Wait canplaythrough Nit: "Wait for canplaythrough" > LayoutTests/fullscreen/full-screen-iframe-legacy.html:31 > + video.load(); This should be unnecessary, setting 'src' triggers a load automatically. 571877 3 pnormand 2012-03-06 08:20:32 -0800 (In reply to comment #2) > (From update of attachment 118759 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=118759&action=review > > > LayoutTests/ChangeLog:8 > > + * fullscreen/full-screen-iframe-legacy.html: Wait canplaythrough > > Nit: "Wait for canplaythrough" > > > LayoutTests/fullscreen/full-screen-iframe-legacy.html:31 > > + video.load(); > > This should be unnecessary, setting 'src' triggers a load automatically. Hum if I remove that call the test times out. I might be wrong but I think setting 'src' on the video is done before the canplaythrough listener is installed on the element. So doing a new load here ensures the event is fired. 571895 4 eric.carlson 2012-03-06 08:58:08 -0800 (In reply to comment #3) > (In reply to comment #2) > > > LayoutTests/fullscreen/full-screen-iframe-legacy.html:31 > > > + video.load(); > > > > This should be unnecessary, setting 'src' triggers a load automatically. > > Hum if I remove that call the test times out. I might be wrong but I think setting 'src' on the video is done before the canplaythrough listener is installed on the element. So doing a new load here ensures the event is fired. The video element is in another frame so I expect the 'canplaythrough' event could fire before or after you attach the listener. I think it would be slightly cleaner to set the 'src' from the main frame instead of calling load(), but I already r+'d this so it is your call. 571918 5 pnormand 2012-03-06 09:46:53 -0800 Committed r109928: <http://trac.webkit.org/changeset/109928> 571920 6 pnormand 2012-03-06 09:47:45 -0800 (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > > LayoutTests/fullscreen/full-screen-iframe-legacy.html:31 > > > > + video.load(); > > > > > > This should be unnecessary, setting 'src' triggers a load automatically. > > > > Hum if I remove that call the test times out. I might be wrong but I think setting 'src' on the video is done before the canplaythrough listener is installed on the element. So doing a new load here ensures the event is fired. > > The video element is in another frame so I expect the 'canplaythrough' event could fire before or after you attach the listener. I think it would be slightly cleaner to set the 'src' from the main frame instead of calling load(), but I already r+'d this so it is your call. Indeed much cleaner. I applied and tested this suggestion before landing. Thanks! 118759 2011-12-12 02:49:02 -0800 2012-03-06 07:24:40 -0800 Patch bug-74279-20111212114859.patch text/plain 2660 pnormand U3VidmVyc2lvbiBSZXZpc2lvbjogMTAyNTY0CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggMzBmZWM2ZTkyZjdlOTczOWM1YmUx YmU3ZjVmOGI0MzA5NTg0NTVkZi4uNGMwZTM0MDEwMjI5NmQxNzZiZjEzYWJkZWJlMDk2Nzg4OGQ5 ZWU0OCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE2IEBACiAyMDExLTEyLTEyICBQaGlsaXBwZSBOb3JtYW5k ICA8cG5vcm1hbmRAaWdhbGlhLmNvbT4KIAorICAgICAgICBbR1RLXSBmdWxsc2NyZWVuL2Z1bGwt c2NyZWVuLWlmcmFtZS1sZWdhY3kuaHRtbCBpcyBmbGFreQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzQyNzkKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGZ1bGxzY3JlZW4vZnVsbC1zY3JlZW4taWZyYW1l LWxlZ2FjeS5odG1sOiBXYWl0IGNhbnBsYXl0aHJvdWdoCisgICAgICAgIGV2ZW50IG9uIHRoZSB2 aWRlbyBhbmQgaW5jcmVhc2UgdGhlIHRpbWVvdXQgdG8gMTAwbXMgdG8gcmVkdWNlCisgICAgICAg IGZsYWtpbmVzcy4KKworMjAxMS0xMi0xMiAgUGhpbGlwcGUgTm9ybWFuZCAgPHBub3JtYW5kQGln YWxpYS5jb20+CisKICAgICAgICAgVW5yZXZpZXdlZCwgR1RLIGJhc2VsaW5lcyBmb3IgdGVzdHMg YWRkZWQgaW4gcjEwMjM4OC4KIAogICAgICAgICAqIHBsYXRmb3JtL2d0ay9hbmltYXRpb25zL2Ny b3NzLWZhZGUtYmFja2dyb3VuZC1pbWFnZS1leHBlY3RlZC50eHQ6IEFkZGVkLgpkaWZmIC0tZ2l0 IGEvTGF5b3V0VGVzdHMvZnVsbHNjcmVlbi9mdWxsLXNjcmVlbi1pZnJhbWUtbGVnYWN5Lmh0bWwg Yi9MYXlvdXRUZXN0cy9mdWxsc2NyZWVuL2Z1bGwtc2NyZWVuLWlmcmFtZS1sZWdhY3kuaHRtbApp bmRleCA3NzgzOGExZDc3MTJlZGIwMDY2YjMwOWYwY2E5MjRjNjJkMmU2OGE3Li5jZDk0ZDY4MWI3 M2EzMTFjNTBhOTk1ZGJiYjQ1ZGQwMmU5NGQ1NGI1IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9m dWxsc2NyZWVuL2Z1bGwtc2NyZWVuLWlmcmFtZS1sZWdhY3kuaHRtbAorKysgYi9MYXlvdXRUZXN0 cy9mdWxsc2NyZWVuL2Z1bGwtc2NyZWVuLWlmcmFtZS1sZWdhY3kuaHRtbApAQCAtMywyNCArMywz MyBAQCBIYW5kbGUgZW50ZXJpbmcgZnVsbCBzY3JlZW4gc2VjdXJpdHkgcmVzdHJpY3Rpb25zPC9w PgogPHA+VG8gdGVzdCBtYW51YWxseSwgY2xpY2sgdGhlIHZpZGVvICJmdWxsIHNjcmVlbiIgYnV0 dG9uIC0gdGhlIHBhZ2Ugc2hvdWxkIGVudGVyIGZ1bGwgc2NyZWVuIG1vZGUuPC9wPgogPHNjcmlw dCBzcmM9ImZ1bGwtc2NyZWVuLXRlc3QuanMiPjwvc2NyaXB0PgogPHNjcmlwdD4KLWZ1bmN0aW9u IHJ1blRlc3QoKSB7Ci0gICAgdmFyIGZyYW1lID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ2Zy YW1lJyk7CiAKLSAgICB3YWl0Rm9yRXZlbnQoZnJhbWUuY29udGVudERvY3VtZW50LCAnd2Via2l0 ZnVsbHNjcmVlbmNoYW5nZScsIGZ1bmN0aW9uKCkgeyAKLSAgICAgICAgICAgIHRlc3QoImRvY3Vt ZW50LmdldEVsZW1lbnRCeUlkKCdmcmFtZScpLmNvbnRlbnREb2N1bWVudC53aWR0aD09ZG9jdW1l bnQud2lkdGgiKQotICAgICAgICAgICAgZW5kVGVzdCgpOwotICAgIH0pOworZnVuY3Rpb24gY2Fu cGxheXRocm91Z2goKSB7CisgICAgdmFyIGZyYW1lID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQo J2ZyYW1lJyk7CiAKICAgICBydW5XaXRoS2V5RG93bihmdW5jdGlvbigpIHsKICAgICAgICAgc2V0 VGltZW91dChmdW5jdGlvbigpIHsgCiAgICAgICAgICAgICBjb25zb2xlV3JpdGUoIkZBSUwgLSBk aWQgbm90IGVudGVyIGZ1bGwgc2NyZWVuISIpOyAKICAgICAgICAgICAgIGVuZFRlc3QoKTsgCi0g ICAgICAgIH0sIDUwKTsKKyAgICAgICAgfSwgMTAwKTsKIAogICAgICAgICB2YXIgdmlkZW8gPSBm cmFtZS5jb250ZW50RG9jdW1lbnQuZ2V0RWxlbWVudHNCeVRhZ05hbWUoJ3ZpZGVvJylbMF07CiAg ICAgICAgIHZpZGVvLndlYmtpdEVudGVyRnVsbFNjcmVlbigpOwogICAgIH0pOwogfQorCitmdW5j dGlvbiBydW5UZXN0KCkgeworICAgIHZhciBmcmFtZSA9IGRvY3VtZW50LmdldEVsZW1lbnRCeUlk KCdmcmFtZScpOworCisgICAgd2FpdEZvckV2ZW50KGZyYW1lLmNvbnRlbnREb2N1bWVudCwgJ3dl YmtpdGZ1bGxzY3JlZW5jaGFuZ2UnLCBmdW5jdGlvbigpIHsgCisgICAgICAgICAgICB0ZXN0KCJk b2N1bWVudC5nZXRFbGVtZW50QnlJZCgnZnJhbWUnKS5jb250ZW50RG9jdW1lbnQud2lkdGg9PWRv Y3VtZW50LndpZHRoIikKKyAgICAgICAgICAgIGVuZFRlc3QoKTsKKyAgICB9KTsKKworICAgIHZh ciB2aWRlbyA9IGZyYW1lLmNvbnRlbnREb2N1bWVudC5nZXRFbGVtZW50c0J5VGFnTmFtZSgndmlk ZW8nKVswXTsKKyAgICB2aWRlby5hZGRFdmVudExpc3RlbmVyKCdjYW5wbGF5dGhyb3VnaCcsIGNh bnBsYXl0aHJvdWdoKTsKKyAgICB2aWRlby5sb2FkKCk7Cit9CiA8L3NjcmlwdD4KIDxpZnJhbWUg aWQ9ImZyYW1lIiBzcmM9InJlc291cmNlcy9sZWdhY3kuaHRtbCIgb25sb2FkPSJydW5UZXN0KCki IHdpZHRoPSIzMzYiIGhlaWdodD0iMjU2Ij4KIDwvaWZyYW1lPgo=