[webkit-reviews] review granted: [Bug 187284] Nullptr crash accessing Document in GenericEventQueue::dispatchOneEvent() : [Attachment 344236] Fixes the crash

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 187284: Nullptr crash accessing Document in
GenericEventQueue::dispatchOneEvent()
https://bugs.webkit.org/show_bug.cgi?id=187284

Attachment 344236: Fixes the crash

https://bugs.webkit.org/attachment.cgi?id=344236&action=review




--- Comment #2 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 344236
  --> https://bugs.webkit.org/attachment.cgi?id=344236
Fixes the crash

View in context: https://bugs.webkit.org/attachment.cgi?id=344236&action=review

As mentioned in irc, I think you also need to block
HTMLMediaElement::prepareForLoad() and HTMLMediaElement::prepareToPlay() so
setting video.src and calling video.load() don't cause media loading and
related events.

It would be nice to have a test of a media element inside a template element,
although that can be added later if you prefer.

> Source/WebCore/ChangeLog:56
> +	   inside a stopped document, which should never is never correct and
causes this crash down the line.

Nit: "which should never is never correct"

> Source/WebCore/Modules/mediasource/MediaSource.cpp:696
> +    ASSERT(scriptExecutionContext());
> +    if (!scriptExecutionContext()->activeDOMObjectsAreStopped()) {

Nit: I can't tell from the diff, but can you change this to an early return?