Move plug-in enumeration back to the main thread
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Feb 2013 02:34:13 +0000 (02:34 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Feb 2013 02:34:13 +0000 (02:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=109337
<rdar://problem/12015046>

Reviewed by Andreas Kling.

Plug-in enumeration was moved to a separate work queue to improve responsiveness, but
doing so lead to crashes when WebKit1 would enumerate plug-ins on the main thread at the same time.
Bug <rdar://problem/13185819> tracks fixing the responsiveness issue by spawning a plug-in process
and have it do the enumeration.

* Shared/Plugins/Netscape/mac/NetscapePluginModuleMac.mm:
(WebKit::getPluginInfoFromCarbonResources):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::connectionWillOpen):
(WebKit::WebProcessProxy::connectionWillClose):
(WebKit::WebProcessProxy::getPlugins):
* UIProcess/WebProcessProxy.h:
(WebCore):
(WebProcessProxy):
* UIProcess/WebProcessProxy.messages.in:
* WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
(WebKit):
(WebKit::WebPlatformStrategies::populatePluginCache):
* WebProcess/WebProcess.cpp:
* WebProcess/WebProcess.h:
(WebProcess):
* WebProcess/WebProcess.messages.in:

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

Source/WebKit2/ChangeLog
Source/WebKit2/Shared/Plugins/Netscape/mac/NetscapePluginModuleMac.mm
Source/WebKit2/UIProcess/WebProcessProxy.cpp
Source/WebKit2/UIProcess/WebProcessProxy.h
Source/WebKit2/UIProcess/WebProcessProxy.messages.in
Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
Source/WebKit2/WebProcess/WebProcess.cpp
Source/WebKit2/WebProcess/WebProcess.h
Source/WebKit2/WebProcess/WebProcess.messages.in

index 8e0f06d007cb6aa935c0d47e5505bb7f28378c7d..3bf01e103408d37b4661301bc38771033ddf23c9 100644 (file)
@@ -1,3 +1,34 @@
+2013-02-08  Anders Carlsson  <andersca@apple.com>
+
+        Move plug-in enumeration back to the main thread
+        https://bugs.webkit.org/show_bug.cgi?id=109337
+        <rdar://problem/12015046>
+
+        Reviewed by Andreas Kling.
+
+        Plug-in enumeration was moved to a separate work queue to improve responsiveness, but
+        doing so lead to crashes when WebKit1 would enumerate plug-ins on the main thread at the same time.
+        Bug <rdar://problem/13185819> tracks fixing the responsiveness issue by spawning a plug-in process
+        and have it do the enumeration.
+
+        * Shared/Plugins/Netscape/mac/NetscapePluginModuleMac.mm:
+        (WebKit::getPluginInfoFromCarbonResources):
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::connectionWillOpen):
+        (WebKit::WebProcessProxy::connectionWillClose):
+        (WebKit::WebProcessProxy::getPlugins):
+        * UIProcess/WebProcessProxy.h:
+        (WebCore):
+        (WebProcessProxy):
+        * UIProcess/WebProcessProxy.messages.in:
+        * WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
+        (WebKit):
+        (WebKit::WebPlatformStrategies::populatePluginCache):
+        * WebProcess/WebProcess.cpp:
+        * WebProcess/WebProcess.h:
+        (WebProcess):
+        * WebProcess/WebProcess.messages.in:
+
 2013-02-08  Dean Jackson  <dino@apple.com>
 
         Snapshotted plug-in should use shadow root
index f2e6d433b391bd9875c83fe12e37ae0324c03c23..af7ef7d3db01ba8890a4ba347a961fa8f49d642a 100644 (file)
@@ -296,7 +296,7 @@ static const ResID PluginNameOrDescriptionStringNumber = 126;
 static const ResID MIMEDescriptionStringNumber = 127;
 static const ResID MIMEListStringStringNumber = 128;
 
-static bool getPluginInfoFromCarbonResourcesOnMainThread(CFBundleRef bundle, PluginModuleInfo& plugin)
+static bool getPluginInfoFromCarbonResources(CFBundleRef bundle, PluginModuleInfo& plugin)
 {
     ASSERT(isMainThread());
 
@@ -352,18 +352,6 @@ static bool getPluginInfoFromCarbonResourcesOnMainThread(CFBundleRef bundle, Plu
     return true;
 }
 
-static bool getPluginInfoFromCarbonResources(CFBundleRef bundle, PluginModuleInfo& plugin)
-{
-    if (isMainThread())
-        return getPluginInfoFromCarbonResourcesOnMainThread(bundle, const_cast<PluginModuleInfo&>(plugin));
-
-    __block bool gotPluginInfo = false;
-    dispatch_sync(dispatch_get_main_queue(), ^{
-        gotPluginInfo = getPluginInfoFromCarbonResourcesOnMainThread(bundle, const_cast<PluginModuleInfo&>(plugin));
-    });
-    return gotPluginInfo;
-}
-
 bool NetscapePluginModule::getPluginInfo(const String& pluginPath, PluginModuleInfo& plugin)
 {
     RetainPtr<CFURLRef> bundleURL = adoptCF(CFURLCreateWithFileSystemPath(kCFAllocatorDefault, pluginPath.createCFString().get(), kCFURLPOSIXPathStyle, false));
index fc81d615590839c5f7fc4c9ef096133241297fd9..ad62bbe0cd5c4ed2ce1394d1e778873bb358e3d3 100644 (file)
@@ -88,14 +88,6 @@ static WebProcessProxy::WebPageProxyMap& globalPageMap()
     return pageMap;
 }
 
-#if ENABLE(NETSCAPE_PLUGIN_API)
-static WorkQueue* pluginWorkQueue()
-{
-    static WorkQueue* pluginWorkQueue = WorkQueue::create("com.apple.WebKit.PluginQueue").leakRef();
-    return pluginWorkQueue;
-}
-#endif // ENABLE(NETSCAPE_PLUGIN_API)
-
 PassRefPtr<WebProcessProxy> WebProcessProxy::create(PassRefPtr<WebContext> context)
 {
     return adoptRef(new WebProcessProxy(context));
@@ -132,7 +124,6 @@ void WebProcessProxy::connectionWillOpen(CoreIPC::Connection* connection)
     ASSERT(this->connection() == connection);
 
     m_context->processWillOpenConnection(this);
-    connection->addQueueClient(this);
 }
 
 void WebProcessProxy::connectionWillClose(CoreIPC::Connection* connection)
@@ -140,7 +131,6 @@ void WebProcessProxy::connectionWillClose(CoreIPC::Connection* connection)
     ASSERT(this->connection() == connection);
 
     m_context->processWillCloseConnection(this);
-    connection->removeQueueClient(this);
 }
 
 void WebProcessProxy::disconnect()
@@ -308,49 +298,15 @@ void WebProcessProxy::addBackForwardItem(uint64_t itemID, const String& original
 }
 
 #if ENABLE(NETSCAPE_PLUGIN_API)
-void WebProcessProxy::sendDidGetPlugins(uint64_t requestID, PassOwnPtr<Vector<PluginInfo> > pluginInfos)
-{
-    ASSERT(isMainThread());
-
-    OwnPtr<Vector<PluginInfo> > plugins(pluginInfos);
-
-#if PLATFORM(MAC)
-    // Add built-in PDF last, so that it's not used when a real plug-in is installed.
-    // NOTE: This has to be done on the main thread as it calls localizedString().
-    if (!m_context->omitPDFSupport()) {
-#if ENABLE(PDFKIT_PLUGIN)
-        plugins->append(PDFPlugin::pluginInfo());
-#endif
-        plugins->append(SimplePDFPlugin::pluginInfo());
-    }
-#endif
-
-    send(Messages::WebProcess::DidGetPlugins(requestID, *plugins), 0);
-}
-
-void WebProcessProxy::handleGetPlugins(uint64_t requestID, bool refresh)
+void WebProcessProxy::getPlugins(bool refresh, Vector<PluginInfo>& plugins)
 {
     if (refresh)
         m_context->pluginInfoStore().refresh();
 
-    OwnPtr<Vector<PluginInfo> > pluginInfos = adoptPtr(new Vector<PluginInfo>);
-
-    {
-        Vector<PluginModuleInfo> plugins = m_context->pluginInfoStore().plugins();
-        for (size_t i = 0; i < plugins.size(); ++i)
-            pluginInfos->append(plugins[i].info);
-    }
-
-    // NOTE: We have to pass the PluginInfo vector to the secondary thread via a pointer as otherwise
-    //       we'd end up with a deref() race on all the WTF::Strings it contains.
-    RunLoop::main()->dispatch(bind(&WebProcessProxy::sendDidGetPlugins, this, requestID, pluginInfos.release()));
+    Vector<PluginModuleInfo> pluginModules = m_context->pluginInfoStore().plugins();
+    for (size_t i = 0; i < pluginModules.size(); ++i)
+        plugins.append(pluginModules[i].info);
 }
-
-void WebProcessProxy::getPlugins(CoreIPC::Connection*, uint64_t requestID, bool refresh)
-{
-    pluginWorkQueue()->dispatch(bind(&WebProcessProxy::handleGetPlugins, this, requestID, refresh));
-}
-
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
 
 #if ENABLE(PLUGIN_PROCESS)
@@ -420,18 +376,6 @@ void WebProcessProxy::didReceiveSyncMessage(CoreIPC::Connection* connection, Cor
     // FIXME: Add unhandled message logging.
 }
 
-void WebProcessProxy::didReceiveMessageOnConnectionWorkQueue(CoreIPC::Connection* connection, OwnPtr<CoreIPC::MessageDecoder>& decoder)
-{
-    if (decoder->messageReceiverName() == Messages::WebProcessProxy::messageReceiverName()) {
-        didReceiveWebProcessProxyMessageOnConnectionWorkQueue(connection, decoder);
-        return;
-    }
-}
-
-void WebProcessProxy::didCloseOnConnectionWorkQueue(CoreIPC::Connection*)
-{
-}
-
 void WebProcessProxy::didClose(CoreIPC::Connection*)
 {
     // Protect ourselves, as the call to disconnect() below may otherwise cause us
index d15eb817ac51fcd182e6c9bf79e17728ff0c01b2..3cdf8180684d9b55eb56cb41b6a7a985debd2ed3 100644 (file)
@@ -46,7 +46,8 @@
 #endif
 
 namespace WebCore {
-    class KURL;
+class KURL;
+struct PluginInfo;
 };
 
 namespace WebKit {
@@ -57,7 +58,7 @@ class WebContext;
 class WebPageGroup;
 struct WebNavigationDataStore;
 
-class WebProcessProxy : public ThreadSafeRefCounted<WebProcessProxy>, public ChildProcessProxy, ResponsivenessTimer::Client, CoreIPC::Connection::QueueClient {
+class WebProcessProxy : public ThreadSafeRefCounted<WebProcessProxy>, public ChildProcessProxy, ResponsivenessTimer::Client {
 public:
     typedef HashMap<uint64_t, RefPtr<WebBackForwardListItem> > WebBackForwardListItemMap;
     typedef HashMap<uint64_t, RefPtr<WebFrameProxy> > WebFrameProxyMap;
@@ -135,9 +136,7 @@ private:
 
     // Plugins
 #if ENABLE(NETSCAPE_PLUGIN_API)
-    void getPlugins(CoreIPC::Connection*, uint64_t requestID, bool refresh);
-    void handleGetPlugins(uint64_t requestID, bool refresh);
-    void sendDidGetPlugins(uint64_t requestID, PassOwnPtr<Vector<WebCore::PluginInfo> >);
+    void getPlugins(bool refresh, Vector<WebCore::PluginInfo>& plugins);
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
 #if ENABLE(PLUGIN_PROCESS)
     void getPluginProcessConnection(const String& pluginPath, uint32_t processType, PassRefPtr<Messages::WebProcessProxy::GetPluginProcessConnection::DelayedReply>);
@@ -159,10 +158,6 @@ private:
     virtual void didClose(CoreIPC::Connection*) OVERRIDE;
     virtual void didReceiveInvalidMessage(CoreIPC::Connection*, CoreIPC::StringReference messageReceiverName, CoreIPC::StringReference messageName) OVERRIDE;
 
-    // CoreIPC::Connection::QueueClient
-    virtual void didReceiveMessageOnConnectionWorkQueue(CoreIPC::Connection*, OwnPtr<CoreIPC::MessageDecoder>&) OVERRIDE;
-    virtual void didCloseOnConnectionWorkQueue(CoreIPC::Connection*) OVERRIDE;
-
     // ResponsivenessTimer::Client
     void didBecomeUnresponsive(ResponsivenessTimer*) OVERRIDE;
     void interactionOccurredWhileUnresponsive(ResponsivenessTimer*) OVERRIDE;
@@ -180,7 +175,6 @@ private:
     // Implemented in generated WebProcessProxyMessageReceiver.cpp
     void didReceiveWebProcessProxyMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&);
     void didReceiveSyncWebProcessProxyMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&, OwnPtr<CoreIPC::MessageEncoder>&);
-    void didReceiveWebProcessProxyMessageOnConnectionWorkQueue(CoreIPC::Connection*, OwnPtr<CoreIPC::MessageDecoder>&);
 
     ResponsivenessTimer m_responsivenessTimer;
     
index 4de93151fbcdd9c2ea9f2e6ee9be87b599056388..e135f25cb1a4436fb7663b0d9054c4eb92baa3e2 100644 (file)
@@ -35,7 +35,7 @@ messages -> WebProcessProxy LegacyReceiver {
 
     # Plugin messages.
 #if ENABLE(NETSCAPE_PLUGIN_API)
-    GetPlugins(uint64_t requestID, bool refresh) DispatchOnConnectionQueue
+    GetPlugins(bool refresh) -> (Vector<WebCore::PluginInfo> plugins)
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
 #if ENABLE(PLUGIN_PROCESS)
     GetPluginProcessConnection(WTF::String pluginPath, uint32_t processType) -> (CoreIPC::Attachment connectionHandle, bool supportsAsynchronousInitialization) Delayed
index 3ac1b63d29696654a0c7cc675c90373d5da96110..fb8aabe8fbe812e79c2105177fa9bab6e7d8c0df 100644 (file)
@@ -271,23 +271,6 @@ void WebPlatformStrategies::getPluginInfo(const WebCore::Page*, Vector<WebCore::
 }
 
 #if ENABLE(NETSCAPE_PLUGIN_API)
-static BlockingResponseMap<Vector<WebCore::PluginInfo> >& responseMap()
-{
-    AtomicallyInitializedStatic(BlockingResponseMap<Vector<WebCore::PluginInfo> >&, responseMap = *new BlockingResponseMap<Vector<WebCore::PluginInfo> >);
-    return responseMap;
-}
-
-void handleDidGetPlugins(uint64_t requestID, const Vector<WebCore::PluginInfo>& plugins)
-{
-    responseMap().didReceiveResponse(requestID, adoptPtr(new Vector<WebCore::PluginInfo>(plugins)));
-}
-
-static uint64_t generateRequestID()
-{
-    static int uniqueID;
-    return atomicIncrement(&uniqueID);
-}
-
 void WebPlatformStrategies::populatePluginCache()
 {
     if (m_pluginCacheIsPopulated)
@@ -296,11 +279,9 @@ void WebPlatformStrategies::populatePluginCache()
     ASSERT(m_cachedPlugins.isEmpty());
     
     // FIXME: Should we do something in case of error here?
-    uint64_t requestID = generateRequestID();
-    WebProcess::shared().connection()->send(Messages::WebProcessProxy::GetPlugins(requestID, m_shouldRefreshPlugins), 0);
+    if (!WebProcess::shared().connection()->sendSync(Messages::WebProcessProxy::GetPlugins(m_shouldRefreshPlugins), Messages::WebProcessProxy::GetPlugins::Reply(m_cachedPlugins), 0))
+        return;
 
-    m_cachedPlugins = *responseMap().waitForResponse(requestID);
-    
     m_shouldRefreshPlugins = false;
     m_pluginCacheIsPopulated = true;
 }
index 22b520e8a08f3c95344f2f6f233b5ecec9a1a6ee..69a1e1e7760f85a6c0439ac185dacfd44727ced2 100644 (file)
@@ -1050,16 +1050,6 @@ void WebProcess::setTextCheckerState(const TextCheckerState& textCheckerState)
     }
 }
 
-#if ENABLE(NETSCAPE_PLUGIN_API)
-void WebProcess::didGetPlugins(CoreIPC::Connection*, uint64_t requestID, const Vector<WebCore::PluginInfo>& plugins)
-{
-#if USE(PLATFORM_STRATEGIES)
-    // Pass this to WebPlatformStrategies.cpp.
-    handleDidGetPlugins(requestID, plugins);
-#endif
-}
-#endif // ENABLE(PLUGIN_PROCESS)
-
 #if !PLATFORM(MAC)
 void WebProcess::initializeProcessName(const ChildProcessInitializationParameters&)
 {
index 96bfaaf8f30ebd3631397cf11d57bc655395d5bb..83459f1c56b033e7f934be008da33bf54647ccf0 100644 (file)
@@ -279,10 +279,6 @@ private:
     void didReceiveWebProcessMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&);
     void didReceiveWebProcessMessageOnConnectionWorkQueue(CoreIPC::Connection*, OwnPtr<CoreIPC::MessageDecoder>&);
 
-#if ENABLE(NETSCAPE_PLUGIN_API)
-    void didGetPlugins(CoreIPC::Connection*, uint64_t requestID, const Vector<WebCore::PluginInfo>&);
-#endif
-
     RefPtr<WebConnectionToUIProcess> m_webConnection;
 
     HashMap<uint64_t, RefPtr<WebPage> > m_pageMap;
index 7a021a00cacf15b795e043bf2faca7e1e51d5169..e46d04ce163c5a82488a3764f2d870cff5fccf99 100644 (file)
@@ -56,9 +56,6 @@ messages -> WebProcess LegacyReceiver {
     DestroyPrivateBrowsingSession()
 
     # Plug-ins.
-#if ENABLE(NETSCAPE_PLUGIN_API)
-    DidGetPlugins(uint64_t requestID, Vector<WebCore::PluginInfo> plugins) DispatchOnConnectionQueue
-#endif
 #if ENABLE(NETSCAPE_PLUGIN_API) && !ENABLE(PLUGIN_PROCESS)
     GetSitesWithPluginData(Vector<WTF::String> pluginPaths, uint64_t callbackID)
     ClearPluginSiteData(Vector<WTF::String> pluginPaths, Vector<WTF::String> sites, uint64_t flags, uint64_t maxAgeInSeconds, uint64_t callbackID)