Bug 121537

Summary: Crashed while visit http://html5video.org/wiki/HTML5_Demos
Product: WebKit Reporter: Xueqing Huang <xqhuang.webkit>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: darin, gyuyoung.kim, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
patch for reviewing darin: review-, darin: commit-queue-

Description Xueqing Huang 2013-09-17 23:24:08 PDT 
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%
Comment 1 Xueqing Huang 2013-09-18 00:42:57 PDT 
Created attachment 211979 [details]
patch for reviewing
Comment 2 Xueqing Huang 2013-09-18 01:13:55 PDT 
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()|.
Comment 3 Xueqing Huang 2013-09-18 18:31:16 PDT 
Darin, could you take a look please?
Comment 4 Darin Adler 2013-09-18 18:39:24 PDT 
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?
Comment 5 Xueqing Huang 2013-09-18 19:23:07 PDT 
(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.
Comment 6 Andreas Kling 2013-09-18 19:24:48 PDT 
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?
Comment 7 Xueqing Huang 2013-09-18 19:29:30 PDT 
(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.
Comment 8 Darin Adler 2013-09-19 20:37:48 PDT 
(In reply to comment #7)
> isHTMLAudioElement(node) only check whether element has a tag name "audio"

That is the bug we have to fix.
Comment 9 Darin Adler 2013-09-19 20:38:18 PDT 
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.
Comment 10 Xueqing Huang 2013-09-19 20:39:51 PDT 
(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.
Comment 11 Zan Dobersek 2013-09-20 00:36:24 PDT 
Bug #120297 is trying to solve the same problem.
Comment 12 Xueqing Huang 2013-09-23 02:45:04 PDT 

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