ExternalTabContainer should subclass WidgetWin rather than Widget and ATL CWi...

ExternalTabContainer should subclass WidgetWin rather than Widget and ATL CWindowImpl. This makes it much easier to extend the API of Widget.

Cleans up ExternalTabContainer to better match chrome style.

BUG=none
TEST=run ui tests, verify ExternalTabContainer tests pass.

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

[ananta, 2009-05-27 13:46:24]

LGTM

http://codereview.chromium.org/113892/diff/1/2
File chrome/browser/external_tab_container.cc (right):

http://codereview.chromium.org/113892/diff/1/2#newcode92
Line 92: SetWindowLong(GWL_STYLE, (GetWindowLong(GWL_STYLE) & ~WS_POPUP) |
style);
Should this be GetWindowLong(GWL_STYLE) | WS_POPUP | style?

[tommi (sloooow) - chröme, 2009-05-27 20:50:29]

http://codereview.chromium.org/113892/diff/1/2
File chrome/browser/external_tab_container.cc (right):

http://codereview.chromium.org/113892/diff/1/2#newcode52
Line 52: views::WidgetWin::Init(parent, bounds, true);
this changes the semantics from before where NULL was the initial parent.  Also,
there's no error handling.

http://codereview.chromium.org/113892/diff/1/2#newcode92
Line 92: SetWindowLong(GWL_STYLE, (GetWindowLong(GWL_STYLE) & ~WS_POPUP) |
style);
On 2009/05/27 13:46:24, ananta wrote:
> Should this be GetWindowLong(GWL_STYLE) | WS_POPUP | style?

I think this is how it should be.  We want to remove the WS_POPUP style at this
point.

http://codereview.chromium.org/113892/diff/1/2#newcode128
Line 128: return _wcsicmp(class_name.c_str(), chrome::kExternalTabWindowClass)
== 0;
maybe
return class_name.compare(chrome::kExternalTabWindowClass) == 0;

I think it's safe to be case sensitive here.