Bug 121537 – Crashed while visit http://html5video.org/wiki/HTML5_Demos

• Component: WebCore Misc.
• Status: RESOLVED DUPLICATE

[Xueqing Huang]

OS: Windows 7
WebKit r155740

Steps To Reproduce:
1. Uninstall QuickTime(wasn't QuickTime SDK).
2. Open WinLauncher.exe;
3. Input "http://html5video.org/wiki/HTML5_Demos".

Expected Result:
Should load http://html5video.org/wiki/HTML5_Demos normally.

Actual Result:
Crash.

How frequently does this problem reproduce?
100%

[Xueqing Huang]

Created attachment 211979 [details]
patch for reviewing

[Xueqing Huang]

If QuickTime(wasn't QuickTime SDK) didn't installed, |MediaPlayer::isAvailable()| in |audioConstructor| return false then HTMLUnknownElement was created to insteated of HTMLAudioElement.
|isHTMLAudioElement(node)| in |isReachableFromDOM| just check whether element has "audio" tag name, |toHTMLAudioElement(node)| cast HTMLUnknownElement to HTMLAudioElement illegally then call |paused()|.

[Xueqing Huang]

Darin, could you take a look please?

[Darin Adler]

Comment on attachment 211979 [details]
patch for reviewing

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

> Source/WebCore/bindings/js/JSNodeCustom.cpp:115
> +            // If QuickTime didn't installed, |MediaPlayer::isAvailable()| return false in
> +            // |audioConstructor| then HTMLUnknowElement was created to instead HTMLAudioElement.

Why does paused return false for HTMLUnknownElement?

[Xueqing Huang]

(In reply to comment #4)
> (From update of attachment 211979 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211979&action=review
> 
> > Source/WebCore/bindings/js/JSNodeCustom.cpp:115
> > +            // If QuickTime didn't installed, |MediaPlayer::isAvailable()| return false in
> > +            // |audioConstructor| then HTMLUnknowElement was created to instead HTMLAudioElement.
> 
> Why does paused return false for HTMLUnknownElement?

The problem was HTMLUnknownElement has not paused() member function, We convert HTMLUnknownElement to HTMLAudioElement illegally then call paused() will crash.

[Andreas Kling]

Comment on attachment 211979 [details]
patch for reviewing

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

>>> Source/WebCore/bindings/js/JSNodeCustom.cpp:115
>>> +            // |audioConstructor| then HTMLUnknowElement was created to instead HTMLAudioElement.
>> 
>> Why does paused return false for HTMLUnknownElement?
> 
> The problem was HTMLUnknownElement has not paused() member function, We convert HTMLUnknownElement to HTMLAudioElement illegally then call paused() will crash.

How does that happen if the isHTMLAudioElement(node) check succeeded on the line just before?

[Xueqing Huang]

(In reply to comment #6)
> (From update of attachment 211979 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211979&action=review
> 
> >>> Source/WebCore/bindings/js/JSNodeCustom.cpp:115
> >>> +            // |audioConstructor| then HTMLUnknowElement was created to instead HTMLAudioElement.
> >> 
> >> Why does paused return false for HTMLUnknownElement?
> > 
> > The problem was HTMLUnknownElement has not paused() member function, We convert HTMLUnknownElement to HTMLAudioElement illegally then call paused() will crash.
> 
> How does that happen if the isHTMLAudioElement(node) check succeeded on the line just before?

isHTMLAudioElement(node) only check whether element has a tag name "audio", see HTMLElementTypeHelpers.h.
But |audioConstructor| in HTMLElementFactory.cpp create HTMLAudioElement failed since MediaPlayer::isAvailable() return false because QuickTime did not installed. HTMLUnknownELement was created as fallback, see HTMLElementFactory::createHTMLElement.

[Darin Adler]

(In reply to comment #7)
> isHTMLAudioElement(node) only check whether element has a tag name "audio"

That is the bug we have to fix.

[Darin Adler]

Comment on attachment 211979 [details]
patch for reviewing

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

>>>>> Source/WebCore/bindings/js/JSNodeCustom.cpp:115
>>>>> +            // |audioConstructor| then HTMLUnknowElement was created to instead HTMLAudioElement.
>>>> 
>>>> Why does paused return false for HTMLUnknownElement?
>>> 
>>> The problem was HTMLUnknownElement has not paused() member function, We convert HTMLUnknownElement to HTMLAudioElement illegally then call paused() will crash.
>> 
>> How does that happen if the isHTMLAudioElement(node) check succeeded on the line just before?
> 
> isHTMLAudioElement(node) only check whether element has a tag name "audio", see HTMLElementTypeHelpers.h.
> But |audioConstructor| in HTMLElementFactory.cpp create HTMLAudioElement failed since MediaPlayer::isAvailable() return false because QuickTime did not installed. HTMLUnknownELement was created as fallback, see HTMLElementFactory::createHTMLElement.

That is the bug we have to fix. We need to make isHTMLAudioElement return false in such cases.

[Xueqing Huang]

(In reply to comment #9)
> (From update of attachment 211979 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211979&action=review
> 
> >>>>> Source/WebCore/bindings/js/JSNodeCustom.cpp:115
> >>>>> +            // |audioConstructor| then HTMLUnknowElement was created to instead HTMLAudioElement.
> >>>> 
> >>>> Why does paused return false for HTMLUnknownElement?
> >>> 
> >>> The problem was HTMLUnknownElement has not paused() member function, We convert HTMLUnknownElement to HTMLAudioElement illegally then call paused() will crash.
> >> 
> >> How does that happen if the isHTMLAudioElement(node) check succeeded on the line just before?
> > 
> > isHTMLAudioElement(node) only check whether element has a tag name "audio", see HTMLElementTypeHelpers.h.
> > But |audioConstructor| in HTMLElementFactory.cpp create HTMLAudioElement failed since MediaPlayer::isAvailable() return false because QuickTime did not installed. HTMLUnknownELement was created as fallback, see HTMLElementFactory::createHTMLElement.
> 
> That is the bug we have to fix. We need to make isHTMLAudioElement return false in such cases.

All right. Thanks for clarification.

[Zan Dobersek]

Bug #120297 is trying to solve the same problem.

[Xueqing Huang]

*** This bug has been marked as a duplicate of bug 120297 ***