Removing a plug-in element from a page opened in a background tab in Safari crashes
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Aug 2012 19:33:37 +0000 (19:33 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Aug 2012 19:33:37 +0000 (19:33 +0000)
<rdar://problem/12057991> and https://bugs.webkit.org/show_bug.cgi?id=93913

Reviewed by Beth Dakin.

.:

* Source/autotools/symbols.filter: Allow this symbol through for DRT's sake.

Source/WebCore:

Expose Page::setCanStartMedia to regression tests so they can pretend to be in a non-windowed WebView.

Test: platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html

* testing/InternalSettings.cpp:
(WebCore::InternalSettings::Backup::Backup):
(WebCore::InternalSettings::Backup::restoreTo):
(WebCore::InternalSettings::setCanStartMedia):
(WebCore):
* testing/InternalSettings.h:
(Backup):
(InternalSettings):
* testing/InternalSettings.idl:

Source/WebKit2:

This only happens in WebKit2 with asynchronous plug-in initialization enabled.

* WebProcess/Plugins/PluginProxy.cpp:
(WebKit::PluginProxy::destroy): Null-check m_connection, as it might not have been created yet.

* win/WebKit2.def: Export Page::setCanStartMedia for InternalSettings/DRT use.

LayoutTests:

Using internal setting setCanStartMedia, pretend the page is in an unwindowed WebView then remove
the plug-in element.

* platform/mac-wk2/plugins/asynchronous-destroy-before-initialization-expected.txt: Added.
* platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html: Added.

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

12 files changed:
ChangeLog
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/plugins/asynchronous-destroy-before-initialization-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/testing/InternalSettings.cpp
Source/WebCore/testing/InternalSettings.h
Source/WebCore/testing/InternalSettings.idl
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp
Source/WebKit2/win/WebKit2.def
Source/autotools/symbols.filter

index c19362d..c3ff177 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-08-15  Brady Eidson  <beidson@apple.com>
+
+        Removing a plug-in element from a page opened in a background tab in Safari crashes
+        <rdar://problem/12057991> and https://bugs.webkit.org/show_bug.cgi?id=93913
+
+        Reviewed by Beth Dakin.
+
+        * Source/autotools/symbols.filter: Allow this symbol through for DRT's sake.
+
 2012-08-14  Keishi Hattori  <keishi@webkit.org>
 
         Share common code between calendar picker and color suggestion picker
index 6a5fe34..0902cd8 100644 (file)
@@ -1,3 +1,16 @@
+2012-08-15  Brady Eidson  <beidson@apple.com>
+
+        Removing a plug-in element from a page opened in a background tab in Safari crashes
+        <rdar://problem/12057991> and https://bugs.webkit.org/show_bug.cgi?id=93913
+
+        Reviewed by Beth Dakin.
+
+        Using internal setting setCanStartMedia, pretend the page is in an unwindowed WebView then remove
+        the plug-in element.
+
+        * platform/mac-wk2/plugins/asynchronous-destroy-before-initialization-expected.txt: Added.
+        * platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html: Added.
+
 2012-08-15  Levi Weintraub  <leviw@chromium.org>
 
         AutoTableLayout truncates preferred widths for cells when it needs to ceil them to contain the contents
diff --git a/LayoutTests/platform/mac-wk2/plugins/asynchronous-destroy-before-initialization-expected.txt b/LayoutTests/platform/mac-wk2/plugins/asynchronous-destroy-before-initialization-expected.txt
new file mode 100644 (file)
index 0000000..9db0b7a
--- /dev/null
@@ -0,0 +1,7 @@
+Tests that when media/plugin playback is disabled (such as when in a background tab), removing a plug-in element that was never initialized does not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Removing the plug-in element did not crash.
+
diff --git a/LayoutTests/platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html b/LayoutTests/platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html
new file mode 100644 (file)
index 0000000..5bf4f95
--- /dev/null
@@ -0,0 +1,45 @@
+<head>
+<script src="../../../fast/js/resources/js-test-pre.js"></script>
+<script>
+
+var startTime = new Date;
+
+if (window.testRunner) {
+    testRunner.overridePreference("WebKit2AsynchronousPluginInitializationEnabled", "1");
+    testRunner.overridePreference("WebKit2AsynchronousPluginInitializationEnabledForAllPlugins", "1");
+    window.internals.settings.setCanStartMedia(false);
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function runTest() 
+{
+    if (!window.testRunner) {
+        debug("This test can only run from within DumpRenderTree because it requires test runner internals.\n");
+        return;
+    }
+       
+       var pluginElement = document.getElementById("TestElement");
+       pluginElement.parentNode.removeChild(pluginElement);
+
+    testPassed("Removing the plug-in element did not crash.");
+
+       var endTime = new Date;
+       if (endTime - startTime > 549)
+               testFailed("This test took over 549ms meaning the plug-in with a 550ms startup delay was actually initialized.  It never should've been initialized.");
+
+    testRunner.notifyDone();
+}
+</script>
+
+</head>
+<body onload="setTimeout('runTest()', 0)">
+<embed id="TestElement" type="application/x-webkit-test-netscape" test="slow-npp-new"></embed>
+<p id="description"></p>
+<div id="console"></div>
+</body>
+
+<script>
+description("Tests that when media/plugin playback is disabled (such as when in a background tab), removing a plug-in element that was never initialized does not crash.");
+var unused = document.body.offsetTop;
+</script>
index 12b4849..c33919b 100644 (file)
@@ -1,3 +1,24 @@
+2012-08-15  Brady Eidson  <beidson@apple.com>
+
+        Removing a plug-in element from a page opened in a background tab in Safari crashes
+        <rdar://problem/12057991> and https://bugs.webkit.org/show_bug.cgi?id=93913
+
+        Reviewed by Beth Dakin.
+
+        Expose Page::setCanStartMedia to regression tests so they can pretend to be in a non-windowed WebView.
+
+        Test: platform/mac-wk2/plugins/asynchronous-destroy-before-initialization.html
+
+        * testing/InternalSettings.cpp:
+        (WebCore::InternalSettings::Backup::Backup):
+        (WebCore::InternalSettings::Backup::restoreTo):
+        (WebCore::InternalSettings::setCanStartMedia):
+        (WebCore):
+        * testing/InternalSettings.h:
+        (Backup):
+        (InternalSettings):
+        * testing/InternalSettings.idl:
+
 2012-08-15  Levi Weintraub  <leviw@chromium.org>
 
         AutoTableLayout truncates preferred widths for cells when it needs to ceil them to contain the contents
index 3592d05..0630278 100755 (executable)
@@ -96,6 +96,7 @@ InternalSettings::Backup::Backup(Page* page, Settings* settings)
 #if ENABLE(DIALOG_ELEMENT)
     , m_originalDialogElementEnabled(RuntimeEnabledFeatures::dialogElementEnabled())
 #endif
+    , m_canStartMedia(page->canStartMedia())
 {
 }
 
@@ -127,6 +128,7 @@ void InternalSettings::Backup::restoreTo(Page* page, Settings* settings)
 #if ENABLE(DIALOG_ELEMENT)
     RuntimeEnabledFeatures::setDialogElementEnabled(m_originalDialogElementEnabled);
 #endif
+    page->setCanStartMedia(m_canStartMedia);
 }
 
 InternalSettings* InternalSettings::from(Page* page)
@@ -433,6 +435,12 @@ bool InternalSettings::cssVariablesEnabled(ExceptionCode& ec)
     return settings()->cssVariablesEnabled();
 }
 
+void InternalSettings::setCanStartMedia(bool enabled, ExceptionCode& ec)
+{
+    InternalSettingsGuardForSettings();
+    m_page->setCanStartMedia(enabled);
+}
+
 void InternalSettings::setMediaPlaybackRequiresUserGesture(bool enabled, ExceptionCode& ec)
 {
     InternalSettingsGuardForSettings();
index 7b0f72a..cfae94f 100755 (executable)
@@ -78,6 +78,7 @@ public:
 #if ENABLE(DIALOG_ELEMENT)
         bool m_originalDialogElementEnabled;
 #endif
+        bool m_canStartMedia;
     };
 
     typedef RefCountedSupplement<Page, InternalSettings> SuperType;
@@ -122,6 +123,7 @@ public:
     void setCSSExclusionsEnabled(bool enabled, ExceptionCode&);
     void setCSSVariablesEnabled(bool enabled, ExceptionCode&);
     bool cssVariablesEnabled(ExceptionCode&);
+    void setCanStartMedia(bool, ExceptionCode&);
     void setMediaPlaybackRequiresUserGesture(bool, ExceptionCode&);
     void setEditingBehavior(const String&, ExceptionCode&);
     void setFixedPositionCreatesStackingContext(bool, ExceptionCode&);
index 6acda98..b9c8f3e 100755 (executable)
@@ -60,6 +60,7 @@ module window {
         void setCSSExclusionsEnabled(in boolean enabled) raises(DOMException);
         void setCSSVariablesEnabled(in boolean enabled) raises(DOMException);
         boolean cssVariablesEnabled() raises(DOMException);
+        void setCanStartMedia(in boolean enabled) raises(DOMException);
         void setMediaPlaybackRequiresUserGesture(in boolean enabled) raises(DOMException);
         void setEditingBehavior(in DOMString behavior) raises(DOMException);
         void setFixedPositionCreatesStackingContext(in boolean creates) raises(DOMException);
index 0c003ea..cca4858 100644 (file)
@@ -1,3 +1,17 @@
+2012-08-15  Brady Eidson  <beidson@apple.com>
+
+        Removing a plug-in element from a page opened in a background tab in Safari crashes
+        <rdar://problem/12057991> and https://bugs.webkit.org/show_bug.cgi?id=93913
+
+        Reviewed by Beth Dakin.
+
+        This only happens in WebKit2 with asynchronous plug-in initialization enabled.
+
+        * WebProcess/Plugins/PluginProxy.cpp:
+        (WebKit::PluginProxy::destroy): Null-check m_connection, as it might not have been created yet.
+
+        * win/WebKit2.def: Export Page::setCanStartMedia for InternalSettings/DRT use.
+
 2012-08-14  Mark Hahnenberg  <mhahnenberg@apple.com>
 
         Change behavior of MasqueradesAsUndefined to better accommodate DFG changes
index 17db257..e380eef 100644 (file)
@@ -199,10 +199,12 @@ void PluginProxy::didFailToCreatePluginInternal()
 
 void PluginProxy::destroy()
 {
-    m_connection->connection()->sendSync(Messages::WebProcessConnection::DestroyPlugin(m_pluginInstanceID, m_waitingOnAsynchronousInitialization), Messages::WebProcessConnection::DestroyPlugin::Reply(), 0);
-
     m_isStarted = false;
 
+    if (!m_connection)
+        return;
+
+    m_connection->connection()->sendSync(Messages::WebProcessConnection::DestroyPlugin(m_pluginInstanceID, m_waitingOnAsynchronousInitialization), Messages::WebProcessConnection::DestroyPlugin::Reply(), 0);
     m_connection->removePluginProxy(this);
 }
 
index dea36ed..6e8e417 100644 (file)
@@ -210,6 +210,7 @@ EXPORTS
         ?scriptExecutionContext@JSDOMGlobalObject@WebCore@@QBEPAVScriptExecutionContext@2@XZ
         ?scriptNameToCode@WebCore@@YA?AW4UScriptCode@@ABVString@WTF@@@Z
         ?scrollElementToRect@FrameView@WebCore@@QAEXPAVElement@2@ABVIntRect@2@@Z
+        ?setCanStartMedia@Page@WebCore@@QAEX_N@Z
         ?setCursiveFontFamily@Settings@WebCore@@QAEXABVAtomicString@WTF@@W4UScriptCode@@@Z
         ?setDeviceScaleFactor@Page@WebCore@@QAEXM@Z
         ?setDocumentState@HistoryItem@WebCore@@QAEXABV?$Vector@VString@WTF@@$0A@@WTF@@@Z
index 82ffc2e..e2ea435 100644 (file)
@@ -30,6 +30,7 @@ _ZN24DumpRenderTreeSupportGtk*;
 _ZN7WebCore4Page13setPaginationERKNS0_10PaginationE;
 _ZN7WebCore4Page18setPageScaleFactorEfRKNS_8IntPointE;
 _ZN7WebCore4Page20setDeviceScaleFactorEf;
+_ZN7WebCore4Page16setCanStartMediaEb;
 _ZN7WebCore4toJSEPN3JSC9ExecStateEPNS_17JSDOMGlobalObjectEPNS_10ClientRectE;
 _ZN7WebCore4toJSEPN3JSC9ExecStateEPNS_17JSDOMGlobalObjectEPNS_10ShadowRootE;
 _ZN7WebCore4toJSEPN3JSC9ExecStateEPNS_17JSDOMGlobalObjectEPNS_14ClientRectListE;
@@ -171,6 +172,7 @@ _ZNK7WebCore14InsertionPoint8isActiveEv;
 _ZN7WebCore26ContextDestructionObserverD2Ev;
 _ZN7WebCore26ContextDestructionObserverC2EPNS_22ScriptExecutionContextE;
 _ZN7WebCore26ContextDestructionObserver16contextDestroyedEv;
+
 local:
 _Z*;
 cti*;