summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-08-25 22:37:14 +0000
committerananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-08-25 22:37:14 +0000
commit77c9ce91fe094be265b3dc9312970a1323b5375c (patch)
tree6994371ffadc46433c15e52b4143185fad3e61e6
parentc5d39118d6c0fc173526c8ea7a0581a60b0f23ae (diff)
downloadchromium_src-77c9ce91fe094be265b3dc9312970a1323b5375c.zip
chromium_src-77c9ce91fe094be265b3dc9312970a1323b5375c.tar.gz
chromium_src-77c9ce91fe094be265b3dc9312970a1323b5375c.tar.bz2
Revert 57403 - Merge 57372 - Fix for the missing plugin message displayed at times on sites like youtube while playing
playlists, etc. The message is displayed when we fail to create a channel for the plugin process. The problem occurs because of a race condition between the plugin channel information being deleted from the channel map and the actual channel being deleted. This race occurs if the plugin is in the context of a sync call to the renderer which results in the plugin instance being deleted and the new one getting created in the same context. This results in the plugin channel information getting deleted from the map. The actual channel gets deleted only when the plugin sync call unwinds which occurs after the browser sends in a request to create the channel for the new plugin instance. Fix is to create the channel name with an incrementing number in the plugin process, if the mode is server. The channel map is still keyed off the process id and renderer id as before. Fixes bug http://code.google.com/p/chromium/issues/detail?id=43595 Bug=43595 Test=Covered by new ui test SelfDeleteCreatePluginInNPNEvaluate. The other change is a fix in the test plugin to delete a windows timer correctly and to add support for a new plugin test case. Review URL: http://codereview.chromium.org/3142039 TBR=ananta@chromium.org Review URL: http://codereview.chromium.org/3140031 TBR=ananta@chromium.org Review URL: http://codereview.chromium.org/3197021 git-svn-id: svn://svn.chromium.org/chrome/branches/472/src@57406 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/plugin/plugin_channel.cc4
-rw-r--r--chrome/plugin/plugin_channel_base.cc15
-rw-r--r--chrome/plugin/plugin_channel_base.h2
-rw-r--r--chrome/test/data/npapi/npn_plugin_delete_create_in_evaluate.html40
-rw-r--r--chrome/test/ui/npapi_uitest.cc14
-rw-r--r--webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc23
-rw-r--r--webkit/glue/plugins/test/plugin_npobject_lifetime_test.h3
-rw-r--r--webkit/glue/plugins/test/plugin_test_factory.cc6
-rw-r--r--webkit/glue/plugins/test/plugin_windowed_test.cc8
9 files changed, 22 insertions, 93 deletions
diff --git a/chrome/plugin/plugin_channel.cc b/chrome/plugin/plugin_channel.cc
index 941db8c..2ea37be 100644
--- a/chrome/plugin/plugin_channel.cc
+++ b/chrome/plugin/plugin_channel.cc
@@ -136,12 +136,12 @@ class PluginChannel::MessageFilter : public IPC::ChannelProxy::MessageFilter {
PluginChannel* PluginChannel::GetPluginChannel(int renderer_id,
MessageLoop* ipc_message_loop) {
// Map renderer ID to a (single) channel to that process.
- std::string channel_key = StringPrintf(
+ std::string channel_name = StringPrintf(
"%d.r%d", base::GetCurrentProcId(), renderer_id);
PluginChannel* channel =
static_cast<PluginChannel*>(PluginChannelBase::GetChannel(
- channel_key,
+ channel_name,
IPC::Channel::MODE_SERVER,
ClassFactory,
ipc_message_loop,
diff --git a/chrome/plugin/plugin_channel_base.cc b/chrome/plugin/plugin_channel_base.cc
index df7cccc..5d18654 100644
--- a/chrome/plugin/plugin_channel_base.cc
+++ b/chrome/plugin/plugin_channel_base.cc
@@ -9,7 +9,6 @@
#include "base/auto_reset.h"
#include "base/hash_tables.h"
#include "base/lazy_instance.h"
-#include "base/string_number_conversions.h"
#include "chrome/common/child_process.h"
#include "ipc/ipc_sync_message.h"
@@ -25,15 +24,13 @@ static PluginChannelMap g_plugin_channels_;
static base::LazyInstance<std::stack<scoped_refptr<PluginChannelBase> > >
lazy_plugin_channel_stack_(base::LINKER_INITIALIZED);
-static int next_pipe_id = 0;
-
PluginChannelBase* PluginChannelBase::GetChannel(
- const std::string& channel_key, IPC::Channel::Mode mode,
+ const std::string& channel_name, IPC::Channel::Mode mode,
PluginChannelFactory factory, MessageLoop* ipc_message_loop,
bool create_pipe_now) {
scoped_refptr<PluginChannelBase> channel;
- PluginChannelMap::const_iterator iter = g_plugin_channels_.find(channel_key);
+ PluginChannelMap::const_iterator iter = g_plugin_channels_.find(channel_name);
if (iter == g_plugin_channels_.end()) {
channel = factory();
} else {
@@ -43,14 +40,10 @@ PluginChannelBase* PluginChannelBase::GetChannel(
DCHECK(channel != NULL);
if (!channel->channel_valid()) {
- channel->channel_name_ = channel_key;
- if (mode == IPC::Channel::MODE_SERVER) {
- channel->channel_name_.append(".");
- channel->channel_name_.append(base::IntToString(next_pipe_id++));
- }
+ channel->channel_name_ = channel_name;
channel->mode_ = mode;
if (channel->Init(ipc_message_loop, create_pipe_now)) {
- g_plugin_channels_[channel_key] = channel;
+ g_plugin_channels_[channel_name] = channel;
} else {
channel = NULL;
}
diff --git a/chrome/plugin/plugin_channel_base.h b/chrome/plugin/plugin_channel_base.h
index 3924c07..026c06f 100644
--- a/chrome/plugin/plugin_channel_base.h
+++ b/chrome/plugin/plugin_channel_base.h
@@ -74,7 +74,7 @@ class PluginChannelBase : public IPC::Channel::Listener,
// must still ref count the returned value. When there are no more routes
// on the channel and its ref count is 0, the object deletes itself.
static PluginChannelBase* GetChannel(
- const std::string& channel_key, IPC::Channel::Mode mode,
+ const std::string& channel_name, IPC::Channel::Mode mode,
PluginChannelFactory factory, MessageLoop* ipc_message_loop,
bool create_pipe_now);
diff --git a/chrome/test/data/npapi/npn_plugin_delete_create_in_evaluate.html b/chrome/test/data/npapi/npn_plugin_delete_create_in_evaluate.html
deleted file mode 100644
index 12f0e4d..0000000
--- a/chrome/test/data/npapi/npn_plugin_delete_create_in_evaluate.html
+++ /dev/null
@@ -1,40 +0,0 @@
-<html>
-<head>
-<script src="npapi.js"></script>
-<script>
-
- function DeletePluginWithinScript() {
- var plugin_div = document.getElementById("PluginDiv");
- var html = plugin_div.innerHTML;
- html = html.replace("npobject_delete_create_plugin_in_evaluate",
- "invoke_js_function_on_create");
- plugin_div.innerHTML = "Object Deleted";
- plugin_div.innerHTML = html;
- }
-
- function PluginCreated() {
- onSuccess("npobject_delete_create_plugin_in_evaluate", 1);
- }
-
-</script>
-</head>
-
-<body>
-<div id="statusPanel" style="border: 1px solid red; width: 100%">
-Test running....
-</div>
-
-Tests the case where a plugin instance is deleted and created in the context
-of the NPN_Evaluate call.
-
-<DIV ID=PluginDiv>
-<embed type="application/vnd.npapi-test"
- src="foo"
- name="npobject_delete_create_plugin_in_evaluate"
- id="1"
- mode="np_embed"
->
-</DIV>
-
-</body>
-</html>
diff --git a/chrome/test/ui/npapi_uitest.cc b/chrome/test/ui/npapi_uitest.cc
index 8a2b50a..07970e8 100644
--- a/chrome/test/ui/npapi_uitest.cc
+++ b/chrome/test/ui/npapi_uitest.cc
@@ -219,20 +219,6 @@ TEST_F(NPAPIVisiblePluginTester, SelfDeletePluginInNPNEvaluate) {
kTestCompleteCookie, kTestCompleteSuccess,
action_max_timeout_ms());
}
-
-TEST_F(NPAPIVisiblePluginTester, SelfDeleteCreatePluginInNPNEvaluate) {
- if (UITest::in_process_renderer())
- return;
-
- const FilePath test_case(
- FILE_PATH_LITERAL("npn_plugin_delete_create_in_evaluate.html"));
- GURL url = ui_test_utils::GetTestUrl(FilePath(kTestDir), test_case);
- ASSERT_NO_FATAL_FAILURE(NavigateToURL(url));
- WaitForFinish("npobject_delete_create_plugin_in_evaluate", "1", url,
- kTestCompleteCookie, kTestCompleteSuccess,
- action_max_timeout_ms());
-}
-
#endif
// Flaky. See http://crbug.com/17645
diff --git a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc
index 10b3239..b62a764 100644
--- a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc
+++ b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc
@@ -17,8 +17,7 @@ NPObjectDeletePluginInNPN_Evaluate*
NPObjectLifetimeTest::NPObjectLifetimeTest(NPP id,
NPNetscapeFuncs *host_functions)
: PluginTest(id, host_functions),
- other_plugin_instance_object_(NULL),
- timer_id_(0) {
+ other_plugin_instance_object_(NULL) {
}
NPError NPObjectLifetimeTest::SetWindow(NPWindow* pNPWindow) {
@@ -31,8 +30,8 @@ NPError NPObjectLifetimeTest::SetWindow(NPWindow* pNPWindow) {
// We attempt to retreive the NPObject for the plugin instance identified
// by the NPObjectLifetimeTestInstance2 class as it may not have been
// instantiated yet.
- timer_id_ = SetTimer(window_handle, kNPObjectLifetimeTimer,
- kNPObjectLifetimeTimerElapse, TimerProc);
+ SetTimer(window_handle, kNPObjectLifetimeTimer, kNPObjectLifetimeTimerElapse,
+ TimerProc);
}
return NPERR_NO_ERROR;
}
@@ -41,11 +40,10 @@ void CALLBACK NPObjectLifetimeTest::TimerProc(
HWND window, UINT message, UINT timer_id,
unsigned long elapsed_milli_seconds) {
+ KillTimer(window, kNPObjectLifetimeTimer);
NPObjectLifetimeTest* this_instance =
reinterpret_cast<NPObjectLifetimeTest*>
(::GetProp(window, L"Plugin_Instance"));
- KillTimer(window, this_instance->timer_id_);
- this_instance->timer_id_ = 0;
this_instance->other_plugin_instance_object_ =
NPObjectLifetimeTestInstance2::plugin_instance_object_;
@@ -109,7 +107,7 @@ NPObjectDeletePluginInNPN_Evaluate::NPObjectDeletePluginInNPN_Evaluate(
NPP id, NPNetscapeFuncs *host_functions)
: PluginTest(id, host_functions),
plugin_instance_object_(NULL),
- timer_id_(0) {
+ npn_evaluate_timer_proc_set_(false) {
g_npn_evaluate_test_instance_ = this;
}
@@ -130,10 +128,12 @@ NPError NPObjectDeletePluginInNPN_Evaluate::SetWindow(NPWindow* np_window) {
// while it is being used in webkit as this leads to crashes and is a
// more accurate representation of the renderer crash as described in
// http://b/issue?id=1134683.
- if (!timer_id_) {
- timer_id_ = SetTimer(window_handle, kNPObjectLifetimeTimer,
- kNPObjectLifetimeTimerElapse, TimerProc);
+ if (!npn_evaluate_timer_proc_set_) {
+ npn_evaluate_timer_proc_set_ = true;
+ SetTimer(window_handle, kNPObjectLifetimeTimer, kNPObjectLifetimeTimerElapse,
+ TimerProc);
}
+
return NPERR_NO_ERROR;
}
@@ -141,8 +141,7 @@ void CALLBACK NPObjectDeletePluginInNPN_Evaluate::TimerProc(
HWND window, UINT message, UINT timer_id,
unsigned long elapsed_milli_seconds) {
- KillTimer(window, g_npn_evaluate_test_instance_->timer_id_);
- g_npn_evaluate_test_instance_->timer_id_ = 0;
+ KillTimer(window, kNPObjectLifetimeTimer);
NPObject *window_obj = NULL;
g_npn_evaluate_test_instance_->HostFunctions()->getvalue(
g_npn_evaluate_test_instance_->id(), NPNVWindowNPObject,
diff --git a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.h b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.h
index 60d0314..4303d99 100644
--- a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.h
+++ b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.h
@@ -30,7 +30,6 @@ class NPObjectLifetimeTest : public PluginTest {
#if defined(OS_WIN)
static void CALLBACK TimerProc(HWND window, UINT message, UINT timer_id,
unsigned long elapsed_milli_seconds);
- UINT_PTR timer_id_;
#endif
DISALLOW_IMPLICIT_CONSTRUCTORS(NPObjectLifetimeTest);
};
@@ -68,10 +67,10 @@ class NPObjectDeletePluginInNPN_Evaluate : public PluginTest {
#if defined(OS_WIN)
static void CALLBACK TimerProc(HWND window, UINT message, UINT timer_id,
unsigned long elapsed_milli_seconds);
- UINT_PTR timer_id_;
#endif
private:
+ bool npn_evaluate_timer_proc_set_;
static NPObjectDeletePluginInNPN_Evaluate* g_npn_evaluate_test_instance_;
DISALLOW_IMPLICIT_CONSTRUCTORS(NPObjectDeletePluginInNPN_Evaluate);
diff --git a/webkit/glue/plugins/test/plugin_test_factory.cc b/webkit/glue/plugins/test/plugin_test_factory.cc
index 62a3977..e2b42b3 100644
--- a/webkit/glue/plugins/test/plugin_test_factory.cc
+++ b/webkit/glue/plugins/test/plugin_test_factory.cc
@@ -66,8 +66,7 @@ PluginTest* CreatePluginTest(const std::string& test_name,
new_test = new NPObjectLifetimeTestInstance2(instance, host_functions);
} else if (test_name == "new_fails") {
new_test = new NewFailsTest(instance, host_functions);
- } else if (test_name == "npobject_delete_plugin_in_evaluate" ||
- test_name == "npobject_delete_create_plugin_in_evaluate") {
+ } else if (test_name == "npobject_delete_plugin_in_evaluate") {
new_test = new NPObjectDeletePluginInNPN_Evaluate(instance, host_functions);
#endif
} else if (test_name == "plugin_javascript_open_popup_with_plugin") {
@@ -87,8 +86,7 @@ PluginTest* CreatePluginTest(const std::string& test_name,
} else if (test_name == "hidden_plugin" ||
test_name == "create_instance_in_paint" ||
test_name == "alert_in_window_message" ||
- test_name == "ensure_scripting_works_in_destroy" ||
- test_name == "invoke_js_function_on_create") {
+ test_name == "ensure_scripting_works_in_destroy") {
new_test = new WindowedPluginTest(instance, host_functions);
#endif
}
diff --git a/webkit/glue/plugins/test/plugin_windowed_test.cc b/webkit/glue/plugins/test/plugin_windowed_test.cc
index 461fc20..5ae8e30 100644
--- a/webkit/glue/plugins/test/plugin_windowed_test.cc
+++ b/webkit/glue/plugins/test/plugin_windowed_test.cc
@@ -36,8 +36,7 @@ NPError WindowedPluginTest::SetWindow(NPWindow* pNPWindow) {
}
if ((test_name() == "create_instance_in_paint" && test_id() == "1") ||
- test_name() == "alert_in_window_message" ||
- test_name() == "invoke_js_function_on_create") {
+ test_name() == "alert_in_window_message") {
static ATOM window_class = 0;
if (!window_class) {
WNDCLASSEX wcex;
@@ -131,11 +130,6 @@ 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() ==
- "invoke_js_function_on_create" &&
- message == WM_PAINT) {
- this_ptr->done_ = true;
- CallJSFunction(this_ptr, "PluginCreated");
}
}
generated by cgit v1.1 at 2025-03-10 18:47:47 +0000