Closed Bug 1216308 Opened 7 years ago Closed 7 years ago

IsCallerChrome crash in HTMLMediaElement::Play

Categories

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

43 Branch
Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: dveditz, Assigned: bholley)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

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.
Thanks for filing this dan, patch coming up.
Summary: crash in nsContentUtils::IsCallerChrome → IsCallerChrome crash in HTMLMediaElement::Play
Blocks: 1072150
Assignee: nobody → bobbyholley
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
Attachment #8675914 - Flags: review?(bzbarsky) → review+
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?
Flags: needinfo?(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=.
Flags: needinfo?(bzbarsky)
https://hg.mozilla.org/mozilla-central/rev/11908baa6ee8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.