Fix UI process crash when a plug-in process crashes with a modal dialog showing

https://bugs.webkit.org/show_bug.cgi?id=81139
<rdar://problem/9641197>

Reviewed by Dan Bernstein.

When a plug-in process crashes, its corresponding PluginProcessProxy object is deleted immediately,
which is bad if we're currently running a nested run loop.

Fix this by making PluginProcessProxy ref-counted and protecting it before the call to -[NSApp runModalForWindow:].

* UIProcess/Plugins/PluginProcessManager.cpp:
(WebKit::PluginProcessManager::pluginProcessWithPath):
(WebKit::PluginProcessManager::getOrCreatePluginProcess):
* UIProcess/Plugins/PluginProcessManager.h:
(PluginProcessManager):
* UIProcess/Plugins/PluginProcessProxy.cpp:
(WebKit::PluginProcessProxy::create):
(WebKit::PluginProcessProxy::pluginProcessCrashedOrFailedToLaunch):
* UIProcess/Plugins/PluginProcessProxy.h:
(PluginProcessProxy):
* UIProcess/Plugins/mac/PluginProcessProxyMac.mm:
(WebKit::PluginProcessProxy::setModalWindowIsShowing):
(WebKit::PluginProcessProxy::beginModal):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@110728 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 30929d8..27342bf 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,30 @@
+2012-03-14  Anders Carlsson  <andersca@apple.com>
+
+        Fix UI process crash when a plug-in process crashes with a modal dialog showing
+        https://bugs.webkit.org/show_bug.cgi?id=81139
+        <rdar://problem/9641197>
+
+        Reviewed by Dan Bernstein.
+
+        When a plug-in process crashes, its corresponding PluginProcessProxy object is deleted immediately,
+        which is bad if we're currently running a nested run loop.
+
+        Fix this by making PluginProcessProxy ref-counted and protecting it before the call to -[NSApp runModalForWindow:].
+
+        * UIProcess/Plugins/PluginProcessManager.cpp:
+        (WebKit::PluginProcessManager::pluginProcessWithPath):
+        (WebKit::PluginProcessManager::getOrCreatePluginProcess):
+        * UIProcess/Plugins/PluginProcessManager.h:
+        (PluginProcessManager):
+        * UIProcess/Plugins/PluginProcessProxy.cpp:
+        (WebKit::PluginProcessProxy::create):
+        (WebKit::PluginProcessProxy::pluginProcessCrashedOrFailedToLaunch):
+        * UIProcess/Plugins/PluginProcessProxy.h:
+        (PluginProcessProxy):
+        * UIProcess/Plugins/mac/PluginProcessProxyMac.mm:
+        (WebKit::PluginProcessProxy::setModalWindowIsShowing):
+        (WebKit::PluginProcessProxy::beginModal):
+
 2012-03-14  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Handle printing errors in WebKit2

diff --git a/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp b/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp
index 35da365..cebf4f7 100644 (file)
--- a/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp
+++ b/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp
@@ -87,8 +87,9 @@ PluginProcessProxy* PluginProcessManager::pluginProcessWithPath(const String& pl
 {
     for (size_t i = 0; i < m_pluginProcesses.size(); ++i) {
         if (m_pluginProcesses[i]->pluginInfo().path == pluginPath)
-            return m_pluginProcesses[i];
+            return m_pluginProcesses[i].get();
     }
+
     return 0;
 }
 
@@ -97,10 +98,12 @@ PluginProcessProxy* PluginProcessManager::getOrCreatePluginProcess(const PluginM
     if (PluginProcessProxy* pluginProcess = pluginProcessWithPath(plugin.path))
         return pluginProcess;
 
-    PluginProcessProxy* pluginProcess = PluginProcessProxy::create(this, plugin).leakPtr();
-    m_pluginProcesses.append(pluginProcess);
+    RefPtr<PluginProcessProxy> pluginProcess = PluginProcessProxy::create(this, plugin);
+    PluginProcessProxy* pluginProcessPtr = pluginProcess.get();
+
+    m_pluginProcesses.append(pluginProcess.release());
 
-    return pluginProcess;
+    return pluginProcessPtr;
 }
 
 } // namespace WebKit

diff --git a/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h b/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h
index 935967b..19aafe8 100644 (file)
--- a/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h
+++ b/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h
@@ -64,7 +64,7 @@ private:
     PluginProcessProxy* getOrCreatePluginProcess(const PluginModuleInfo&);
     PluginProcessProxy* pluginProcessWithPath(const String& pluginPath);
 
-    Vector<PluginProcessProxy*> m_pluginProcesses;
+    Vector<RefPtr<PluginProcessProxy> > m_pluginProcesses;
 };
 
 } // namespace WebKit

diff --git a/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp b/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp
index 893f847..43fa2f4 100644 (file)
--- a/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp
+++ b/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp
@@ -47,9 +47,9 @@ using namespace WebCore;
 
 namespace WebKit {
 
-PassOwnPtr<PluginProcessProxy> PluginProcessProxy::create(PluginProcessManager* PluginProcessManager, const PluginModuleInfo& pluginInfo)
+PassRefPtr<PluginProcessProxy> PluginProcessProxy::create(PluginProcessManager* PluginProcessManager, const PluginModuleInfo& pluginInfo)
 {
-    return adoptPtr(new PluginProcessProxy(PluginProcessManager, pluginInfo));
+    return adoptRef(new PluginProcessProxy(PluginProcessManager, pluginInfo));
 }
 
 PluginProcessProxy::PluginProcessProxy(PluginProcessManager* PluginProcessManager, const PluginModuleInfo& pluginInfo)
@@ -151,9 +151,8 @@ void PluginProcessProxy::pluginProcessCrashedOrFailedToLaunch()
     while (!m_pendingClearSiteDataReplies.isEmpty())
         didClearSiteData(m_pendingClearSiteDataReplies.begin()->first);
 
-    // Tell the plug-in process manager to forget about this plug-in process proxy.
+    // Tell the plug-in process manager to forget about this plug-in process proxy. This may cause us to be deleted.
     m_pluginProcessManager->removePluginProcessProxy(this);
-    delete this;
 }
 
 void PluginProcessProxy::didReceiveMessage(CoreIPC::Connection* connection, CoreIPC::MessageID messageID, CoreIPC::ArgumentDecoder* arguments)

diff --git a/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.h b/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.h
index a75d5b1..e509e97 100644 (file)
--- a/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.h
+++ b/Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.h
@@ -52,9 +52,9 @@ class WebPluginSiteDataManager;
 class WebProcessProxy;
 struct PluginProcessCreationParameters;
 
-class PluginProcessProxy : CoreIPC::Connection::Client, ProcessLauncher::Client {
+class PluginProcessProxy : public RefCounted<PluginProcessProxy>, CoreIPC::Connection::Client, ProcessLauncher::Client {
 public:
-    static PassOwnPtr<PluginProcessProxy> create(PluginProcessManager*, const PluginModuleInfo&);
+    static PassRefPtr<PluginProcessProxy> create(PluginProcessManager*, const PluginModuleInfo&);
     ~PluginProcessProxy();
 
     const PluginModuleInfo& pluginInfo() const { return m_pluginInfo; }

diff --git a/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessProxyMac.mm b/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessProxyMac.mm
index 4e539be..290451a 100644 (file)
--- a/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessProxyMac.mm
+++ b/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessProxyMac.mm
@@ -202,7 +202,7 @@ void PluginProcessProxy::exitFullscreen()
 
 void PluginProcessProxy::setModalWindowIsShowing(bool modalWindowIsShowing)
 {
-    if (modalWindowIsShowing == m_modalWindowIsShowing) 
+    if (modalWindowIsShowing == m_modalWindowIsShowing)
         return;
     
     m_modalWindowIsShowing = modalWindowIsShowing;
@@ -223,7 +223,11 @@ void PluginProcessProxy::beginModal()
     
     m_activationObserver = [[NSNotificationCenter defaultCenter] addObserverForName:NSApplicationWillBecomeActiveNotification object:NSApp queue:nil
                                                                          usingBlock:^(NSNotification *){ applicationDidBecomeActive(); }];
-    
+
+    // The call to -[NSApp runModalForWindow:] below will run a nested run loop, and if the plug-in process
+    // crashes the PluginProcessProxy object can be destroyed. Protect against this here.
+    RefPtr<PluginProcessProxy> protect(this);
+
     [NSApp runModalForWindow:m_placeholderWindow.get()];
     
     [m_placeholderWindow.get() orderOut:nil];