Stop using an atom to store plugin name/version on Windows.

On the swarming bots we are seeing flakes in the plugin tests. Setting the atom fails because the atom table fills up. I tracked this down to the fact that sometimes the atom destruction path doesn't get hit depending on how the plugin shuts down. Rather than fix it, I removed this code and used a simpler mechanism to get the title & version in the browser process.

BUG=412042

Review URL: https://codereview.chromium.org/557893003

Cr-Commit-Position: refs/heads/master@{#294088}

chrome/browser/hang_monitor/hung_plugin_action.cc

@@ -83,10 +83,9 @@ bool HungPluginAction::OnHungWindowDetected(HWND hung_window,
   if (top_level_window_process_id != hung_window_process_id) {
     base::string16 plugin_name;
     base::string16 plugin_version;
-    GetPluginNameAndVersion(hung_window,
-                            top_level_window_process_id,
-                            &plugin_name,
-                            &plugin_version);
+
+    content::PluginService::GetInstance()->GetPluginInfoFromWindow(
+        hung_window, &plugin_name, &plugin_version);
     if (plugin_name.empty()) {
       plugin_name = l10n_util::GetStringUTF16(IDS_UNKNOWN_PLUGIN_NAME);
     } else if (kGTalkPluginName == plugin_name) {
@@ -160,30 +159,6 @@ void HungPluginAction::OnWindowResponsive(HWND window) {
   }
 }
 
-bool HungPluginAction::GetPluginNameAndVersion(HWND plugin_window,
-                                               DWORD browser_process_id,
-                                               base::string16* plugin_name,
-                                               base::string16* plugin_version) {
-  DCHECK(plugin_name);
-  DCHECK(plugin_version);
-  HWND window_to_check = plugin_window;
-  while (NULL != window_to_check) {
-    DWORD process_id = 0;
-    GetWindowThreadProcessId(window_to_check, &process_id);
-    if (process_id == browser_process_id) {
-      // If we have reached a window the that belongs to the browser process
-      // we have gone too far.
-      return false;
-    }
-    if (content::PluginService::GetInstance()->GetPluginInfoFromWindow(
-            window_to_check, plugin_name, plugin_version)) {
-      return true;
-    }
-    window_to_check = GetParent(window_to_check);
-  }
-  return false;
-}
-
 // static
 BOOL CALLBACK HungPluginAction::DismissMessageBox(HWND window, LPARAM ignore) {
   base::string16 class_name = gfx::GetClassName(window);

content/browser/plugin_browsertest.cc

@@ -242,11 +242,9 @@ IN_PROC_BROWSER_TEST_F(PluginTest, MAYBE(GetJavaScriptURL2)) {
 }
 
 // Test is flaky on linux/cros/win builders.  http://crbug.com/71904
-#if !defined(OS_WIN)  // http://crbug.com/412042
 IN_PROC_BROWSER_TEST_F(PluginTest, GetURLRedirectNotification) {
   LoadAndWait(GetURL("geturl_redirect_notify.html"));
 }
-#endif
 
 // Tests that identity is preserved for NPObjects passed from a plugin
 // into JavaScript.
@@ -356,11 +354,9 @@ IN_PROC_BROWSER_TEST_F(PluginTest, MAYBE(PluginThreadAsyncCall)) {
   LoadAndWait(GetURL("plugin_thread_async_call.html"));
 }
 
-#if !defined(OS_WIN)  // http://crbug.com/412042
 IN_PROC_BROWSER_TEST_F(PluginTest, PluginSingleRangeRequest) {
   LoadAndWait(GetURL("plugin_single_range_request.html"));
 }
-#endif
 
 #if !defined(OS_WIN) // http://crbug.com/396373
 // Test checking the privacy mode is on.

content/browser/plugin_process_host.cc

@@ -16,12 +16,14 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/lock.h"
 #include "content/browser/browser_child_process_host_impl.h"
 #include "content/browser/loader/resource_message_filter.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
@@ -56,6 +58,24 @@
 
 namespace content {
 
+namespace {
+
+base::LazyInstance<std::map<base::ProcessId, WebPluginInfo> >
+    g_process_webplugin_info = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<base::Lock>::Leaky
+    g_process_webplugin_info_lock = LAZY_INSTANCE_INITIALIZER;
+}
+
+bool PluginProcessHost::GetWebPluginInfoFromPluginPid(base::ProcessId pid,
+                                                      WebPluginInfo* info) {
+  base::AutoLock lock(g_process_webplugin_info_lock.Get());
+  if (!g_process_webplugin_info.Get().count(pid))
+    return false;
+
+  *info = g_process_webplugin_info.Get()[pid];
+  return true;
+}
+
 #if defined(OS_WIN)
 void PluginProcessHost::OnPluginWindowDestroyed(HWND window, HWND parent) {
   // The window is destroyed at this point, we just care about its parent, which
@@ -106,8 +126,9 @@ class PluginSandboxedProcessLauncherDelegate
 };
 
 PluginProcessHost::PluginProcessHost()
+    : pid_(base::kNullProcessId)
 #if defined(OS_MACOSX)
-    : plugin_cursor_visible_(true)
+    , plugin_cursor_visible_(true)
 #endif
 {
   process_.reset(new BrowserChildProcessHostImpl(PROCESS_TYPE_PLUGIN, this));
@@ -144,6 +165,11 @@ PluginProcessHost::~PluginProcessHost() {
 #endif
   // Cancel all pending and sent requests.
   CancelRequests();
+
+  {
+    base::AutoLock lock(g_process_webplugin_info_lock.Get());
+    g_process_webplugin_info.Get()[pid_] = info_;
+  }
 }
 
 bool PluginProcessHost::Send(IPC::Message* message) {
@@ -284,6 +310,12 @@ void PluginProcessHost::OnChannelConnected(int32 peer_pid) {
   }
 
   pending_requests_.clear();
+
+  pid_ = peer_pid;
+  {
+    base::AutoLock lock(g_process_webplugin_info_lock.Get());
+    g_process_webplugin_info.Get()[pid_] = info_;
+  }
 }
 
 void PluginProcessHost::OnChannelError() {

content/browser/plugin_process_host.h

@@ -16,6 +16,7 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/process/process_handle.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/browser_child_process_host_delegate.h"
 #include "content/public/browser/browser_child_process_host_iterator.h"
@@ -120,6 +121,12 @@ class CONTENT_EXPORT PluginProcessHost : public BrowserChildProcessHostDelegate,
   void AddWindow(HWND window);
 #endif
 
+  // Given a pid of a plugin process, returns the plugin information in |info|
+  // if we know about that process. Otherwise returns false.
+  // This method can be called on any thread.
+  static bool GetWebPluginInfoFromPluginPid(base::ProcessId pid,
+                                            WebPluginInfo* info);
+
  private:
   // Sends a message to the plugin process to request creation of a new channel
   // for the given mime type.
@@ -161,6 +168,9 @@ class CONTENT_EXPORT PluginProcessHost : public BrowserChildProcessHostDelegate,
   // Information about the plugin.
   WebPluginInfo info_;
 
+  // The pid of the plugin process.
+  int pid_;
+
 #if defined(OS_WIN)
   // Tracks plugin parent windows created on the UI thread.
   std::set<HWND> plugin_parent_windows_set_;

content/browser/plugin_service_impl.cc

@@ -841,10 +841,15 @@ bool PluginServiceImpl::GetPluginInfoFromWindow(
   if (!IsPluginWindow(window))
     return false;
 
-  GetPluginPropertyFromWindow(
-          window, kPluginNameAtomProperty, plugin_name);
-  GetPluginPropertyFromWindow(
-          window, kPluginVersionAtomProperty, plugin_version);
+
+  DWORD process_id = 0;
+  GetWindowThreadProcessId(window, &process_id);
+  WebPluginInfo info;
+  if (!PluginProcessHost::GetWebPluginInfoFromPluginPid(process_id, &info))
+    return false;
+
+  *plugin_name = info.name;
+  *plugin_version = info.version;
   return true;
 }
 

content/child/npapi/webplugin_delegate_impl_win.cc

@@ -499,35 +499,6 @@ bool WebPluginDelegateImpl::WindowedCreatePlugin() {
 
   BOOL result = SetProp(windowed_handle_, kWebPluginDelegateProperty, this);
   DCHECK(result == TRUE) << "SetProp failed, last error = " << GetLastError();
-  // Get the name and version of the plugin, create atoms and set them in a
-  // window property. Use atoms so that other processes can access the name and
-  // version of the plugin that this window is hosting.
-  if (instance_ != NULL) {
-    PluginLib* plugin_lib = instance()->plugin_lib();
-    if (plugin_lib != NULL) {
-      std::wstring plugin_name = plugin_lib->plugin_info().name;
-      if (!plugin_name.empty()) {
-        ATOM plugin_name_atom = GlobalAddAtomW(plugin_name.c_str());
-        DCHECK_NE(0, plugin_name_atom) << " last error = " <<
-            GetLastError();
-        result = SetProp(windowed_handle_,
-            kPluginNameAtomProperty,
-            reinterpret_cast<HANDLE>(plugin_name_atom));
-        DCHECK(result == TRUE) << "SetProp failed, last error = " <<
-            GetLastError();
-      }
-      base::string16 plugin_version = plugin_lib->plugin_info().version;
-      if (!plugin_version.empty()) {
-        ATOM plugin_version_atom = GlobalAddAtomW(plugin_version.c_str());
-        DCHECK_NE(0, plugin_version_atom);
-        result = SetProp(windowed_handle_,
-            kPluginVersionAtomProperty,
-            reinterpret_cast<HANDLE>(plugin_version_atom));
-        DCHECK(result == TRUE) << "SetProp failed, last error = " <<
-            GetLastError();
-      }
-    }
-  }
 
   // Calling SetWindowLongPtrA here makes the window proc ASCII, which is
   // required by at least the Shockwave Director plug-in.
@@ -1027,14 +998,6 @@ LRESULT CALLBACK WebPluginDelegateImpl::NativeWndProc(
 
     if (message == WM_NCDESTROY) {
       RemoveProp(hwnd, kWebPluginDelegateProperty);
-      ATOM plugin_name_atom = reinterpret_cast<ATOM>(
-          RemoveProp(hwnd, kPluginNameAtomProperty));
-      if (plugin_name_atom != 0)
-        GlobalDeleteAtom(plugin_name_atom);
-      ATOM plugin_version_atom = reinterpret_cast<ATOM>(
-          RemoveProp(hwnd, kPluginVersionAtomProperty));
-      if (plugin_version_atom != 0)
-        GlobalDeleteAtom(plugin_version_atom);
       ClearThrottleQueueForWindow(hwnd);
     }
   }

content/common/plugin_constants_win.cc

@@ -9,8 +9,6 @@ namespace content {
 const base::char16 kNativeWindowClassName[] = L"NativeWindowClass";
 const base::char16 kWrapperNativeWindowClassName[] =
     L"WrapperNativeWindowClass";
-const base::char16 kPluginNameAtomProperty[] = L"PluginNameAtom";
-const base::char16 kPluginVersionAtomProperty[] = L"PluginVersionAtom";
 const base::char16 kDummyActivationWindowName[] = L"DummyWindowForActivation";
 const base::char16 kPaintMessageName[] = L"Chrome_CustomPaintil";
 const base::char16 kRegistryMozillaPlugins[] = L"SOFTWARE\\MozillaPlugins";

content/common/plugin_constants_win.h

@@ -21,8 +21,6 @@ extern const base::char16 kNativeWindowClassName[];
 // is created on the browser UI thread.
 extern const base::char16 kWrapperNativeWindowClassName[];
 
-extern const base::char16 kPluginNameAtomProperty[];
-extern const base::char16 kPluginVersionAtomProperty[];
 extern const base::char16 kDummyActivationWindowName[];
 
 // The name of the custom window message that the browser uses to tell the