Adds a class that makes it easy to subclass a HWND and filter messages sent to it.

Adds a class that makes it easy to subclass a HWND and filter messages sent to it.

BUG=none
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=134383

[Ben Goodger (Google), 2012-04-27 19:41:37]

I could be persuaded to put this in ui/base/win instead of in views. It's not
really views-specific.

[Ben Goodger (Google), 2012-04-27 19:51:51]

On 2012/04/27 19:41:37, Ben Goodger (Google) wrote:
> I could be persuaded to put this in ui/base/win instead of in views. It's not
> really views-specific.

I persuaded myself.

[cpu_(ooo_6.6-7.5), 2012-04-27 20:20:25]

http://codereview.chromium.org/10256008/diff/1/ui/views/widget/hwnd_subclass.cc
File ui/views/widget/hwnd_subclass.cc (right):

http://codereview.chromium.org/10256008/diff/1/ui/views/widget/hwnd_subclass....
ui/views/widget/hwnd_subclass.cc:23: l_param) : 0;
why return 0 instead of CallWindowProc?

http://codereview.chromium.org/10256008/diff/1/ui/views/widget/hwnd_subclass....
ui/views/widget/hwnd_subclass.cc:30:
HWNDSubclass::HWNDSubclass(gfx::AcceleratedWidget target)
so a gfx::AcceleatedWidget is a hwnd? in the h file the declaration is explicit
HWNDSubclass(HWND target);

http://codereview.chromium.org/10256008/diff/1/ui/views/widget/hwnd_subclass....
ui/views/widget/hwnd_subclass.cc:32:
original_wnd_proc_(GetClassWndProc(target)),
but the window could be already be subclassed...

http://codereview.chromium.org/10256008/diff/1/ui/views/widget/hwnd_subclass....
ui/views/widget/hwnd_subclass.cc:40: ui::SetWindowProc(target_,
original_wnd_proc_);
I wont't bother unsubclassing, see my comment of line 23. I could be subclassed
again.

http://codereview.chromium.org/10256008/diff/1/ui/views/widget/hwnd_subclass....
ui/views/widget/hwnd_subclass.cc:78: return
reinterpret_cast<WNDPROC>(GetWindowLong(target, GWL_WNDPROC));
are we using this function?

[Ben Goodger (Google), 2012-04-27 20:57:20]

Updated. I had to do a bit of work to get ui_unittests to build on win_aura,
which is now also included in this CL.

[tfarina, 2012-04-27 21:06:02]

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.cc
File ui/base/win/hwnd_subclass.cc (right):

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.c...
ui/base/win/hwnd_subclass.cc:11: const char* kHWNDSubclassKey =
"__UI_BASE_WIN_HWND_SUBCLASS_PROC__";
nit: const char kFoo[] = "...";

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.c...
ui/base/win/hwnd_subclass.cc:20: return wrapped_wnd_proc ?
wrapped_wnd_proc->OnWndProc(hwnd,
nit: if you move wrapped_wnd_proc->OnWndProc(...) to the next line I bet you can
make it fit in one line.

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.c...
ui/base/win/hwnd_subclass.cc:26: }
nit: // namespace

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.c...
ui/base/win/hwnd_subclass.cc:60: WNDPROC HWNDSubclass::GetCurrentWndProc(HWND
target) {
nit: Worth moving this to unnamed namespace instead of making it a static method
of HWNDSubclass?

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.h
File ui/base/win/hwnd_subclass.h (right):

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.h...
ui/base/win/hwnd_subclass.h:15: #include "ui/gfx/native_widget_types.h"
nit: do you really need this include?

[cpu_(ooo_6.6-7.5), 2012-04-27 21:07:12]

lgtm

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.cc
File ui/base/win/hwnd_subclass.cc (right):

http://codereview.chromium.org/10256008/diff/6004/ui/base/win/hwnd_subclass.c...
ui/base/win/hwnd_subclass.cc:23: l_param) : 0;
so when the HWNDSubclass is destroyed, the messages won't go to the default
wndproc, they will be ignored. Is that what you want?

[Ben Goodger (Google), 2012-04-27 21:39:37]

Nope you're right, I added a call to DefWindowProc here. It does seem
unfortunate that I can't safely unsubclass though.

[cpu_(ooo_6.6-7.5), 2012-04-27 22:30:09]

still lgtm