22105 2008-11-06 10:14:02 -0800 Removing a media element from the document may trigger last second load() 2009-02-04 22:42:11 -0800 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) All All RESOLVED FIXED P3 Normal --- 1 vestbo vestbo adele koivisto oldest_to_newest 97829 0 vestbo 2008-11-06 10:14:02 -0800 Removing a media element from the document may trigger last-second load() The current spec says: "When a media element is removed from a Document, if the media element's networkState attribute has a value other than NETWORK_EMPTY then the user agent must act as if the pause() method had been invoked." And for pause(): "If the media element's networkState attribute has the value NETWORK_EMPTY, then the user agent must invoke the load() method and wait for it to return." So in effect, if we add a guard for NETWORK_EMPTY in HTMLMediaElement::removedFromDocument() we can get rid of one of the conditions where we load() during removedFromDocument(). The other condition is when there's no m_player. What I'm wondering is if we really need to load() if m_player is nil in pause() (and play for that matter) and/or if the networkState() == EMPTY check enough? Ie., will there be cases where networkState() > EMPTY but m_player is 0? 97832 1 24946 vestbo 2008-11-06 10:27:34 -0800 Created attachment 24946 Preliminary patch for the first load() condition 97859 2 24946 koivisto 2008-11-06 12:19:23 -0800 Comment on attachment 24946 Preliminary patch for the first load() condition Looks good. How about a test case? 101069 3 24946 eric 2008-12-02 14:16:58 -0800 Comment on attachment 24946 Preliminary patch for the first load() condition Clearing review flag since this patch is lacking a test case and changelog. 101152 4 vestbo 2008-12-02 23:40:26 -0800 No problem Eric, the patch was just for discussion. Assigning to me, and test case in the pipeline. 101322 5 25740 vestbo 2008-12-04 09:05:03 -0800 Created attachment 25740 Patch with test case Same check for networkState() != EMPTY but with test case. There's still a code path that can trigger this condition: if the element is removed from the document while m_player is 0, but I'm not sure when that will happen. We could guard for that too in removedFromDocument(), but would then miss out on the timeupdateEvent and pauseEvent sent from pause() (if that's even a code path we could hit). Any thoughts about this Antti? Thanks! 101324 6 25740 darin 2008-12-04 09:11:42 -0800 Comment on attachment 25740 Patch with test case r=me 108398 7 bfulgham 2009-02-04 22:42:11 -0800 Committed in revision r40666. Confirmed media test cases pass with update. 24946 2008-11-06 10:27:34 -0800 2008-12-04 09:05:03 -0800 Preliminary patch for the first load() condition last-second-load.patch text/plain 1025 vestbo Y29tbWl0IDk3OGM1MTBiNDMwZDdkODQwZTAxYjJkOGY2NTBmMDE4ODAzMzQyOWMKQXV0aG9yOiBU b3IgQXJuZSBWZXN0YsO4IDx0YXZlc3Rib0B0cm9sbHRlY2guY29tPgpEYXRlOiAgIFRodSBOb3Yg NiAxNzo1OTo0OSAyMDA4ICswMTAwCgogICAgUHJldmVudCBsYXN0LXNlY29uZCBsb2FkKCkgb2Yg bWVkaWEgZWxlbWVudCB3aGVuIHJlbW92ZWQgZnJvbSBkb2N1bWVudAogICAgCiAgICBIVE1MNSBz cGVjIHNwZWNpZmllcyB0aGF0IHBhdXNlKCkgc2hvdWxkIGJlIHNpbXVsYXRlZCBvbmx5IGlmIHRo ZSBtZWRpYQogICAgZWxlbWVudCdzIG5ldHdvcmtTdGF0ZSBhdHRyaWJ1dGUgaGFzIGEgdmFsdWUg b3RoZXIgdGhhbiBORVRXT1JLX0VNUFRZLgoKZGlmZiAtLWdpdCBhL1dlYkNvcmUvaHRtbC9IVE1M TWVkaWFFbGVtZW50LmNwcCBiL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcAppbmRl eCBmODZjNDRkLi45YjFiMWJhIDEwMDY0NAotLS0gYS9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxl bWVudC5jcHAKKysrIGIvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwCkBAIC0xMzQs MTAgKzEzNCwxMCBAQCB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6Omluc2VydGVkSW50b0RvY3VtZW50 KCkKIAogdm9pZCBIVE1MTWVkaWFFbGVtZW50OjpyZW1vdmVkRnJvbURvY3VtZW50KCkKIHsKLSAg ICAvLyBGSVhNRTogcGF1c2UoKSBtYXkgaW52b2tlIGxvYWQoKSB3aGljaCBzZWVtIGxpa2UgYSBz dHJhbmdlIHRoaW5nIHRvIGRvIGFzIGEgc2lkZSBlZmZlY3QKLSAgICAvLyBvZiByZW1vdmluZyBh biBlbGVtZW50LiBUaGlzIG1pZ2h0IG5lZWQgdG8gYmUgZml4ZWQgaW4gdGhlIHNwZWMuCi0gICAg RXhjZXB0aW9uQ29kZSBlYzsKLSAgICBwYXVzZShlYyk7CisgICAgaWYgKG5ldHdvcmtTdGF0ZSgp ICE9IEVNUFRZKSB7CisgICAgICAgIEV4Y2VwdGlvbkNvZGUgZWM7CisgICAgICAgIHBhdXNlKGVj KTsKKyAgICB9CiAgICAgSFRNTEVsZW1lbnQ6OnJlbW92ZWRGcm9tRG9jdW1lbnQoKTsKIH0KIAo= 25740 2008-12-04 09:05:03 -0800 2008-12-04 09:11:42 -0800 Patch with test case 0001-Make-sure-we-don-t-load-on-removing-the-media-elemen.patch text/plain 3948 vestbo RnJvbSA5YjI1MmJiNjUyMjQwZDk5YzE3ZTBhNDcyYTU4NTJlMTkzYzU4OGJkIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiA9P3V0Zi04P3E/VG9yPTIwQXJuZT0yMFZlc3RiPUMzPUI4Pz0g PHRhdmVzdGJvQHRyb2xsdGVjaC5jb20+CkRhdGU6IFRodSwgNCBEZWMgMjAwOCAxNzo1NTozNyAr MDEwMApTdWJqZWN0OiBbUEFUQ0hdIE1ha2Ugc3VyZSB3ZSBkb24ndCBsb2FkIG9uIHJlbW92aW5n IHRoZSBtZWRpYSBlbGVtZW50IGZyb20gdGhlIGRvY3VtZW50CgotLS0KIExheW91dFRlc3RzL0No YW5nZUxvZyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAgOSArKysrKysrKysKIC4u Li9yZW1vdmUtZnJvbS1kb2N1bWVudC1uby1sb2FkLWV4cGVjdGVkLnR4dCAgICAgIHwgICAgNyAr KysrKysrCiAuLi4vbWVkaWEvcmVtb3ZlLWZyb20tZG9jdW1lbnQtbm8tbG9hZC5odG1sICAgICAg ICB8ICAgMTIgKysrKysrKysrKysrCiBXZWJDb3JlL0NoYW5nZUxvZyAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICB8ICAgMTEgKysrKysrKysrKysKIFdlYkNvcmUvaHRtbC9IVE1MTWVk aWFFbGVtZW50LmNwcCAgICAgICAgICAgICAgICAgIHwgICAgOCArKysrLS0tLQogNSBmaWxlcyBj aGFuZ2VkLCA0MyBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQogY3JlYXRlIG1vZGUgMTAw NjQ0IExheW91dFRlc3RzL21lZGlhL3JlbW92ZS1mcm9tLWRvY3VtZW50LW5vLWxvYWQtZXhwZWN0 ZWQudHh0CiBjcmVhdGUgbW9kZSAxMDA2NDQgTGF5b3V0VGVzdHMvbWVkaWEvcmVtb3ZlLWZyb20t ZG9jdW1lbnQtbm8tbG9hZC5odG1sCgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9n IGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4IGNhZjFiMTYuLmM4ZmZiOTYgMTAwNjQ0Ci0t LSBhL0xheW91dFRlc3RzL0NoYW5nZUxvZworKysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAg LTEsMyArMSwxMiBAQAorMjAwOC0xMi0wNCAgVG9yIEFybmUgVmVzdGLDuCAgPHRhdmVzdGJvQHRy b2xsdGVjaC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgTWFrZSBzdXJlIHdlIGRvbid0IGxvYWQgb24gcmVtb3ZpbmcgdGhlIG1lZGlhIGVsZW1l bnQgZnJvbSB0aGUgZG9jdW1lbnQKKworICAgICAgICAqIG1lZGlhL3JlbW92ZS1mcm9tLWRvY3Vt ZW50LW5vLWxvYWQtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAgICAgKiBtZWRpYS9yZW1vdmUt ZnJvbS1kb2N1bWVudC1uby1sb2FkLmh0bWw6IEFkZGVkLgorCiAyMDA4LTEyLTAzICBHZW9mZnJl eSBHYXJlbiAgPGdnYXJlbkBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgT2xpdmVy IEh1bnQuCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9tZWRpYS9yZW1vdmUtZnJvbS1kb2N1bWVu dC1uby1sb2FkLWV4cGVjdGVkLnR4dCBiL0xheW91dFRlc3RzL21lZGlhL3JlbW92ZS1mcm9tLWRv Y3VtZW50LW5vLWxvYWQtZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAw MDAwMDAuLmNmYWMxMDMKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9tZWRpYS9yZW1v dmUtZnJvbS1kb2N1bWVudC1uby1sb2FkLWV4cGVjdGVkLnR4dApAQCAtMCwwICsxLDcgQEAKK1Rl c3QgdGhhdCByZW1vdmluZyBhIG1lZGlhIGVsZW1lbnQgZnJvbSB0aGUgdHJlZSB3aGVuIG5vIG1l ZGlhIGhhcyBiZWVuIGxvYWRlZCBkb2VzIG5vdCBnZW5lcmF0ZSBhIGxvYWQgZXZlbnQuCisKK1RF U1QodmlkZW8ubmV0d29ya1N0YXRlID09IEhUTUxNZWRpYUVsZW1lbnQuRU1QVFkpIE9LCitSVU4o ZG9jdW1lbnQuYm9keS5yZW1vdmVDaGlsZCh2aWRlbykpCitURVNUKGRpZFJlY2VpdmVMb2FkU3Rh cnRFdmVudCA9PSBmYWxzZTspIE9LCitFTkQgT0YgVEVTVAorCmRpZmYgLS1naXQgYS9MYXlvdXRU ZXN0cy9tZWRpYS9yZW1vdmUtZnJvbS1kb2N1bWVudC1uby1sb2FkLmh0bWwgYi9MYXlvdXRUZXN0 cy9tZWRpYS9yZW1vdmUtZnJvbS1kb2N1bWVudC1uby1sb2FkLmh0bWwKbmV3IGZpbGUgbW9kZSAx MDA2NDQKaW5kZXggMDAwMDAwMC4uM2Y4NDYyZgotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRl c3RzL21lZGlhL3JlbW92ZS1mcm9tLWRvY3VtZW50LW5vLWxvYWQuaHRtbApAQCAtMCwwICsxLDEy IEBACis8dmlkZW8gY29udHJvbHM+PC92aWRlbz4KKzxwPlRlc3QgdGhhdCByZW1vdmluZyBhIG1l ZGlhIGVsZW1lbnQgZnJvbSB0aGUgdHJlZSB3aGVuIG5vIG1lZGlhIGhhcyBiZWVuIGxvYWRlZCBk b2VzIG5vdCBnZW5lcmF0ZSBhIGxvYWQgZXZlbnQuPC9wPgorPHNjcmlwdCBzcmM9dmlkZW8tdGVz dC5qcz48L3NjcmlwdD4KKzxzY3JpcHQ+CitkaWRSZWNlaXZlTG9hZFN0YXJ0RXZlbnQgPSBmYWxz ZTsKK3ZpZGVvLmFkZEV2ZW50TGlzdGVuZXIoJ2xvYWRzdGFydCcsIGZ1bmN0aW9uKCkgeyBkaWRS ZWNlaXZlTG9hZFN0YXJ0RXZlbnQgPSB0cnVlOyB9KTsKK3ZpZGVvLnNyYyA9ICJjb250ZW50L3Rl c3QubXA0IjsKK3Rlc3QoInZpZGVvLm5ldHdvcmtTdGF0ZSA9PSBIVE1MTWVkaWFFbGVtZW50LkVN UFRZIik7CitydW4oImRvY3VtZW50LmJvZHkucmVtb3ZlQ2hpbGQodmlkZW8pIik7Cit0ZXN0KCJk aWRSZWNlaXZlTG9hZFN0YXJ0RXZlbnQgPT0gZmFsc2U7Iik7CitlbmRUZXN0KCk7Cis8L3Njcmlw dD4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5k ZXggN2ZlZDBkMi4uYTNkOWRmYiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIv V2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAwOC0xMi0wNCAgVG9yIEFybmUg VmVzdGLDuCAgPHRhdmVzdGJvQHRyb2xsdGVjaC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTWFrZSBzdXJlIHdlIGRvbid0IGxvYWQgb24gcmVt b3ZpbmcgdGhlIG1lZGlhIGVsZW1lbnQgZnJvbSB0aGUgZG9jdW1lbnQKKworICAgICAgICBUZXN0 OiBtZWRpYS9yZW1vdmUtZnJvbS1kb2N1bWVudC1uby1sb2FkLmh0bWwKKworICAgICAgICAqIGh0 bWwvSFRNTE1lZGlhRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFFbGVt ZW50OjpyZW1vdmVkRnJvbURvY3VtZW50KToKKwogMjAwOC0xMi0wNCAgRnJpZWRlbWFubiBLbGVp bnQgIDxmcmllZGVtYW5uLmtsZWludEBub2tpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkg VG9yIEFybmUgVmVzdGLDuC4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVt ZW50LmNwcCBiL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcAppbmRleCBkZjM4ZjAx Li44YTljZDVlIDEwMDY0NAotLS0gYS9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAK KysrIGIvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwCkBAIC0xMzcsMTAgKzEzNywx MCBAQCB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6Omluc2VydGVkSW50b0RvY3VtZW50KCkKIAogdm9p ZCBIVE1MTWVkaWFFbGVtZW50OjpyZW1vdmVkRnJvbURvY3VtZW50KCkKIHsKLSAgICAvLyBGSVhN RTogcGF1c2UoKSBtYXkgaW52b2tlIGxvYWQoKSB3aGljaCBzZWVtIGxpa2UgYSBzdHJhbmdlIHRo aW5nIHRvIGRvIGFzIGEgc2lkZSBlZmZlY3QKLSAgICAvLyBvZiByZW1vdmluZyBhbiBlbGVtZW50 LiBUaGlzIG1pZ2h0IG5lZWQgdG8gYmUgZml4ZWQgaW4gdGhlIHNwZWMuCi0gICAgRXhjZXB0aW9u Q29kZSBlYzsKLSAgICBwYXVzZShlYyk7CisgICAgaWYgKG5ldHdvcmtTdGF0ZSgpICE9IEVNUFRZ KSB7CisgICAgICAgIEV4Y2VwdGlvbkNvZGUgZWM7CisgICAgICAgIHBhdXNlKGVjKTsKKyAgICB9 CiAgICAgSFRNTEVsZW1lbnQ6OnJlbW92ZWRGcm9tRG9jdW1lbnQoKTsKIH0KIAotLSAKMS42LjAu cmMxLjI3Lmc5YjZiZgoK