Fix <rdar://5517707> Crash on wptv.wp.pl when "make bigger" button is clicked
authoraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2007 01:07:19 +0000 (01:07 +0000)
committeraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2007 01:07:19 +0000 (01:07 +0000)
WebCore:

        Fix <rdar://5517707> Crash on wptv.wp.pl when "make bigger" button is clicked

        Windows Media Player has a modal message loop that will deliver
        messages to us at inappropriate times and we will crash if we handle
        them when they are delivered. In PluginViewWin, we add a quirk for
        Media Player to set a flag whenever we give the plugin a chance to
        execute code, and in SharedTimerWin we check if the plugin is
        executing code and repost messages if so.

        Reviewed by Anders.

        * platform/win/SharedTimerWin.cpp:
        (WebCore::TimerWindowWndProc): Repost messages if we're calling a
        plugin.
        * plugins/win/PluginViewWin.cpp: Surround all calls to the plugin with
        setCallingPlugin(true/false).
        (WebCore::PluginViewWin::updateWindow):
        (WebCore::PluginViewWin::dispatchNPEvent):
        (WebCore::PluginViewWin::setNPWindowRect):
        (WebCore::PluginViewWin::start):
        (WebCore::PluginViewWin::stop):
        (WebCore::PluginViewWin::performRequest):
        (WebCore::PluginViewWin::bindingInstance):
        (WebCore::PluginViewWin::determineQuirks):
        (WebCore::PluginViewWin::setCallingPlugin): Added.
        (WebCore::PluginViewWin::isCallingPlugin): Added.
        * plugins/win/PluginViewWin.h: Added a new quirk.

WebKit/win:

        Fix <rdar://5517707> Crash on wptv.wp.pl when "make bigger" button is clicked

        Reviewed by Anders.

        * WebView.cpp:
        (WebViewWndProc): Repost paint messages and ignore all other messages
        when we're calling a plugin.

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

WebCore/ChangeLog
WebCore/platform/win/SharedTimerWin.cpp
WebCore/plugins/win/PluginViewWin.cpp
WebCore/plugins/win/PluginViewWin.h
WebKit/win/ChangeLog
WebKit/win/WebView.cpp

index d9f5297..d7a802a 100644 (file)
@@ -1,3 +1,33 @@
+2007-12-13  Adam Roben  <aroben@apple.com>
+
+        Fix <rdar://5517707> Crash on wptv.wp.pl when "make bigger" button is clicked
+
+        Windows Media Player has a modal message loop that will deliver
+        messages to us at inappropriate times and we will crash if we handle
+        them when they are delivered. In PluginViewWin, we add a quirk for
+        Media Player to set a flag whenever we give the plugin a chance to
+        execute code, and in SharedTimerWin we check if the plugin is
+        executing code and repost messages if so.
+
+        Reviewed by Anders.
+
+        * platform/win/SharedTimerWin.cpp:
+        (WebCore::TimerWindowWndProc): Repost messages if we're calling a
+        plugin.
+        * plugins/win/PluginViewWin.cpp: Surround all calls to the plugin with
+        setCallingPlugin(true/false).
+        (WebCore::PluginViewWin::updateWindow):
+        (WebCore::PluginViewWin::dispatchNPEvent):
+        (WebCore::PluginViewWin::setNPWindowRect):
+        (WebCore::PluginViewWin::start):
+        (WebCore::PluginViewWin::stop):
+        (WebCore::PluginViewWin::performRequest):
+        (WebCore::PluginViewWin::bindingInstance):
+        (WebCore::PluginViewWin::determineQuirks):
+        (WebCore::PluginViewWin::setCallingPlugin): Added.
+        (WebCore::PluginViewWin::isCallingPlugin): Added.
+        * plugins/win/PluginViewWin.h: Added a new quirk.
+
 2007-12-13  Alp Toker  <alp@atoker.com>
 
         Add a missing DEPENDPATH. Fixes non-clean builds following networking
index 1ebc255..8ff58d2 100644 (file)
@@ -27,6 +27,7 @@
 #include "SharedTimer.h"
 
 #include "Page.h"
+#include "PluginViewWin.h"
 #include "SystemTime.h"
 #include "Widget.h"
 #include <wtf/Assertions.h>
@@ -45,6 +46,15 @@ const int sharedTimerID = 1000;
 
 LRESULT CALLBACK TimerWindowWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
+    // Windows Media Player has a modal message loop that will deliver messages
+    // to us at inappropriate times and we will crash if we handle them when
+    // they are delivered. We repost all messages so that we will get to handle
+    // them once the modal loop exits.
+    if (PluginViewWin::isCallingPlugin()) {
+        PostMessage(hWnd, message, wParam, lParam);
+        return 0;
+    }
+
     if (message == timerFiredMessage) {
         processingCustomTimerMessage = true;
         sharedTimerFiredFunction();
index ae27139..c4087e8 100644 (file)
@@ -96,6 +96,7 @@ private:
 };
 
 static const double MessageThrottleTimeInterval = 0.001;
+static int s_callingPlugin;
 
 class PluginMessageThrottlerWin {
 public:
@@ -345,6 +346,7 @@ void PluginViewWin::updateWindow() const
         //rgn = ::CreateRectRgn(0, 0, 0, 0);
         //::SetWindowRgn(m_window, rgn, false);
 
+        setCallingPlugin(true);
         if (m_windowRect != oldWindowRect)
             ::MoveWindow(m_window, m_windowRect.x(), m_windowRect.y(), m_windowRect.width(), m_windowRect.height(), true);
 
@@ -352,6 +354,7 @@ void PluginViewWin::updateWindow() const
         rgn = ::CreateRectRgn(m_clipRect.x(), m_clipRect.y(), m_clipRect.right(), m_clipRect.bottom());
         ::SetWindowRgn(m_window, rgn, true);
         ::UpdateWindow(m_window);
+        setCallingPlugin(false);
     }
 }
 
@@ -448,7 +451,9 @@ bool PluginViewWin::dispatchNPEvent(NPEvent* npEvent)
     }
 
     KJS::JSLock::DropAllLocks dropAllLocks;
+    setCallingPlugin(true);
     bool result = m_plugin->pluginFuncs()->event(m_instance, &npEvent);
+    setCallingPlugin(false);
 
     if (shouldPop) 
         popPopupsEnabledState();
@@ -679,7 +684,9 @@ void PluginViewWin::setNPWindowRect(const IntRect& rect)
 
     if (m_plugin->pluginFuncs()->setwindow) {
         KJS::JSLock::DropAllLocks dropAllLocks;
+        setCallingPlugin(true);
         m_plugin->pluginFuncs()->setwindow(m_instance, &m_npWindow);
+        setCallingPlugin(false);
 
         if (!m_isWindowed)
             return;
@@ -704,7 +711,9 @@ bool PluginViewWin::start()
     PluginViewWin::setCurrentPluginView(this);
     {
         KJS::JSLock::DropAllLocks dropAllLocks;
+        setCallingPlugin(true);
         npErr = m_plugin->pluginFuncs()->newp((NPMIMEType)m_mimeType.data(), m_instance, m_mode, m_paramCount, m_paramNames, m_paramValues, NULL);
+        setCallingPlugin(false);
         LOG_NPERROR(npErr);
     }
     PluginViewWin::setCurrentPluginView(0);
@@ -752,12 +761,17 @@ void PluginViewWin::stop()
 
     // Clear the window
     m_npWindow.window = 0;
-    if (m_plugin->pluginFuncs()->setwindow)
+    if (m_plugin->pluginFuncs()->setwindow) {
+        setCallingPlugin(true);
         m_plugin->pluginFuncs()->setwindow(m_instance, &m_npWindow);
+        setCallingPlugin(false);
+    }
 
     // Destroy the plugin
     NPSavedData* savedData = 0;
+    setCallingPlugin(true);
     NPError npErr = m_plugin->pluginFuncs()->destroy(m_instance, &savedData);
+    setCallingPlugin(false);
     LOG_NPERROR(npErr);
 
     if (savedData) {
@@ -832,7 +846,9 @@ void PluginViewWin::performRequest(PluginRequestWin* request)
             // FIXME: <rdar://problem/4807469> This should be sent when the document has finished loading
             if (request->sendNotification()) {
                 KJS::JSLock::DropAllLocks dropAllLocks;
+                setCallingPlugin(true);
                 m_plugin->pluginFuncs()->urlnotify(m_instance, requestURL.deprecatedString().utf8(), NPRES_DONE, request->notifyData());
+                setCallingPlugin(false);
             }
         }
         return;
@@ -1389,7 +1405,9 @@ KJS::Bindings::Instance* PluginViewWin::bindingInstance()
     NPError npErr;
     {
         KJS::JSLock::DropAllLocks dropAllLocks;
+        setCallingPlugin(true);
         npErr = m_plugin->pluginFuncs()->getvalue(m_instance, NPPVpluginScriptableNPObject, &object);
+        setCallingPlugin(false);
     }
 
     if (npErr != NPERR_NO_ERROR || !object)
@@ -1445,6 +1463,13 @@ void PluginViewWin::determineQuirks(const String& mimeType)
         // Windowless mode does not work at all with the WMP plugin so just remove that parameter 
         // and don't pass it to the plug-in.
         m_quirks |= PluginQuirkRemoveWindowlessVideoParam;
+
+        // WMP has a modal message loop that it enters whenever we call it or
+        // ask it to paint. This modal loop can deliver messages to other
+        // windows in WebKit at times when they are not expecting them (for
+        // example, delivering a WM_PAINT message during a layout), and these
+        // can cause crashes.
+        m_quirks |= PluginQuirkHasModalMessageLoop;
     }
 
     // The DivX plugin sets its size on the first NPP_SetWindow call and never updates its size, so
@@ -1622,4 +1647,22 @@ void PluginViewWin::didFail(const ResourceError& error)
     m_manualStream->didFail(0, error);
 }
 
+void PluginViewWin::setCallingPlugin(bool b) const
+{
+    if (!(m_quirks & PluginQuirkHasModalMessageLoop))
+        return;
+
+    if (b)
+        ++s_callingPlugin;
+    else
+        --s_callingPlugin;
+
+    ASSERT(s_callingPlugin >= 0);
+}
+
+bool PluginViewWin::isCallingPlugin()
+{
+    return s_callingPlugin > 0;
+}
+
 } // namespace WebCore
index c1f58b2..878d3de 100644 (file)
@@ -70,7 +70,8 @@ namespace WebCore {
         PluginQuirkRemoveWindowlessVideoParam = 1 << 3,
         PluginQuirkThrottleWMUserPlusOneMessages = 1 << 4,
         PluginQuirkDontUnloadPlugin = 1 << 5,
-        PluginQuirkDontCallWndProcForSameMessageRecursively = 1 << 6
+        PluginQuirkDontCallWndProcForSameMessageRecursively = 1 << 6,
+        PluginQuirkHasModalMessageLoop = 1 << 7
     };
 
     enum PluginStatus {
@@ -140,6 +141,9 @@ namespace WebCore {
         void didReceiveData(const char*, int);
         void didFinishLoading();
         void didFail(const ResourceError&);
+
+        static bool isCallingPlugin();
+
     private:
         void setParameters(const Vector<String>& paramNames, const Vector<String>& paramValues);
         void init();
@@ -148,6 +152,7 @@ namespace WebCore {
         static void setCurrentPluginView(PluginViewWin*);
         NPError load(const FrameLoadRequest&, bool sendNotification, void* notifyData);
         NPError handlePost(const char* url, const char* target, uint32 len, const char* buf, bool file, void* notifyData, bool sendNotification, bool allowHeaders);
+        void setCallingPlugin(bool) const;
         RefPtr<PluginPackageWin> m_plugin;
         Element* m_element;
         Frame* m_parentFrame;
index 86f4d41..0b9cbe1 100644 (file)
@@ -1,3 +1,13 @@
+2007-12-13  Adam Roben  <aroben@apple.com>
+
+        Fix <rdar://5517707> Crash on wptv.wp.pl when "make bigger" button is clicked
+
+        Reviewed by Anders.
+
+        * WebView.cpp:
+        (WebViewWndProc): Repost paint messages and ignore all other messages
+        when we're calling a plugin.
+
 2007-12-13  Steve Falkenburg  <sfalken@apple.com>
 
         Fix project dependencies based on JavaScriptCore change.
index 6331a9b..282698b 100644 (file)
@@ -86,6 +86,7 @@
 #include <WebCore/PlatformWheelEvent.h>
 #include <WebCore/PluginDatabaseWin.h>
 #include <WebCore/PlugInInfoStore.h>
+#include <WebCore/PluginViewWin.h>
 #include <WebCore/ProgressTracker.h>
 #include <WebCore/ResourceHandle.h>
 #include <WebCore/ResourceHandleClient.h>
@@ -1561,6 +1562,17 @@ static LRESULT CALLBACK WebViewWndProc(HWND hWnd, UINT message, WPARAM wParam, L
 
     ASSERT(webView);
 
+    // Windows Media Player has a modal message loop that will deliver messages
+    // to us at inappropriate times and we will crash if we handle them when
+    // they are delivered. We repost paint messages so that we eventually get
+    // a chance to paint once the modal loop has exited, but other messages
+    // aren't safe to repost, so we just drop them.
+    if (PluginViewWin::isCallingPlugin()) {
+        if (message == WM_PAINT)
+            PostMessage(hWnd, message, wParam, lParam);
+        return 0;
+    }
+
     bool handled = true;
 
     switch (message) {