1216308 - IsCallerChrome crash in HTMLMediaElement::Play

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Veditz [:dveditz]]

This bug was filed from the Socorro interface and is 
report bp-79e40c32-e0d9-45ad-8d51-ea9402151019.
=============================================================

This has the same signature as bug 1216072 that bholley has a patch for, and bug 1215398 which is patched on my build. It has a different stack though: nsContentUtils::IsCallerChrome is not called from nsGlobalWindow::MoveByOuter but from mozilla::dom::HTMLMediaElement::Play(mozilla::ErrorResult&). That call was added by cpearce in bug 1180563 in July and I haven't noticed this crash until now.

Steps are pretty reliable (and relevant to bug 1189563), at least on my Mac OS X system -- haven't tried elsewhere.

1. Command-Click on a youtube link to open in the background (doesn't seem to matter which)  https://www.youtube.com/watch?v=4niz8TfY794
2. wait several seconds to make sure it's all loaded
3. click on the new tab -- boom.

Expected results: get to watch a video
Actual results: get reacquainted with about:sessionrestore

If I normal-click the link it works fine, as does creating a new tab and then pasting the URL into the current tab.

Comment 1

[Bobby Holley (:bholley)]

Thanks for filing this dan, patch coming up.

Comment 2

[Bobby Holley (:bholley)]

Attachment: Hoist IsCallerChrome check in HTMLMediaElement::Play to API entry point. v1

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8675914 [details] [diff] [review]
Hoist IsCallerChrome check in HTMLMediaElement::Play to API entry point. v1

I'd really rather we kept PlayInternal as a void method taking an ErrorResult arg.  I keep hoping people will stop using the intended-to-be-transitional operator= on ErrorResult...

r=me

Comment 4

[Bobby Holley (:bholley)]

I did it this way because things that take ErrorResult& look like WebIDL APIs, and I was trying to distinguish them that way. Would you prefer for me to switch it?

Comment 5

[Boris Zbarsky [:bzbarsky]]

I guess I can live with this returning nsresult, esp. if we make the caller use an nsresult temporary and Throw it if NS_FAILED instead of using operator=.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11908baa6ee8

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/11908baa6ee8

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/11908baa6ee8

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/924e0fbf9726

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/924e0fbf9726