REGRESSION (WebKit2): Crash in Flash on USA Today photo gallery
authorjhoneycutt@apple.com <jhoneycutt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 May 2011 21:56:21 +0000 (21:56 +0000)
committerjhoneycutt@apple.com <jhoneycutt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 May 2011 21:56:21 +0000 (21:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=61428
<rdar://problem/9457006>

Reviewed by Adam Roben.

Source/WebKit2:

The crash occurs when Flash posts a message to a window that it
creates, and in processing the message, it calls NPN_Evaluate to
evaluate JavaScript that removes the plug-in from the page. Flash then
crashes when we return to Flash code.

* Platform/WorkItem.h:
(DerefWorkItem::DerefWorkItem):
Initialize m_ptr.
(DerefWorkItem::execute):
Deref the object.
(WorkItem::createDeref):
Create and return a DerefWorkItem.

* WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::unprotectPluginFromDestruction):
If the PluginView has only one reference left, deref it asynchronously.

Tools:

The crash occurs when Flash posts a message to a window that it
creates, and in processing the message, it calls NPN_Evaluate to
evaluate JavaScript that removes the plug-in from the page. Flash then
crashes when we return to Flash code.

This test emulates that behavior.

* DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp: Added.
(CallJSThatDestroysPlugin::CallJSThatDestroysPlugin):
Initialize member vars.
(CallJSThatDestroysPlugin::~CallJSThatDestroysPlugin):
Remove our custom property from the message window, and destroy it.
(CallJSThatDestroysPlugin::NPP_Destroy):
Set m_isDestroyed, log that the plug-in was destroyed, and notify the
layout test controller that we're done.
(wndProc):
Get the PluginTest object, and call its runTest() function.
(CallJSThatDestroysPlugin::NPP_New):
Setup the test: register a class for the message-only window, create
it, and post a message to it to run the test.
(CallJSThatDestroysPlugin::runTest):
Execute JS that removes the plug-in from the page, and if we're not
destroyed, log a success message.

* DumpRenderTree/TestNetscapePlugIn/win/TestNetscapePlugin.vcproj:
Add new test to project.

LayoutTests:

* platform/win/plugins/call-javascript-that-destroys-plugin-expected.txt: Added.
* platform/win/plugins/call-javascript-that-destroys-plugin.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/win/plugins/call-javascript-that-destroys-plugin-expected.txt [new file with mode: 0644]
LayoutTests/platform/win/plugins/call-javascript-that-destroys-plugin.html [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/Platform/WorkItem.h
Source/WebKit2/WebProcess/Plugins/PluginView.cpp
Tools/ChangeLog
Tools/DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp [new file with mode: 0644]
Tools/DumpRenderTree/TestNetscapePlugIn/win/TestNetscapePlugin.vcproj

index f6fdc7e..4da35bd 100644 (file)
@@ -1,3 +1,14 @@
+2011-05-25  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        REGRESSION (WebKit2): Crash in Flash on USA Today photo gallery
+        https://bugs.webkit.org/show_bug.cgi?id=61428
+        <rdar://problem/9457006>
+
+        Reviewed by Adam Roben.
+
+        * platform/win/plugins/call-javascript-that-destroys-plugin-expected.txt: Added.
+        * platform/win/plugins/call-javascript-that-destroys-plugin.html: Added.
+
 2011-05-25  Andrew Scherkus  <scherkus@chromium.org>
 
         Reviewed by Eric Carlson.
diff --git a/LayoutTests/platform/win/plugins/call-javascript-that-destroys-plugin-expected.txt b/LayoutTests/platform/win/plugins/call-javascript-that-destroys-plugin-expected.txt
new file mode 100644 (file)
index 0000000..c616ee9
--- /dev/null
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: line 0: PLUGIN: Success: executed script, and plug-in is not yet destroyed.
+CONSOLE MESSAGE: line 0: PLUGIN: Plug-in destroyed.
+This tests that, if a plug-in directs the browser to evaluate JavaScript that removes the plug-in from the page, the plug-in is destroyed asynchronously.
diff --git a/LayoutTests/platform/win/plugins/call-javascript-that-destroys-plugin.html b/LayoutTests/platform/win/plugins/call-javascript-that-destroys-plugin.html
new file mode 100644 (file)
index 0000000..9f95eb5
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script>
+        if (window.layoutTestController)
+            layoutTestController.dumpAsText();
+    </script>
+</head>
+<body>
+    <embed type="application/x-webkit-test-netscape" test="call-javascript-that-destroys-plugin"></embed>
+    <p>This tests that, if a plug-in directs the browser to evaluate JavaScript that removes the plug-in from the page,
+    the plug-in is destroyed asynchronously.</p>
+</body>
+</html>
+
index 763e705..30ec516 100644 (file)
@@ -1,3 +1,28 @@
+2011-05-25  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        REGRESSION (WebKit2): Crash in Flash on USA Today photo gallery
+        https://bugs.webkit.org/show_bug.cgi?id=61428
+        <rdar://problem/9457006>
+
+        Reviewed by Adam Roben.
+
+        The crash occurs when Flash posts a message to a window that it
+        creates, and in processing the message, it calls NPN_Evaluate to
+        evaluate JavaScript that removes the plug-in from the page. Flash then
+        crashes when we return to Flash code.
+
+        * Platform/WorkItem.h:
+        (DerefWorkItem::DerefWorkItem):
+        Initialize m_ptr.
+        (DerefWorkItem::execute):
+        Deref the object.
+        (WorkItem::createDeref):
+        Create and return a DerefWorkItem.
+
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::unprotectPluginFromDestruction):
+        If the PluginView has only one reference left, deref it asynchronously.
+
 2011-05-25  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Anders Carlsson.
index 64bff58..f2868d0 100644 (file)
@@ -41,6 +41,9 @@ public:
 
     static PassOwnPtr<WorkItem> create(void (*)());
 
+    template<typename C>
+    static PassOwnPtr<WorkItem> createDeref(C*);
+
     virtual ~WorkItem() { }
     virtual void execute() = 0;
 
@@ -184,4 +187,28 @@ inline PassOwnPtr<WorkItem> WorkItem::create(void (*function)())
     return adoptPtr(static_cast<WorkItem*>(new FunctionWorkItem0(function)));
 }
 
+template<typename C>
+class DerefWorkItem : private WorkItem {
+    // We only allow WorkItem to create this.
+    friend class WorkItem;
+
+    explicit DerefWorkItem(C* ptr)
+        : m_ptr(ptr)
+    {
+    }
+
+    virtual void execute()
+    {
+        m_ptr->deref();
+    }
+
+    C* m_ptr;
+};
+
+template<typename C>
+PassOwnPtr<WorkItem> WorkItem::createDeref(C* ptr)
+{
+    return adoptPtr(static_cast<WorkItem*>(new DerefWorkItem<C>(ptr)));
+}
+
 #endif // WorkItem_h
index bb68f0c..59c989c 100644 (file)
@@ -1129,7 +1129,17 @@ void PluginView::protectPluginFromDestruction()
 
 void PluginView::unprotectPluginFromDestruction()
 {
-    if (!m_isBeingDestroyed)
+    if (m_isBeingDestroyed)
+        return;
+
+    // A plug-in may ask us to evaluate JavaScript that removes the plug-in from the
+    // page, but expect the object to still be alive when the call completes. Flash,
+    // for example, may crash if the plug-in is destroyed and we return to code for
+    // the destroyed object higher on the stack. To prevent this, if the plug-in has
+    // only one remaining reference, call deref() asynchronously.
+    if (hasOneRef())
+        RunLoop::main()->scheduleWork(WorkItem::createDeref(this));
+    else
         deref();
 }
 
index 61c4749..31a052b 100644 (file)
@@ -1,3 +1,38 @@
+2011-05-25  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        REGRESSION (WebKit2): Crash in Flash on USA Today photo gallery
+        https://bugs.webkit.org/show_bug.cgi?id=61428
+        <rdar://problem/9457006>
+
+        Reviewed by Adam Roben.
+
+        The crash occurs when Flash posts a message to a window that it
+        creates, and in processing the message, it calls NPN_Evaluate to
+        evaluate JavaScript that removes the plug-in from the page. Flash then
+        crashes when we return to Flash code.
+
+        This test emulates that behavior.
+
+        * DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp: Added.
+        (CallJSThatDestroysPlugin::CallJSThatDestroysPlugin):
+        Initialize member vars.
+        (CallJSThatDestroysPlugin::~CallJSThatDestroysPlugin):
+        Remove our custom property from the message window, and destroy it.
+        (CallJSThatDestroysPlugin::NPP_Destroy):
+        Set m_isDestroyed, log that the plug-in was destroyed, and notify the
+        layout test controller that we're done.
+        (wndProc):
+        Get the PluginTest object, and call its runTest() function.
+        (CallJSThatDestroysPlugin::NPP_New):
+        Setup the test: register a class for the message-only window, create
+        it, and post a message to it to run the test.
+        (CallJSThatDestroysPlugin::runTest):
+        Execute JS that removes the plug-in from the page, and if we're not
+        destroyed, log a success message.
+
+        * DumpRenderTree/TestNetscapePlugIn/win/TestNetscapePlugin.vcproj:
+        Add new test to project.
+
 2011-05-25  Tony Chang  <tony@chromium.org>
 
         Reviewed by Adam Barth.
diff --git a/Tools/DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp b/Tools/DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp
new file mode 100644 (file)
index 0000000..39c6365
--- /dev/null
@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2011 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "PluginTest.h"
+
+#include <string.h>
+#include <vector>
+
+using namespace std;
+
+// Test that evaluating JavaScript that removes the plug-in from the page
+// destroys the plug-in asynchronously.
+class CallJSThatDestroysPlugin : public PluginTest {
+public:
+    CallJSThatDestroysPlugin(NPP npp, const string& identifier)
+        : PluginTest(npp, identifier)
+        , m_isDestroyed(false)
+        , m_window(0)
+    {
+    }
+    ~CallJSThatDestroysPlugin();
+
+    void runTest();
+
+private:
+    virtual NPError NPP_New(NPMIMEType, uint16_t, int16_t, char*[], char*[], NPSavedData*);
+    virtual NPError NPP_Destroy(NPSavedData**);
+
+    bool m_isDestroyed;
+    HWND m_window;
+};
+
+static PluginTest::Register<CallJSThatDestroysPlugin> registrar("call-javascript-that-destroys-plugin");
+
+static const LPCWSTR pluginTestProperty = L"PluginTestProperty";
+static const UINT runTestWindowMessage = WM_USER + 1;
+static const LPCWSTR messageWindowClassName = L"CallJSThatDestroysPluginMessageWindow";
+
+CallJSThatDestroysPlugin::~CallJSThatDestroysPlugin()
+{
+    ::RemovePropW(m_window, pluginTestProperty);
+    ::DestroyWindow(m_window);
+}
+
+void CallJSThatDestroysPlugin::runTest()
+{
+    executeScript("var plugin = document.getElementsByTagName('embed')[0]; plugin.parentElement.removeChild(plugin);");
+    if (!m_isDestroyed)
+        log("Success: executed script, and plug-in is not yet destroyed.");
+}
+
+static LRESULT CALLBACK wndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
+{
+    if (msg != runTestWindowMessage)
+        return ::DefWindowProcW(hwnd, msg, wParam, lParam);
+
+    CallJSThatDestroysPlugin* pluginTest = reinterpret_cast<CallJSThatDestroysPlugin*>(::GetPropW(hwnd, pluginTestProperty));
+    pluginTest->runTest();
+    return 0;
+}
+
+NPError CallJSThatDestroysPlugin::NPP_New(NPMIMEType, uint16_t, int16_t, char*[], char*[], NPSavedData*)
+{
+    assert(!m_window);
+
+    waitUntilDone();
+
+    WNDCLASSEXW wndClass = {0};
+    wndClass.cbSize = sizeof(wndClass);
+    wndClass.lpfnWndProc = wndProc;
+    wndClass.hCursor = LoadCursor(0, IDC_ARROW);
+    wndClass.hInstance = GetModuleHandle(0);
+    wndClass.lpszClassName = messageWindowClassName;
+
+    ::RegisterClassExW(&wndClass);
+    m_window = ::CreateWindowExW(0, messageWindowClassName, 0, WS_CHILD, 0, 0, 0, 0, HWND_MESSAGE, 0, GetModuleHandle(0), 0);
+
+    ::SetPropW(m_window, pluginTestProperty, reinterpret_cast<HANDLE>(this));
+    ::PostMessageW(m_window, runTestWindowMessage, 0, 0);
+
+    return NPERR_NO_ERROR;
+}
+
+NPError CallJSThatDestroysPlugin::NPP_Destroy(NPSavedData**)
+{
+    m_isDestroyed = true;
+    log("Plug-in destroyed.");
+    notifyDone();
+    return NPERR_NO_ERROR;
+}
index 631e3c6..4e0bd8c 100644 (file)
                        Name="Tests"
                        >
                        <File
+                               RelativePath=".\CallJSThatDestroysPlugin.cpp"
+                               >
+                       </File>
+                       <File
                                RelativePath="..\Tests\DocumentOpenInDestroyStream.cpp"
                                >
                        </File>