1347836 - Crash in nsLayoutUtils::SurfaceFromElement

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-53b199e7-30f5-4c3d-8c8a-e39c82170315.
=============================================================

There is 1 crash in nightly 55 with buildid 20170315030215. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1345403.

[1] https://hg.mozilla.org/mozilla-central/rev?node=99e3488b1ea4ed83a5b2233f2eb08defb6025cd4

Comment 1

[Tzuhao Kuo [:kaku]]

http://searchfox.org/mozilla-central/rev/571c1fd0ba0617f83175ccc06ed6f3eb0a1a8b92/dom/html/HTMLMediaElement.cpp#1546-1547

Here, in HTMLMediaElement::GetCurrentImage(), we lock images in the ImageContainer by a AutoLockImage and then return the first locked image as a raw pointer. While leaving HTMLMediaElement::GetCurrentImage(), the AutoLockImage is destroyed and so the images are un-locked which means the the returned raw pointer might be dangling.

We should instead return an already_AddRefed object.

Comment 2

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1347836 part 1 - fix comment;

Review commit: https://reviewboard.mozilla.org/r/120962/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/120962/

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1347836 part 2 - make HTMLMediaElement::GetCurrentImage() return an already_AddRefed<layers::Image>;

Review commit: https://reviewboard.mozilla.org/r/120964/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/120964/

Comment 4

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc80e404423db1ee21e89dec2f0532ed2adb359b
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8ba41a4f3c439ab54b2b34d2b556f38ee82e98e

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8848004 [details]
Bug 1347836 part 1 - fix comment;

https://reviewboard.mozilla.org/r/120962/#review122920

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8848005 [details]
Bug 1347836 part 2 - make HTMLMediaElement::GetCurrentImage() return an already_AddRefed<layers::Image>;

https://reviewboard.mozilla.org/r/120964/#review122922

Comment 7

[Tzuhao Kuo [:kaku]]

Try looks good and thanks for the review!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c475d5fd958
part 1 - fix comment; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/0b4a997d9fbb
part 2 - make HTMLMediaElement::GetCurrentImage() return an already_AddRefed<layers::Image>; r=jwwang

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6c475d5fd958
https://hg.mozilla.org/mozilla-central/rev/0b4a997d9fbb