Bug 36563 - A plug-in makes Safari crash on http://www.itscodingtime.com/
Summary: A plug-in makes Safari crash on http://www.itscodingtime.com/
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-03-24 15:43 PDT by Alexey Proskuryakov
Modified: 2010-03-25 09:54 PDT (History)
1 user (show)

See Also:


Attachments
proposed fix (12.64 KB, patch)
2010-03-24 17:23 PDT, Alexey Proskuryakov
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-03-24 15:43:06 PDT
Steps to reproduce: install Silverlight, go to http://www.itscodingtime.com/.

Here is what happens:
1) WebKit calls NetscapePluginInstanceProxy::resize(), and starts waiting for reply.
2) Instead of a reply, it gets an WKPCGetProperty call from plug-in first.
3) The object the plug-in wants to get a property of is actually a plug-in object, so WebKit calls plugin host via ProxyInstance::fieldNamed().
4) Silverlight crashes, taking plug-in host with it.
5) NetscapePluginHostProxy::pluginHostDied() destroys host proxy object, and clears all references to it.
6) WebKit returns to "event loop" in NetscapePluginInstanceProxy::processRequestsAndWaitForReply, waiting for a reply to resize().
7) But m_pluginHostProxy is null already, so it crashes.
Comment 1 Alexey Proskuryakov 2010-03-24 17:23:59 PDT
Created attachment 51566 [details]
proposed fix
Comment 2 WebKit Review Bot 2010-03-24 17:24:29 PDT
Attachment 51566 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.h:46:  More than one command on the same line  [whitespace/newline] [4]
WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.h:47:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Oliver Hunt 2010-03-24 17:30:34 PDT
Comment on attachment 51566 [details]
proposed fix


> +unsigned NetscapePluginHostProxy::s_processingRequests;
It's good form to initialise this.


In all honesty all the ++ and --'s make me wish we just had a struct to automatically increment and decrement this count.
Comment 4 Anders Carlsson 2010-03-24 17:50:29 PDT
Comment on attachment 51566 [details]
proposed fix

r=me
Comment 5 Alexey Proskuryakov 2010-03-24 17:53:51 PDT
Committed <http://trac.webkit.org/changeset/56474>.

>> +unsigned NetscapePluginHostProxy::s_processingRequests;
> It's good form to initialise this.

I'm not sure about that - initializing with 0 just adds visual noise.
Comment 6 Alexey Proskuryakov 2010-03-25 09:54:10 PDT
<rdar://problem/7786332>