Bug 62901 - Playing video from the manifest crashes on Windows
Summary: Playing video from the manifest crashes on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-06-17 12:45 PDT by Jer Noble
Modified: 2011-06-21 12:54 PDT (History)
1 user (show)

See Also:


Attachments
Patch (3.04 KB, patch)
2011-06-17 12:57 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (2.95 KB, patch)
2011-06-21 12:24 PDT, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-06-17 12:45:44 PDT
Playing video from the manifest crashes on Windows
Comment 1 Jer Noble 2011-06-17 12:57:09 PDT
Created attachment 97639 [details]
Patch
Comment 2 Jer Noble 2011-06-17 12:57:27 PDT
<rdar://problem/9631240>
Comment 3 Ada Chan 2011-06-17 13:04:46 PDT
One thing I overlooked: in the method above, QTMovie::loadPath(), should we null check cfURL and urlStringRef before releasing also?

Can use RetainPtr also.
Comment 4 Darin Adler 2011-06-17 13:16:17 PDT
Comment on attachment 97639 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:694
> +#if USE(CF) && PLATFORM(WIN)
> +    RetainPtr<CFStringRef> cfPath(AdoptCF, path.createCFString());
> +    RetainPtr<CFURLRef> cfURL(AdoptCF, CFURLCreateWithFileSystemPath(0, cfPath.get(), kCFURLWindowsPathStyle, false));
> +    KURL url(cfURL.get());
> +#else
>      KURL url;
>  
>      url.setProtocol("file");
>      url.setPath(path);
> +#endif

This is probably the best short term fix. But longer term we would like a helper function that does this.
Comment 5 Jer Noble 2011-06-17 14:02:32 PDT
(In reply to comment #3)
> One thing I overlooked: in the method above, QTMovie::loadPath(), should we null check cfURL and urlStringRef before releasing also?

Yes, we should.  This patch adds null-checks there as well.

> Can use RetainPtr also.

Unfortunately, we can't use RetainPtr from within the QTMovie subproject.  This is because of the (ridiculous) way QuickTime includes work; they have their own versions of CF headers which conflict with the AAS versions.
Comment 6 Jer Noble 2011-06-17 14:23:01 PDT
Committed r89172: <http://trac.webkit.org/changeset/89172>
Comment 7 Jer Noble 2011-06-21 12:24:38 PDT
Created attachment 98041 [details]
Patch