Factor out window creation into base::WindowImpl. This class will be used in...

Factor out window creation into base::WindowImpl.  This class will be used in place of CWindowImpl to reduce our dependency on ATL.

BUG=none
TEST=none

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

[Peter Kasting, 2009-08-06 00:54:20]

Only one real problem, although I didn't go over things with a fine-toothed comb
(I trust you copied all the important bits correctly)

http://codereview.chromium.org/165022/diff/1/5
File base/window_impl.cc (right):

http://codereview.chromium.org/165022/diff/1/5#newcode196
Line 196: /*if (ViewsDelegate::views_delegate)
Um... What's up with this?  Maybe need to call a virtual function that
subclasses can use to do this, or something?

http://codereview.chromium.org/165022/diff/1/6
File base/window_impl.h (right):

http://codereview.chromium.org/165022/diff/1/6#newcode34
Line 34: BEGIN_MSG_MAP_EX(WidgetWin)
Nit: Do you need this?  I'd leave it out if not

http://codereview.chromium.org/165022/diff/1/2
File views/widget/widget_win.cc (right):

http://codereview.chromium.org/165022/diff/1/2#newcode65
Line 65: // Force creation of the RootView.  We may get a WM_SIZE after the
window is
Nit: Did you mean to have "Otherwise," before "We may"?

[James Hawkins, 2009-08-06 01:14:38]

http://codereview.chromium.org/165022/diff/1/5
File base/window_impl.cc (right):

http://codereview.chromium.org/165022/diff/1/5#newcode196
Line 196: /*if (ViewsDelegate::views_delegate)
On 2009/08/06 00:54:20, Peter Kasting wrote:
> Um... What's up with this?  Maybe need to call a virtual function that
> subclasses can use to do this, or something?

I wish lint would warn about the use of c-style comments.  I commented it like
this so that I wouldn't forget to fix it :-/  I need to add hIcon to the
ClassInfo struct and a method to set it in WindowImpl.  It'll be in the next
revision.

http://codereview.chromium.org/165022/diff/1/6
File base/window_impl.h (right):

http://codereview.chromium.org/165022/diff/1/6#newcode34
Line 34: BEGIN_MSG_MAP_EX(WidgetWin)
On 2009/08/06 00:54:20, Peter Kasting wrote:
> Nit: Do you need this?  I'd leave it out if not

Unfortunately we do.  The WndProc must call ProcessWindowMessage to handle the
messages in the message map (said message map is overridden by child classes). 
The other option is to have this WndProc just call DefWindowProc and have all
child classes call ProcessWindowMessage.  Having this empty message map seems
like a good tradeoff though.

http://codereview.chromium.org/165022/diff/1/2
File views/widget/widget_win.cc (right):

http://codereview.chromium.org/165022/diff/1/2#newcode65
Line 65: // Force creation of the RootView.  We may get a WM_SIZE after the
window is
On 2009/08/06 00:54:20, Peter Kasting wrote:
> Nit: Did you mean to have "Otherwise," before "We may"?

That was the idea.  I can add it.

[Ben Goodger (Google), 2009-08-07 20:15:43]

LGTM providing the items Peter listed are resolved.

http://codereview.chromium.org/165022/diff/15/19
File base/window_impl.cc (right):

http://codereview.chromium.org/165022/diff/15/19#newcode118
Line 118: void WindowImpl::Init(gfx::NativeView parent, const gfx::Rect& bounds)
{
you should probably use HWND in windows-specific code.

[James Hawkins, 2009-08-07 20:27:44]

On 2009/08/07 20:15:43, Ben Goodger wrote:
> LGTM providing the items Peter listed are resolved.
> 

Comments addressed.

> http://codereview.chromium.org/165022/diff/15/19
> File base/window_impl.cc (right):
> 
> http://codereview.chromium.org/165022/diff/15/19#newcode118
> Line 118: void WindowImpl::Init(gfx::NativeView parent, const gfx::Rect&
bounds)
> {
> you should probably use HWND in windows-specific code.

Done.

[Peter Kasting, 2009-08-07 20:35:49]

LGTM