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: | P1 | Keywords: | InRadar | ||||||||||
Version: | 523.x (Safari 3) | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Attachments: |
|
Description
Mark Ambachtsheer
2007-03-19 11:56:37 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>). 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. 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.
My original understanding was that the GWorld code would not run during normal operation of Safari. Bruce, comment? > My original understanding was that the GWorld code would not run during normal
> operation of Safari.
Correct. This but will never happen in Safari.
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).
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.
Created attachment 13816 [details]
Proposed fix.
Forgot NP_NO_QUICKDRAW. Added.
The patch attached to bug 13026 contains a fix for this one as well, now. 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. |