Bug 13120

Summary:

Plug-ins that draw through the QuickDraw interface may crash by hanging onto old GWorlds.

Product:

WebKit

Reporter:

Mark Ambachtsheer <mark.a>

Component:

Plug-ins

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

ap, bruceq, darin, mark.a

Priority:

Keywords:

InRadar

Version:

523.x (Safari 3)

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
stack trace of crash	none
Proposed fix.	none
Proposed fix.	none
Proposed fix.	none

Mark Ambachtsheer

Reported 2007-03-19 11:56:37 PDT

This is related to http://bugs.webkit.org/show_bug.cgi?id=12515. It turns out that Flash plugins in particular may hang onto the GWorlds that are generated during update events and re-use them when responding to other events (e.g. "data came back from the internet" events). In particular, this snippet will cause Flash to crash when WebKit draws inside a bitmap CGContext: <html> <head><title></title></head> <body> <script type="text/javascript" src="http://widgetserver.com/syndication/subscriber/InsertPanel.js?panelId=59c092a7-2fcb-418d-a633-40d76fac6bc5"></script> </body> The plugin hangs onto the GWorld pointer created by the new code written for http://bugs.webkit.org/show_bug.cgi?id=12515 . However, the new code disposes that GWorld immediately after the response to the initial event is complete. When the plugin gets some data back from the Internet, it tries to draw to that disposed GWorld. This is on Mac OS X 10.4.9 with WebKit 522+.

Attachments
stack trace of crash (1.60 KB, text/plain) 2007-03-19 12:54 PDT, Mark Ambachtsheer	no flags	Details
Proposed fix. (1.93 KB, patch) 2007-03-24 15:17 PDT, Mark Ambachtsheer	no flags	Details Formatted Diff Diff
Proposed fix. (2.11 KB, text/plain) 2007-03-25 16:46 PDT, Mark Ambachtsheer	no flags	Details
Proposed fix. (2.16 KB, patch) 2007-03-25 16:59 PDT, Mark Ambachtsheer	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2007-03-19 12:33:25 PDT

I cannot reproduce this with revision 20289. I have saved the snippet to a file, opened and reloaded it several times, but didn't get any crashes. Is there some trick necessary to reproduce this? Could you please post a backtrace? Changing priority/severity to match guidelines (Blocker is for bug that block WebKit development, see <http://bugs.webkit.org/page.cgi?id=fields.html#bug_severity>).

Mark Ambachtsheer

Comment 2 2007-03-19 12:44:06 PDT

Important to note that this will not reproduce in Safari-hosted WebKit which draws to onscreen CGContexts. It will only cause a crash when drawing to offscreen bitmap CGContextRefs as in http://bugs.webkit.org/show_bug.cgi?id=12515 . Also: I have a partial patch that prevents the crash, but I think additional window dirtying may be necessary in this case to notify the host that it should update from the bitmap context again. Can provide my unfinished patch on request.

Mark Ambachtsheer

Comment 3 2007-03-19 12:54:34 PDT

Created attachment 13706 [details] stack trace of crash This is a stack trace showing one variation on the crash. WebCore delivers data to Flash plugin; Flash plugin re-uses GWorldPtr that is no longer valid; Flash plugin crashes.

Darin Adler

Comment 4 2007-03-19 18:07:58 PDT

My original understanding was that the GWorld code would not run during normal operation of Safari. Bruce, comment?

Bruce Q. Hammond

Comment 5 2007-03-19 18:17:39 PDT

> My original understanding was that the GWorld code would not run during normal > operation of Safari. Correct. This but will never happen in Safari.

Mark Rowe (bdash)

Comment 6 2007-03-21 22:30:27 PDT

<rdar://problem/5080339>

Mark Ambachtsheer

Comment 7 2007-03-24 15:17:53 PDT

Created attachment 13800 [details] Proposed fix. The problem is that in the QuickDraw drawing model, plugins may draw to their port during any operation. So for example, when a plugin receives NPP_Write, it may choose to draw immediately. The previous bitmap GWorld implementation did not account for this; it setup a temporary GWorld before passing off to NPP_HandleEvent, then tore down the temporary GWorld when control was returned to WebKit. The solution is to keep the temporary GWorld around until the next temporary GWorld is created. In this way the plugin always has a valid GWorld to which it can draw. Unfortunately, when drawing to a bitmap GWorld in this way, there does seem to be an issue in detecting when the bitmap should be painted back on the host application's canvas. I have investigated several avenues to fix this but haven't come up with a good solution. That problem is not fixed by this patch (which I hope will be fine, since it wasn't covered by this bug in the first place).

Mark Ambachtsheer

Comment 8 2007-03-25 16:46:44 PDT

Created attachment 13815 [details] Proposed fix. Bruce rightly pointed out that the first patch would leak a GWorld when the view was destroyed. New patch that corrects the leak.

Mark Ambachtsheer

Comment 9 2007-03-25 16:59:46 PDT

Created attachment 13816 [details] Proposed fix. Forgot NP_NO_QUICKDRAW. Added.

Darin Adler

Comment 10 2007-03-28 09:31:50 PDT

The patch attached to bug 13026 contains a fix for this one as well, now.

Darin Adler

Comment 11 2007-04-02 14:32:44 PDT

Sadly, I couldn't come up with a good way to do regression tests. Sending WebKit/ChangeLog Sending WebKit/Plugins/WebBaseNetscapePluginView.h Sending WebKit/Plugins/WebBaseNetscapePluginView.mm Transmitting file data ... Committed revision 20668.

Note You need to log in before you can comment on or make changes to this bug.