Page cycler tests failed on Vista because our windowless plugin code fails to...

Page cycler tests failed on Vista because our windowless plugin code fails to write the original flash windowless proc
in the window handle as a property via SetProp. Some debugging revealed that SetProp internally calls GlobalAddAtom which
fails at times with STATUS_NO_MEMORY. It appears that the flash plugin process runs out win32 heap quota at times.

To work around this we save away the original proc in a map.

This fixes bug http://code.google.com/p/chromium/issues/detail?id=48478

Bug=48478
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51831

[jam, 2010-07-08 04:07:50]

lgtm



On Wed, Jul 7, 2010 at 8:58 PM, <ananta@chromium.org> wrote:

> Reviewers: John Abd-El-Malek,
>
> Description:
> Page cycler tests failed on Vista because our windowless plugin code fails
> to
> write the original flash windowless proc
> in the window handle as a property via SetProp. Some debugging revealed
> that
> SetProp internally calls GlobalAddAtom which
> fails at times with STATUS_NO_MEMORY. It appears that the flash plugin
> process
> runs out win32 heap quota at times.
>
> To work around this we save away the original proc in a map.
>
> This fixes bug http://code.google.com/p/chromium/issues/detail?id=48478
>
> Bug=48478
>
>
> Please review this at http://codereview.chromium.org/2897002/show
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     webkit/glue/plugins/webplugin_delegate_impl_win.cc
>
>
> Index: webkit/glue/plugins/webplugin_delegate_impl_win.cc
> ===================================================================
> --- webkit/glue/plugins/webplugin_delegate_impl_win.cc  (revision 51716)
> +++ webkit/glue/plugins/webplugin_delegate_impl_win.cc  (working copy)
> @@ -4,6 +4,7 @@
>
>  #include "webkit/glue/plugins/webplugin_delegate_impl.h"
>
> +#include <map>
>  #include <string>
>  #include <vector>
>
> @@ -37,7 +38,6 @@
>  const wchar_t kWebPluginDelegateProperty[] = L"WebPluginDelegateProperty";
>  const wchar_t kPluginNameAtomProperty[] = L"PluginNameAtom";
>  const wchar_t kDummyActivationWindowName[] = L"DummyWindowForActivation";
> -const wchar_t kPluginOrigProc[] = L"OriginalPtr";
>  const wchar_t kPluginFlashThrottle[] = L"FlashThrottle";
>
>  // The fastest we are willing to process WM_USER+1 events for Flash.
> @@ -62,7 +62,10 @@
>
>  typedef std::deque<MSG> ThrottleQueue;
>  base::LazyInstance<ThrottleQueue>
> g_throttle_queue(base::LINKER_INITIALIZED);
> +base::LazyInstance<std::map<HWND, WNDPROC> > g_window_handle_proc_map(
> +    base::LINKER_INITIALIZED);
>
> +
>  // Helper object for patching the TrackPopupMenu API.
>  base::LazyInstance<iat_patch::IATPatchFunction>
> g_iat_patch_track_popup_menu(
>     base::LINKER_INITIALIZED);
> @@ -644,12 +647,16 @@
>  // static
>  LRESULT CALLBACK WebPluginDelegateImpl::FlashWindowlessWndProc(HWND hwnd,
>     UINT message, WPARAM wparam, LPARAM lparam) {
> -  WNDPROC old_proc = reinterpret_cast<WNDPROC>(GetProp(hwnd,
> kPluginOrigProc));
> +  std::map<HWND, WNDPROC>::iterator index =
> +      g_window_handle_proc_map.Get().find(hwnd);
> +
> +  WNDPROC old_proc = (*index).second;
>   DCHECK(old_proc);
>
>   switch (message) {
>     case WM_NCDESTROY: {
>       WebPluginDelegateImpl::ClearThrottleQueueForWindow(hwnd);
> +      g_window_handle_proc_map.Get().erase(index);
>       break;
>     }
>     // Flash may flood the message queue with WM_USER+1 message causing
> 100% CPU
> @@ -687,11 +694,7 @@
>         window, GWLP_WNDPROC,
>         reinterpret_cast<LONG>(wnd_proc)));
>     DCHECK(old_flash_proc);
> -    BOOL result = SetProp(window, kPluginOrigProc, old_flash_proc);
> -    if (!result) {
> -      LOG(ERROR) << "SetProp failed, last error = " << GetLastError();
> -      return FALSE;
> -    }
> +    g_window_handle_proc_map.Get()[window] = old_flash_proc;
>   }
>
>   return TRUE;
>
>
>