summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-08-25 18:47:07 +0000
committerananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-08-25 18:47:07 +0000
commitad4703ed344d3987bfc4fa351ee2c44836d60e00 (patch)
treec24b1c1849f94007ed5a60638cbc75d88d14d8aa
parenta4a16bbc5a15fecf65d04128b55e51298286c15a (diff)
downloadchromium_src-ad4703ed344d3987bfc4fa351ee2c44836d60e00.zip
chromium_src-ad4703ed344d3987bfc4fa351ee2c44836d60e00.tar.gz
chromium_src-ad4703ed344d3987bfc4fa351ee2c44836d60e00.tar.bz2
This CL ensures that plugins always peek in the context of outgoing sync calls.
I will be watching the reliability test runs closely for any crashes which creep in due to reentrancies into plugins caused by this CL. This fixes bug http://code.google.com/p/chromium/issues/detail?id=15985 It is a touch tricky to implement a test case for this. Will add one hopefully in a subsequent CL Bug=15985 Test=Covered by UI test Review URL: http://codereview.chromium.org/173211 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24266 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/plugin/plugin_channel.cc19
-rw-r--r--chrome/test/data/npapi/ensure_plugins_pump_messages_in_sync_calls.html39
-rw-r--r--chrome/test/ui/npapi_uitest.cc10
-rw-r--r--webkit/glue/plugins/test/plugin_client.cc3
-rw-r--r--webkit/glue/plugins/test/plugin_windowed_test.cc9
-rw-r--r--webkit/tools/npapi_layout_test_plugin/PluginObject.cpp15
6 files changed, 90 insertions, 5 deletions
diff --git a/chrome/plugin/plugin_channel.cc b/chrome/plugin/plugin_channel.cc
index 04d9afc..6aa10d4 100644
--- a/chrome/plugin/plugin_channel.cc
+++ b/chrome/plugin/plugin_channel.cc
@@ -73,6 +73,25 @@ bool PluginChannel::Send(IPC::Message* msg) {
LOG(INFO) << "sending message @" << msg << " on channel @" << this
<< " with type " << msg->type();
}
+
+ if (msg->is_sync()) {
+ IPC::SyncMessage* sync_msg = static_cast<IPC::SyncMessage*>(msg);
+ // Clear the existing pump messages event if any in the message. This
+ // is because we won't be relying on this event being set to decide whether
+ // to pump messages or not.
+ sync_msg->set_pump_messages_event(NULL);
+
+ // Signal that the channel should continue to pump messages while it waits
+ // for the sync call to complete. This is to ensure that the following
+ // scenario does not result in a deadlock.
+ // 1. Plugin1 issues a sync request to the renderer
+ // 2. The renderer then issues a sync request to plugin2.
+ // 3. Plugin2 then dispatches a native message to plugin1
+ // If we don't pump messages from Plugin1, then it would cause a deadlock
+ // as the three processes above are waiting for each other.
+ sync_msg->EnableMessagePumping();
+ }
+
bool result = PluginChannelBase::Send(msg);
in_send_--;
return result;
diff --git a/chrome/test/data/npapi/ensure_plugins_pump_messages_in_sync_calls.html b/chrome/test/data/npapi/ensure_plugins_pump_messages_in_sync_calls.html
new file mode 100644
index 0000000..f840278
--- /dev/null
+++ b/chrome/test/data/npapi/ensure_plugins_pump_messages_in_sync_calls.html
@@ -0,0 +1,39 @@
+<html>
+<head>
+<title>Test for verifying if plugins pump messages in sync calls.</title>
+<script src="npapi.js"></script>
+<script>
+function SetFocusToPlugin2() {
+ var plugin = window.document["plugin2"];
+ plugin.resetFocus = 1;
+}
+</script>
+</head>
+
+<body>
+<div id="statusPanel" style="border: 1px solid red; width: 100%">
+Test running....
+</div>
+
+<h2>Test to verify whether plugins pump messages in sync calls.</h2>
+
+<p>
+This test verifies whether a plugin continues to pump messages in outgoing
+sync calls. This ensures that it remains responsive and does not cause
+other plugins and the browser to hang
+</p>
+
+<DIV ID=PluginDiv>
+<embed type="application/vnd.npapi-test"
+ src="foo"
+ name="src_plugin_for_outgoing_sync_call"
+ id="1"
+ mode="np_embed">
+</embed>
+</DIV>
+
+<embed name="plugin2" type="application/x-webkit-test-netscape" resetFocusWindow="1"></embed>
+
+</body>
+</html>
+
diff --git a/chrome/test/ui/npapi_uitest.cc b/chrome/test/ui/npapi_uitest.cc
index eb19042..a4fa299 100644
--- a/chrome/test/ui/npapi_uitest.cc
+++ b/chrome/test/ui/npapi_uitest.cc
@@ -298,3 +298,13 @@ TEST_F(NPAPIVisiblePluginTester, MultipleInstancesSyncCalls) {
kTestCompleteSuccess, kShortWaitTimeout);
}
+TEST_F(NPAPIVisiblePluginTester, EnsurePluginsPumpInSyncCalls) {
+ if (UITest::in_process_renderer())
+ return;
+
+ GURL url = GetTestUrl(L"npapi",
+ L"ensure_plugins_pump_messages_in_sync_calls.html");
+ NavigateToURL(url);
+ WaitForFinish("src_plugin_for_outgoing_sync_call", "1", url,
+ kTestCompleteCookie, kTestCompleteSuccess, kShortWaitTimeout);
+}
diff --git a/webkit/glue/plugins/test/plugin_client.cc b/webkit/glue/plugins/test/plugin_client.cc
index bcad2a8..dfa151d 100644
--- a/webkit/glue/plugins/test/plugin_client.cc
+++ b/webkit/glue/plugins/test/plugin_client.cc
@@ -155,7 +155,8 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
// TODO(port): plugin_windowed_test.*.
} else if (test_name == "hidden_plugin" ||
test_name == "create_instance_in_paint" ||
- test_name == "alert_in_window_message") {
+ test_name == "alert_in_window_message" ||
+ test_name == "src_plugin_for_outgoing_sync_call") {
new_test = new NPAPIClient::WindowedPluginTest(instance,
NPAPIClient::PluginClient::HostFunctions());
#endif
diff --git a/webkit/glue/plugins/test/plugin_windowed_test.cc b/webkit/glue/plugins/test/plugin_windowed_test.cc
index 3a15ae0..ace8a4a 100644
--- a/webkit/glue/plugins/test/plugin_windowed_test.cc
+++ b/webkit/glue/plugins/test/plugin_windowed_test.cc
@@ -33,7 +33,8 @@ NPError WindowedPluginTest::SetWindow(NPWindow* pNPWindow) {
}
if ((test_name() == "create_instance_in_paint" && test_id() == "1") ||
- test_name() == "alert_in_window_message") {
+ test_name() == "alert_in_window_message" ||
+ test_name() == "src_plugin_for_outgoing_sync_call") {
static ATOM window_class = 0;
if (!window_class) {
WNDCLASSEX wcex;
@@ -96,6 +97,12 @@ LRESULT CALLBACK WindowedPluginTest::WindowProc(
// and verify that we don't hang the browser.
CallJSFunction(this_ptr, "CallAlert");
CallJSFunction(this_ptr, "CallAlert");
+ } else if (this_ptr->test_name() == "src_plugin_for_outgoing_sync_call" &&
+ message == WM_PAINT) {
+ this_ptr->done_ = true;
+ SetFocus(window);
+ CallJSFunction(this_ptr, "SetFocusToPlugin2");
+ this_ptr->SignalTestCompleted();
}
}
diff --git a/webkit/tools/npapi_layout_test_plugin/PluginObject.cpp b/webkit/tools/npapi_layout_test_plugin/PluginObject.cpp
index 2bfab8f..26abd8d 100644
--- a/webkit/tools/npapi_layout_test_plugin/PluginObject.cpp
+++ b/webkit/tools/npapi_layout_test_plugin/PluginObject.cpp
@@ -72,7 +72,8 @@ static bool identifiersInitialized = false;
#define ID_PROPERTY_LOG_DESTROY 4
#define ID_PROPERTY_RETURN_ERROR_FROM_NEWSTREAM 5
#define ID_PROPERTY_TEST_OBJECT_COUNT 6
-#define NUM_PROPERTY_IDENTIFIERS 7
+#define ID_PROPERTY_RESET_FOCUS 7
+#define NUM_PROPERTY_IDENTIFIERS 8
static NPIdentifier pluginPropertyIdentifiers[NUM_PROPERTY_IDENTIFIERS];
static const NPUTF8 *pluginPropertyIdentifierNames[NUM_PROPERTY_IDENTIFIERS] = {
@@ -83,6 +84,7 @@ static const NPUTF8 *pluginPropertyIdentifierNames[NUM_PROPERTY_IDENTIFIERS] = {
"logDestroy",
"returnErrorFromNewStream",
"testObjectCount",
+ "resetFocus"
};
#define ID_TEST_CALLBACK_METHOD 0
@@ -211,6 +213,13 @@ static bool pluginSetProperty(NPObject* obj, NPIdentifier name, const NPVariant*
} else if (name == pluginPropertyIdentifiers[ID_PROPERTY_RETURN_ERROR_FROM_NEWSTREAM]) {
plugin->returnErrorFromNewStream = NPVARIANT_TO_BOOLEAN(*variant);
return true;
+ } else if (name == pluginPropertyIdentifiers[ID_PROPERTY_RESET_FOCUS]) {
+#ifdef OS_WIN
+ if (NPVARIANT_TO_BOOLEAN(*variant)) {
+ SetFocus(NULL);
+ }
+ return true;
+#endif // OS_WIN
}
return false;
@@ -748,9 +757,9 @@ static bool pluginInvoke(NPObject* header, NPIdentifier name, const NPVariant* a
return testCallbackAndGetValue(plugin, args, argCount, result);
} else if (name == pluginMethodIdentifiers[ID_TEST_CONSTRUCT]) {
return testConstruct(plugin, args, argCount, result);
- } else if (name == pluginMethodIdentifiers[ID_DESTROY_NULL_STREAM])
+ } else if (name == pluginMethodIdentifiers[ID_DESTROY_NULL_STREAM])
return destroyNullStream(plugin, args, argCount, result);
-
+
return false;
}
generated by cgit v1.1 at 2025-02-10 21:52:13 +0000