Change SingletonHwnd to be based on WindowImpl.

Change SingletonHwnd to be based on WindowImpl.

This removes the need for some Windows plumbing in
SingletonHwnd and fixes an issue where a WndProc
message would be getting sent after the Singleton
destruction, which was being triggered in ui_tests.

WindowImpl is robust against the above issue because
it stores a pointer to itself in the window user data
and clears it when it is destroyed.

BUG=105550
TEST=Running ui_tests with use_canvas_skia=1 on Windows shouldn't crash.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=130448

[sky, 2012-04-02 22:12:46]

LGTM

[msw, 2012-04-02 22:51:03]

http://codereview.chromium.org/9965082/diff/5001/ui/base/win/singleton_hwnd.cc
File ui/base/win/singleton_hwnd.cc (right):

http://codereview.chromium.org/9965082/diff/5001/ui/base/win/singleton_hwnd.c...
ui/base/win/singleton_hwnd.cc:35: result = ::DefWindowProc(window, message,
wparam, lparam);
Can you return false here without explicitly calling DefWindowProc?

DefWindowProc can potentially open up a can of worms. There's no visible window
here to experience visible artifacts like NativeWidgetWin, but you might face
re-entrance and potentially wonky behavior by Windows.

[Alexei Svitkine (slow), 2012-04-03 14:19:15]

On 2012/04/02 22:51:03, msw wrote:
> http://codereview.chromium.org/9965082/diff/5001/ui/base/win/singleton_hwnd.cc
> File ui/base/win/singleton_hwnd.cc (right):
> 
>
http://codereview.chromium.org/9965082/diff/5001/ui/base/win/singleton_hwnd.c...
> ui/base/win/singleton_hwnd.cc:35: result = ::DefWindowProc(window, message,
> wparam, lparam);
> Can you return false here without explicitly calling DefWindowProc?
> 
> DefWindowProc can potentially open up a can of worms. There's no visible
window
> here to experience visible artifacts like NativeWidgetWin, but you might face
> re-entrance and potentially wonky behavior by Windows.

I don't think we can get rid of DefWindowProc() here. The reason is that the
WndProc return value meaning depends on the message sent, so e.g. we can't e.g.
just return 0.

Note that the original code I'm changing also returned DefWindowProc(), so I'm
not introducing anything new in this area with this CL.

On the other hand, I can just return false from ProcessWindowMessage() to make
the code shorter. This will make WindowImpl call DefWindowProc() for us, so the
function still gets called, but it's less code. I've made that change.

[msw, 2012-04-03 17:48:22]

Ah you're right; LGTM.

[commit-bot: I haz the power, 2012-04-03 17:50:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asvitkine@chromium.org/9965082/1003

[commit-bot: I haz the power, 2012-04-03 18:38:58]

Try job failure for 9965082-1003 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-03 18:40:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asvitkine@chromium.org/9965082/1003