Bug 36563

Summary: A plug-in makes Safari crash on http://www.itscodingtime.com/
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Plug-insAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P1 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
Attachments:
Description Flags
proposed fix andersca: review+

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>