- Revision
- 87324
- Author
- jhoneyc...@apple.com
- Date
- 2011-05-25 14:56:21 -0700 (Wed, 25 May 2011)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (87323 => 87324)
--- trunk/LayoutTests/ChangeLog 2011-05-25 21:47:39 UTC (rev 87323)
+++ trunk/LayoutTests/ChangeLog 2011-05-25 21:56:21 UTC (rev 87324)
@@ -1,3 +1,14 @@
+2011-05-25 Jon Honeycutt <jhoneyc...@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 <scher...@chromium.org>
Reviewed by Eric Carlson.
Added: trunk/LayoutTests/platform/win/plugins/call-_javascript_-that-destroys-plugin-expected.txt (0 => 87324)
--- trunk/LayoutTests/platform/win/plugins/call-_javascript_-that-destroys-plugin-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/win/plugins/call-_javascript_-that-destroys-plugin-expected.txt 2011-05-25 21:56:21 UTC (rev 87324)
@@ -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.
Added: trunk/LayoutTests/platform/win/plugins/call-_javascript_-that-destroys-plugin.html (0 => 87324)
--- trunk/LayoutTests/platform/win/plugins/call-_javascript_-that-destroys-plugin.html (rev 0)
+++ trunk/LayoutTests/platform/win/plugins/call-_javascript_-that-destroys-plugin.html 2011-05-25 21:56:21 UTC (rev 87324)
@@ -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>
+
Modified: trunk/Source/WebKit2/ChangeLog (87323 => 87324)
--- trunk/Source/WebKit2/ChangeLog 2011-05-25 21:47:39 UTC (rev 87323)
+++ trunk/Source/WebKit2/ChangeLog 2011-05-25 21:56:21 UTC (rev 87324)
@@ -1,3 +1,28 @@
+2011-05-25 Jon Honeycutt <jhoneyc...@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 <m...@apple.com>
Reviewed by Anders Carlsson.
Modified: trunk/Source/WebKit2/Platform/WorkItem.h (87323 => 87324)
--- trunk/Source/WebKit2/Platform/WorkItem.h 2011-05-25 21:47:39 UTC (rev 87323)
+++ trunk/Source/WebKit2/Platform/WorkItem.h 2011-05-25 21:56:21 UTC (rev 87324)
@@ -41,6 +41,9 @@
static PassOwnPtr<WorkItem> create(void (*)());
+ template<typename C>
+ static PassOwnPtr<WorkItem> createDeref(C*);
+
virtual ~WorkItem() { }
virtual void execute() = 0;
@@ -184,4 +187,28 @@
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
Modified: trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp (87323 => 87324)
--- trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp 2011-05-25 21:47:39 UTC (rev 87323)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp 2011-05-25 21:56:21 UTC (rev 87324)
@@ -1129,7 +1129,17 @@
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();
}
Modified: trunk/Tools/ChangeLog (87323 => 87324)
--- trunk/Tools/ChangeLog 2011-05-25 21:47:39 UTC (rev 87323)
+++ trunk/Tools/ChangeLog 2011-05-25 21:56:21 UTC (rev 87324)
@@ -1,3 +1,38 @@
+2011-05-25 Jon Honeycutt <jhoneyc...@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 <t...@chromium.org>
Reviewed by Adam Barth.
Added: trunk/Tools/DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp (0 => 87324)
--- trunk/Tools/DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp (rev 0)
+++ trunk/Tools/DumpRenderTree/TestNetscapePlugIn/win/CallJSThatDestroysPlugin.cpp 2011-05-25 21:56:21 UTC (rev 87324)
@@ -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;
+}
Modified: trunk/Tools/DumpRenderTree/TestNetscapePlugIn/win/TestNetscapePlugin.vcproj (87323 => 87324)
--- trunk/Tools/DumpRenderTree/TestNetscapePlugIn/win/TestNetscapePlugin.vcproj 2011-05-25 21:47:39 UTC (rev 87323)
+++ trunk/Tools/DumpRenderTree/TestNetscapePlugIn/win/TestNetscapePlugin.vcproj 2011-05-25 21:56:21 UTC (rev 87324)
@@ -390,6 +390,10 @@
Name="Tests"
>
<File
+ RelativePath=".\CallJSThatDestroysPlugin.cpp"
+ >
+ </File>
+ <File
RelativePath="..\Tests\DocumentOpenInDestroyStream.cpp"
>
</File>
