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?
Created attachment 24946 [details] Preliminary patch for the first load() condition
Comment on attachment 24946 [details] Preliminary patch for the first load() condition Looks good. How about a test case?
Comment on attachment 24946 [details] Preliminary patch for the first load() condition Clearing review flag since this patch is lacking a test case and changelog.
No problem Eric, the patch was just for discussion. Assigning to me, and test case in the pipeline.
Created attachment 25740 [details] 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!
Comment on attachment 25740 [details] Patch with test case r=me
Committed in revision r40666. Confirmed media test cases pass with update.