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.
Review URL: http://codereview.chromium.org/113892

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16996 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 122 insertions, 246 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 0923492..c2b2db8 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -59,6 +59,7 @@
 
 #if defined(OS_WIN)
 #include "chrome/browser/views/bookmark_bar_view.h"
+#include "views/widget/root_view.h"
 #include "views/widget/widget_win.h"
 #include "views/window/window.h"
 #endif
@@ -2428,7 +2429,7 @@ void AutomationProvider::CreateExternalTab(HWND parent,
   if (tab_contents) {
     *tab_handle = tab_tracker_->Add(&tab_contents->controller());
     external_tab_container->set_tab_handle(*tab_handle);
-    *tab_container_window = *external_tab_container;
+    *tab_container_window = external_tab_container->GetNativeView();
   } else {
     delete external_tab_container;
   }
diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc
index 608b34e..24fb0c0 100644
--- a/chrome/browser/external_tab_container.cc
+++ b/chrome/browser/external_tab_container.cc
@@ -18,8 +18,6 @@
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/test/automation/automation_messages.h"
-// Included for SetRootViewForHWND.
-#include "views/widget/widget_win.h"
 
 static const wchar_t kWindowObjectKey[] = L"ChromeWindowObject";
 
@@ -29,7 +27,6 @@ static const wchar_t kWindowObjectKey[] = L"ChromeWindowObject";
 ExternalTabContainer::ExternalTabContainer(
     AutomationProvider* automation)
     : automation_(automation),
-      root_view_(this),
       tab_contents_(NULL),
       external_accel_table_(NULL),
       external_accel_entry_count_(0),
@@ -39,34 +36,27 @@ ExternalTabContainer::ExternalTabContainer(
 }
 
 ExternalTabContainer::~ExternalTabContainer() {
-  Uninitialize(m_hWnd);
+  Uninitialize(GetNativeView());
 }
 
-bool ExternalTabContainer::Init(Profile* profile, HWND parent,
-                                const gfx::Rect& dimensions,
-                                unsigned int style) {
+bool ExternalTabContainer::Init(Profile* profile,
+                                HWND parent,
+                                const gfx::Rect& bounds,
+                                DWORD style) {
   if (IsWindow()) {
     NOTREACHED();
     return false;
   }
 
-  // First create the container window
-  if (!Create(NULL, dimensions.ToRECT())) {
-    NOTREACHED();
-    return false;
-  }
+  set_window_style(WS_POPUP);
+  views::WidgetWin::Init(parent, bounds, true);
 
   // We don't ever remove the prop because the lifetime of this object
   // is the same as the lifetime of the window
-  SetProp(*this, kWindowObjectKey, this);
+  SetProp(GetNativeView(), kWindowObjectKey, this);
 
-  views::SetRootViewForHWND(m_hWnd, &root_view_);
-  // CreateFocusManager will subclass this window and delete the FocusManager
-  // instance when this window goes away.
   views::FocusManager* focus_manager =
-      views::FocusManager::CreateFocusManager(m_hWnd, GetRootView());
-
-  DCHECK(focus_manager);
+      views::FocusManager::GetFocusManager(GetNativeView());
   focus_manager->AddKeystrokeListener(this);
 
   tab_contents_ = new TabContents(profile, NULL, MSG_ROUTING_NONE, NULL);
@@ -76,7 +66,8 @@ bool ExternalTabContainer::Init(Profile* profile, HWND parent,
   // Create a TabContentsContainer to handle focus cycling using Tab and
   // Shift-Tab.
   tab_contents_container_ = new TabContentsContainer;
-  root_view_.AddChildView(tab_contents_container_);
+  SetContentsView(tab_contents_container_);
+
   // Note that SetTabContents must be called after AddChildView is called
   tab_contents_container_->ChangeTabContents(tab_contents_);
 
@@ -98,58 +89,62 @@ bool ExternalTabContainer::Init(Profile* profile, HWND parent,
   // Note that it's important to do this before we call SetParent since
   // during the SetParent call we will otherwise get a WA_ACTIVATE call
   // that causes us to steal the current focus.
-  ModifyStyle(WS_POPUP, style, 0);
+  SetWindowLong(GWL_STYLE, (GetWindowLong(GWL_STYLE) & ~WS_POPUP) | style);
 
   // Now apply the parenting and style
   if (parent)
-    SetParent(parent);
+    SetParent(GetNativeView(), parent);
 
   ::ShowWindow(tab_contents_->GetNativeView(), SW_SHOWNA);
   return true;
 }
 
-bool ExternalTabContainer::Uninitialize(HWND window) {
-  if (::IsWindow(window)) {
-    views::FocusManager* focus_manager =
-        views::FocusManager::GetFocusManager(window);
-    if (focus_manager) {
-      focus_manager->RemoveKeystrokeListener(this);
-    }
-  }
+void ExternalTabContainer::SetAccelerators(HACCEL accel_table,
+                                           int accel_table_entry_count) {
+  external_accel_table_ = accel_table;
+  external_accel_entry_count_ = accel_table_entry_count;
+}
 
-  root_view_.RemoveAllChildViews(true);
-  if (tab_contents_) {
-    NotificationService::current()->Notify(
-        NotificationType::EXTERNAL_TAB_CLOSED,
-        Source<NavigationController>(&tab_contents_->controller()),
-        Details<ExternalTabContainer>(this));
+void ExternalTabContainer::ProcessUnhandledAccelerator(const MSG& msg) {
+  // We just received an accelerator key that we had sent to external host
+  // back. Since the external host was not interested in handling this, we
+  // need to dispatch this message as if we had just peeked this out. (we
+  // also need to call TranslateMessage to generate a WM_CHAR if needed).
+  TranslateMessage(&msg);
+  DispatchMessage(&msg);
+}
 
-    delete tab_contents_;
-    tab_contents_ = NULL;
+void ExternalTabContainer::SetInitialFocus(bool reverse) {
+  DCHECK(tab_contents_);
+  if (tab_contents_) {
+    static_cast<TabContents*>(tab_contents_)->Focus();
+    static_cast<TabContents*>(tab_contents_)->SetInitialFocus(reverse);
   }
-
-  return true;
 }
 
-void ExternalTabContainer::OnFinalMessage(HWND window) {
-  delete this;
+// static
+bool ExternalTabContainer::IsExternalTabContainer(HWND window) {
+  std::wstring class_name = win_util::GetClassName(window);
+  return _wcsicmp(class_name.c_str(), chrome::kExternalTabWindowClass) == 0;
 }
 
-LRESULT ExternalTabContainer::OnSize(UINT, WPARAM, LPARAM, BOOL& handled) {
-  if (tab_contents_) {
-    RECT client_rect = {0};
-    GetClientRect(&client_rect);
-    ::SetWindowPos(tab_contents_->GetNativeView(), NULL, client_rect.left,
-                   client_rect.top, client_rect.right - client_rect.left,
-                   client_rect.bottom - client_rect.top, SWP_NOZORDER);
+// static
+ExternalTabContainer* ExternalTabContainer::GetContainerForTab(
+    HWND tab_window) {
+  HWND parent_window = ::GetParent(tab_window);
+  if (!::IsWindow(parent_window)) {
+    return NULL;
   }
-  return 0;
+  if (!IsExternalTabContainer(parent_window)) {
+    return NULL;
+  }
+  ExternalTabContainer* container = reinterpret_cast<ExternalTabContainer*>(
+      GetProp(parent_window, kWindowObjectKey));
+  return container;
 }
 
-LRESULT ExternalTabContainer::OnDestroy(UINT, WPARAM, LPARAM, BOOL& handled) {
-  Uninitialize(m_hWnd);
-  return 0;
-}
+////////////////////////////////////////////////////////////////////////////////
+// ExternalTabContainer, TabContentsDelegate implementation:
 
 void ExternalTabContainer::OpenURLFromTab(TabContents* source,
                            const GURL& url,
@@ -241,6 +236,12 @@ void ExternalTabContainer::ForwardMessageToExternalHost(
   }
 }
 
+ExtensionFunctionDispatcher* ExternalTabContainer::
+    CreateExtensionFunctionDispatcher(RenderViewHost* render_view_host,
+                                      const std::string& extension_id) {
+  return new ExtensionFunctionDispatcher(render_view_host, NULL, extension_id);
+}
+
 bool ExternalTabContainer::TakeFocus(bool reverse) {
   if (automation_) {
     views::FocusManager* focus_manager =
@@ -256,6 +257,9 @@ bool ExternalTabContainer::TakeFocus(bool reverse) {
   return true;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ExternalTabContainer, NotificationObserver implementation:
+
 void ExternalTabContainer::Observe(NotificationType type,
                                    const NotificationSource& source,
                                    const NotificationDetails& details) {
@@ -316,64 +320,16 @@ void ExternalTabContainer::Observe(NotificationType type,
   }
 }
 
-void ExternalTabContainer::GetBounds(gfx::Rect* out,
-                                     bool including_frame) const {
-  CRect crect;
-  GetWindowRect(&crect);
-  *out = gfx::Rect(crect);
-}
-
-void ExternalTabContainer::SetBounds(const gfx::Rect& bounds) {
-  SetBounds(bounds, NULL);
-}
-
-void ExternalTabContainer::SetBounds(const gfx::Rect& bounds,
-                                     gfx::NativeView other_window) {
-  NOTIMPLEMENTED();
-}
+////////////////////////////////////////////////////////////////////////////////
+// ExternalTabContainer, views::WidgetWin overrides:
 
-void ExternalTabContainer::Close() {
-  NOTIMPLEMENTED();
+void ExternalTabContainer::OnDestroy() {
+  Uninitialize(GetNativeView());
+  WidgetWin::OnDestroy();
 }
 
-void ExternalTabContainer::CloseNow() {
-  NOTIMPLEMENTED();
-}
-
-void ExternalTabContainer::Show() {
-  NOTIMPLEMENTED();
-}
-
-void ExternalTabContainer::Hide() {
-  NOTIMPLEMENTED();
-}
-
-gfx::NativeView ExternalTabContainer::GetNativeView() const {
-  return m_hWnd;
-}
-
-void ExternalTabContainer::PaintNow(const gfx::Rect& update_rect) {
-  RECT native_update_rect = update_rect.ToRECT();
-  RedrawWindow(&native_update_rect,
-               NULL,
-               RDW_INVALIDATE | RDW_ALLCHILDREN | RDW_NOERASE);
-}
-
-void ExternalTabContainer::SetOpacity(unsigned char opacity) {
-  NOTIMPLEMENTED();
-}
-
-views::RootView* ExternalTabContainer::GetRootView() {
-  return const_cast<views::RootView*>(&root_view_);
-}
-
-bool ExternalTabContainer::IsVisible() const {
-  return !!::IsWindowVisible(*this);
-}
-
-bool ExternalTabContainer::IsActive() const {
-  return win_util::IsWindowActive(*this);
-}
+////////////////////////////////////////////////////////////////////////////////
+// ExternalTabContainer, views::KeystrokeListener implementation:
 
 bool ExternalTabContainer::ProcessKeyStroke(HWND window, UINT message,
                                             WPARAM wparam, LPARAM lparam) {
@@ -412,52 +368,25 @@ bool ExternalTabContainer::ProcessKeyStroke(HWND window, UINT message,
   return false;
 }
 
-void ExternalTabContainer::SetAccelerators(HACCEL accel_table,
-                                           int accel_table_entry_count) {
-  external_accel_table_ = accel_table;
-  external_accel_entry_count_ = accel_table_entry_count;
-}
+////////////////////////////////////////////////////////////////////////////////
+// ExternalTabContainer, private:
 
-void ExternalTabContainer::ProcessUnhandledAccelerator(const MSG& msg) {
-  // We just received an accelerator key that we had sent to external host
-  // back. Since the external host was not interested in handling this, we
-  // need to dispatch this message as if we had just peeked this out. (we
-  // also need to call TranslateMessage to generate a WM_CHAR if needed).
-  TranslateMessage(&msg);
-  DispatchMessage(&msg);
-}
-
-void ExternalTabContainer::SetInitialFocus(bool reverse) {
-  DCHECK(tab_contents_);
-  if (tab_contents_) {
-    static_cast<TabContents*>(tab_contents_)->Focus();
-    static_cast<TabContents*>(tab_contents_)->SetInitialFocus(reverse);
+void ExternalTabContainer::Uninitialize(HWND window) {
+  if (::IsWindow(window)) {
+    views::FocusManager* focus_manager =
+        views::FocusManager::GetFocusManager(window);
+    if (focus_manager)
+      focus_manager->RemoveKeystrokeListener(this);
   }
-}
-
-// static
-bool ExternalTabContainer::IsExternalTabContainer(HWND window) {
-  std::wstring class_name = win_util::GetClassName(window);
-  return _wcsicmp(class_name.c_str(), chrome::kExternalTabWindowClass) == 0;
-}
 
-ExtensionFunctionDispatcher* ExternalTabContainer::
-    CreateExtensionFunctionDispatcher(RenderViewHost* render_view_host,
-                                      const std::string& extension_id) {
-  return new ExtensionFunctionDispatcher(render_view_host, NULL, extension_id);
-}
+  if (tab_contents_) {
+    NotificationService::current()->Notify(
+        NotificationType::EXTERNAL_TAB_CLOSED,
+        Source<NavigationController>(&tab_contents_->controller()),
+        Details<ExternalTabContainer>(this));
 
-// static
-ExternalTabContainer* ExternalTabContainer::GetContainerForTab(
-    HWND tab_window) {
-  HWND parent_window = ::GetParent(tab_window);
-  if (!::IsWindow(parent_window)) {
-    return NULL;
-  }
-  if (!IsExternalTabContainer(parent_window)) {
-    return NULL;
+    delete tab_contents_;
+    tab_contents_ = NULL;
   }
-  ExternalTabContainer* container = reinterpret_cast<ExternalTabContainer*>(
-      GetProp(parent_window, kWindowObjectKey));
-  return container;
 }
+
diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h
index 536dd52..4edb8b9 100644
--- a/chrome/browser/external_tab_container.h
+++ b/chrome/browser/external_tab_container.h
@@ -5,25 +5,15 @@
 #ifndef CHROME_BROWSER_EXTERNAL_TAB_CONTAINER_H_
 #define CHROME_BROWSER_EXTERNAL_TAB_CONTAINER_H_
 
-#include <atlbase.h>
-#include <atlapp.h>
-#include <atlcrack.h>
-#include <atlmisc.h>
-#include <string>
-
-#include "base/basictypes.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
-#include "chrome/common/chrome_constants.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
-#include "views/focus/focus_manager.h"
-#include "views/widget/root_view.h"
-#include "views/widget/widget.h"
+#include "views/widget/widget_win.h"
 
 class AutomationProvider;
-class TabContents;
 class Profile;
 class TabContentsContainer;
+
 // This class serves as the container window for an external tab.
 // An external tab is a Chrome tab that is meant to displayed in an
 // external process. This class provides the FocusManger needed by the
@@ -31,39 +21,39 @@ class TabContentsContainer;
 // TODO(beng): Should override WidgetWin instead of Widget.
 class ExternalTabContainer : public TabContentsDelegate,
                              public NotificationObserver,
-                             public views::Widget,
-                             public views::KeystrokeListener,
-                             public CWindowImpl<ExternalTabContainer,
-                                                CWindow,
-                                                CWinTraits<WS_POPUP |
-                                                    WS_CLIPCHILDREN>> {
+                             public views::WidgetWin,
+                             public views::KeystrokeListener {
  public:
-  BEGIN_MSG_MAP(ExternalTabContainer)
-    MESSAGE_HANDLER(WM_SIZE, OnSize)
-    MESSAGE_HANDLER(WM_DESTROY, OnDestroy)
-  END_MSG_MAP()
-
-  DECLARE_WND_CLASS(chrome::kExternalTabWindowClass)
-
-  ExternalTabContainer(AutomationProvider* automation);
+  explicit ExternalTabContainer(AutomationProvider* automation);
   ~ExternalTabContainer();
 
-  TabContents* tab_contents() const {
-    return tab_contents_;
-  }
+  TabContents* tab_contents() const { return tab_contents_; }
 
   // Temporary hack so we can send notifications back
-  void set_tab_handle(int handle) {
-    tab_handle_ = handle;
-  }
+  void set_tab_handle(int handle) { tab_handle_ = handle; }
 
-  bool Init(Profile* profile, HWND parent, const gfx::Rect& dimensions,
-            unsigned int style);
+  bool Init(Profile* profile,
+            HWND parent,
+            const gfx::Rect& bounds,
+            DWORD style);
 
-  // This function gets called from two places, which is fine.
-  // 1. OnFinalMessage
-  // 2. In the destructor.
-  bool Uninitialize(HWND window);
+  // Sets the keyboard accelerators needed by the external host
+  void SetAccelerators(HACCEL accel_table, int accel_table_entry_count);
+
+  // This is invoked when the external host reflects back to us a keyboard
+  // message it did not process
+  void ProcessUnhandledAccelerator(const MSG& msg);
+
+  // See TabContents::SetInitialFocus. Called from AutomationProvider.
+  void SetInitialFocus(bool reverse);
+
+  // A helper method that tests whether the given window is an
+  // ExternalTabContainer window
+  static bool IsExternalTabContainer(HWND window);
+
+  // A helper method that retrieves the ExternalTabContainer object that
+  // hosts the given tab window.
+  static ExternalTabContainer* GetContainerForTab(HWND tab_window);
 
   // Overridden from TabContentsDelegate:
   virtual void OpenURLFromTab(TabContents* source,
@@ -93,86 +83,42 @@ class ExternalTabContainer : public TabContentsDelegate,
   virtual bool IsExternalTabContainer() const {
     return true;
   };
-
-  // Creates an ExtensionFunctionDispatcher that has no browser
   virtual ExtensionFunctionDispatcher *CreateExtensionFunctionDispatcher(
       RenderViewHost* render_view_host,
       const std::string& extension_id);
-
   virtual bool TakeFocus(bool reverse);
 
-  // Notification service callback.
+  // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  /////////////////////////////////////////////////////////////////////////////
-  // views::Widget
-  /////////////////////////////////////////////////////////////////////////////
-  virtual void GetBounds(gfx::Rect* out, bool including_frame) const;
-  virtual void SetBounds(const gfx::Rect& bounds);
-  virtual void SetBounds(const gfx::Rect& bounds,
-                         gfx::NativeWindow other_window);
-  virtual void Close();
-  virtual void CloseNow();
-  virtual void Show();
-  virtual void Hide();
-  virtual gfx::NativeView GetNativeView() const;
-  virtual void PaintNow(const gfx::Rect& update_rect);
-  virtual void SetOpacity(unsigned char opacity);
-  virtual views::RootView* GetRootView();
-  virtual Widget* GetRootWidget() const { return NULL; }
-  virtual bool IsVisible() const;
-  virtual bool IsActive() const;
-  virtual bool GetAccelerator(int cmd_id,
-                              views::Accelerator* accelerator) {
-    return false;
-  }
-
-  // views::KeystrokeListener implementation
-  // This method checks whether this key[down|up] message is needed by the
-  // external host. If so, it sends it over to the external host
+  // Overridden from views::KeystrokeListener:
   virtual bool ProcessKeyStroke(HWND window, UINT message, WPARAM wparam,
                                 LPARAM lparam);
 
-  // Sets the keyboard accelerators needed by the external host
-  void SetAccelerators(HACCEL accel_table, int accel_table_entry_count);
-
-  // This is invoked when the external host reflects back to us a keyboard
-  // message it did not process
-  void ProcessUnhandledAccelerator(const MSG& msg);
-
-  // See TabContents::SetInitialFocus
-  void SetInitialFocus(bool reverse);
-
-  // A helper method that tests whether the given window is an
-  // ExternalTabContainer window
-  static bool IsExternalTabContainer(HWND window);
-
-  // A helper method that retrieves the ExternalTabContainer object that
-  // hosts the given tab window.
-  static ExternalTabContainer* GetContainerForTab(HWND tab_window);
-
  protected:
-  LRESULT OnSize(UINT, WPARAM, LPARAM, BOOL& handled);
-  LRESULT OnDestroy(UINT, WPARAM, LPARAM, BOOL& handled);
-  void OnFinalMessage(HWND window);
+  // Overridden from views::WidgetWin:
+  virtual void OnDestroy();
+
+ private:
+  // Unhook the keystroke listener and notify about the closing TabContents.
+  // This function gets called from two places, which is fine.
+  // 1. OnFinalMessage
+  // 2. In the destructor.
+  void Uninitialize(HWND window);
 
- protected:
   TabContents* tab_contents_;
   scoped_refptr<AutomationProvider> automation_;
 
   NotificationRegistrar registrar_;
 
-  // Root view
-  views::RootView root_view_;
   // The accelerator table of the external host.
   HACCEL external_accel_table_;
   unsigned int external_accel_entry_count_;
   // A view to handle focus cycling
   TabContentsContainer* tab_contents_container_;
 
- private:
   int tab_handle_;
   // A failed navigation like a 404 is followed in chrome with a success
   // navigation for the 404 page. We need to ignore the next navigation