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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57372 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 93 insertions, 22 deletions

diff --git a/chrome/plugin/plugin_channel.cc b/chrome/plugin/plugin_channel.cc
index 2ea37be..941db8c 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_name = StringPrintf(
+  std::string channel_key = StringPrintf(
       "%d.r%d", base::GetCurrentProcId(), renderer_id);
 
   PluginChannel* channel =
       static_cast<PluginChannel*>(PluginChannelBase::GetChannel(
-          channel_name,
+          channel_key,
           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 5d18654..df7cccc 100644
--- a/chrome/plugin/plugin_channel_base.cc
+++ b/chrome/plugin/plugin_channel_base.cc
@@ -9,6 +9,7 @@
 #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"
 
@@ -24,13 +25,15 @@ 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_name, IPC::Channel::Mode mode,
+    const std::string& channel_key, 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_name);
+  PluginChannelMap::const_iterator iter = g_plugin_channels_.find(channel_key);
   if (iter == g_plugin_channels_.end()) {
     channel = factory();
   } else {
@@ -40,10 +43,14 @@ PluginChannelBase* PluginChannelBase::GetChannel(
   DCHECK(channel != NULL);
 
   if (!channel->channel_valid()) {
-    channel->channel_name_ = channel_name;
+    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->mode_ = mode;
     if (channel->Init(ipc_message_loop, create_pipe_now)) {
-      g_plugin_channels_[channel_name] = channel;
+      g_plugin_channels_[channel_key] = channel;
     } else {
       channel = NULL;
     }
diff --git a/chrome/plugin/plugin_channel_base.h b/chrome/plugin/plugin_channel_base.h
index a67049a..30fdc22 100644
--- a/chrome/plugin/plugin_channel_base.h
+++ b/chrome/plugin/plugin_channel_base.h
@@ -75,7 +75,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_name, IPC::Channel::Mode mode,
+      const std::string& channel_key, 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
new file mode 100644
index 0000000..12f0e4d
--- /dev/null
+++ b/chrome/test/data/npapi/npn_plugin_delete_create_in_evaluate.html
@@ -0,0 +1,40 @@
+<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 5739b05..f9323b3 100644
--- a/chrome/test/ui/npapi_uitest.cc
+++ b/chrome/test/ui/npapi_uitest.cc
@@ -219,6 +219,20 @@ 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 b62a764..10b3239 100644
--- a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc
+++ b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.cc
@@ -17,7 +17,8 @@ NPObjectDeletePluginInNPN_Evaluate*
 NPObjectLifetimeTest::NPObjectLifetimeTest(NPP id,
                                            NPNetscapeFuncs *host_functions)
     : PluginTest(id, host_functions),
-      other_plugin_instance_object_(NULL) {
+      other_plugin_instance_object_(NULL),
+      timer_id_(0) {
 }
 
 NPError NPObjectLifetimeTest::SetWindow(NPWindow* pNPWindow) {
@@ -30,8 +31,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.
-    SetTimer(window_handle, kNPObjectLifetimeTimer, kNPObjectLifetimeTimerElapse,
-             TimerProc);
+    timer_id_ = SetTimer(window_handle, kNPObjectLifetimeTimer,
+                         kNPObjectLifetimeTimerElapse, TimerProc);
   }
   return NPERR_NO_ERROR;
 }
@@ -40,10 +41,11 @@ 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_;
@@ -107,7 +109,7 @@ NPObjectDeletePluginInNPN_Evaluate::NPObjectDeletePluginInNPN_Evaluate(
     NPP id, NPNetscapeFuncs *host_functions)
     : PluginTest(id, host_functions),
       plugin_instance_object_(NULL),
-      npn_evaluate_timer_proc_set_(false) {
+      timer_id_(0) {
   g_npn_evaluate_test_instance_ = this;
 }
 
@@ -128,12 +130,10 @@ 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 (!npn_evaluate_timer_proc_set_) {
-    npn_evaluate_timer_proc_set_ = true;
-    SetTimer(window_handle, kNPObjectLifetimeTimer, kNPObjectLifetimeTimerElapse,
-             TimerProc);
+  if (!timer_id_) {
+    timer_id_ = SetTimer(window_handle, kNPObjectLifetimeTimer,
+                        kNPObjectLifetimeTimerElapse, TimerProc);
   }
-
   return NPERR_NO_ERROR;
 }
 
@@ -141,7 +141,8 @@ void CALLBACK NPObjectDeletePluginInNPN_Evaluate::TimerProc(
     HWND window, UINT message, UINT timer_id,
     unsigned long elapsed_milli_seconds) {
 
-  KillTimer(window, kNPObjectLifetimeTimer);
+  KillTimer(window, g_npn_evaluate_test_instance_->timer_id_);
+  g_npn_evaluate_test_instance_->timer_id_ = 0;
   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 4303d99..60d0314 100644
--- a/webkit/glue/plugins/test/plugin_npobject_lifetime_test.h
+++ b/webkit/glue/plugins/test/plugin_npobject_lifetime_test.h
@@ -30,6 +30,7 @@ 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);
 };
@@ -67,10 +68,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 e2b42b3..62a3977 100644
--- a/webkit/glue/plugins/test/plugin_test_factory.cc
+++ b/webkit/glue/plugins/test/plugin_test_factory.cc
@@ -66,7 +66,8 @@ 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") {
+  } else if (test_name == "npobject_delete_plugin_in_evaluate" ||
+             test_name == "npobject_delete_create_plugin_in_evaluate") {
     new_test = new NPObjectDeletePluginInNPN_Evaluate(instance, host_functions);
 #endif
   } else if (test_name == "plugin_javascript_open_popup_with_plugin") {
@@ -86,7 +87,8 @@ 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 == "ensure_scripting_works_in_destroy" ||
+             test_name == "invoke_js_function_on_create") {
     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 5ae8e30..461fc20 100644
--- a/webkit/glue/plugins/test/plugin_windowed_test.cc
+++ b/webkit/glue/plugins/test/plugin_windowed_test.cc
@@ -36,7 +36,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() == "invoke_js_function_on_create") {
     static ATOM window_class = 0;
     if (!window_class) {
       WNDCLASSEX wcex;
@@ -130,6 +131,11 @@ 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");
     }
   }