Bug 35798 - Crash due to infinite recursion when viewing composited video on Windows
Summary: Crash due to infinite recursion when viewing composited video on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Chris Marrin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-03-05 10:48 PST by Chris Marrin
Modified: 2010-03-05 13:05 PST (History)
2 users (show)

See Also:


Attachments
Patch (25.54 KB, patch)
2010-03-05 11:59 PST, Chris Marrin
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2010-03-05 10:48:10 PST
Playing any video on Windows with HW compositing enabled will crash due to infinite recursion. Here's the top of the stack:

 	WebKit.dll!WebCore::MediaPlayerPrivate::notifySyncRequired(const WebCore::GraphicsLayer * __formal=0x0b955bd0)  Line 866 + 0xb bytes	C++
 	WebKit.dll!WebCore::GraphicsLayerCACF::notifySyncRequired()  Line 94 + 0x31 bytes	C++
 	WebKit.dll!WebCore::MediaPlayerPrivate::notifySyncRequired(const WebCore::GraphicsLayer * __formal=0x0b955bd0)  Line 869	C++
 	WebKit.dll!WebCore::GraphicsLayerCACF::notifySyncRequired()  Line 94 + 0x31 bytes	C++
 	WebKit.dll!WebCore::MediaPlayerPrivate::notifySyncRequired(const WebCore::GraphicsLayer * __formal=0x0b955bd0)  Line 869	C++

Proposed solution:
1. Make WKCACFLayer be a base class, with a virtual drawInContext() method
2. Subclass WKCACFLayer for GraphicsLayer (equivalent of WebLayer)
3. Subclass WKCACFLayer for the root, to allow the root layer to communicate with the WKCACFLayerRenderer.
4. Eliminate the notifySyncRequired callbacks.
Comment 1 Chris Marrin 2010-03-05 11:59:28 PST
Created attachment 50109 [details]
Patch
Comment 2 WebKit Review Bot 2010-03-05 12:01:28 PST
Attachment 50109 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/win/GraphicsLayerCACF.cpp:120:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Simon Fraser (smfr) 2010-03-05 12:10:30 PST
Comment on attachment 50109 [details]
Patch

> Index: WebCore/platform/graphics/win/GraphicsLayerCACF.cpp
> ===================================================================

> +class WebLayer : public WKCACFLayer {

> +private:
> +   GraphicsLayerCACF* m_owner;
> +};

Could WebLayer get away with having pointer to the base class (GraphicsLayer?). No point making it too incestuous with GraphicsLayerCACF if you can avoid it.

This this a great improvement to these classes!
Comment 4 Simon Fraser (smfr) 2010-03-05 12:35:38 PST
<rdar://problem/7706319>
Comment 5 Chris Marrin 2010-03-05 13:05:41 PST
Fixed in http://trac.webkit.org/changeset/55592