Re-land this change: http://codereview.chromium.org/10967/show

Now with two improvements:
- no longer create WebContentsViews as WS_VISIBLE, to prevent them from showing up in the taskbar
- make sure overridden CreateView in NativeUIContents and StatusView have the correct signature.


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

19 files changed, 44 insertions, 71 deletions

diff --git a/chrome/browser/back_forward_menu_model_unittest.cc b/chrome/browser/back_forward_menu_model_unittest.cc
index a1d9e47..e194480 100644
--- a/chrome/browser/back_forward_menu_model_unittest.cc
+++ b/chrome/browser/back_forward_menu_model_unittest.cc
@@ -95,7 +95,7 @@ class BackFwdMenuModelTest : public testing::Test {
  protected:
   TabContents* CreateTabContents() {
     TabContents* contents = new BackFwdMenuModelTestTabContents;
-    contents->CreateView(::GetDesktopWindow(), gfx::Rect());
+    contents->CreateView();
     contents->SetupController(profile_);
     return contents;
   }
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 156503f..49333b6 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1234,9 +1234,7 @@ TabContents* Browser::CreateTabContentsForURL(
   TabContentsType type = TabContents::TypeForURL(&real_url);
   DCHECK(type != TAB_CONTENTS_UNKNOWN_TYPE);
 
-  HWND parent_hwnd = reinterpret_cast<HWND>(window_->GetNativeHandle());
-  TabContents* contents = TabContents::CreateWithType(type, parent_hwnd,
-                                                      profile, instance);
+  TabContents* contents = TabContents::CreateWithType(type, profile, instance);
   contents->SetupController(profile);
 
   if (!defer_load) {
@@ -1264,9 +1262,7 @@ void Browser::DuplicateContentsAt(int index) {
   if (type_ == TYPE_NORMAL) {
     // If this is a tabbed browser, just create a duplicate tab inside the same
     // window next to the tab being duplicated.
-    HWND parent_hwnd = reinterpret_cast<HWND>(window_->GetNativeHandle());
-    new_contents = contents->controller()->Clone(
-        parent_hwnd)->active_contents();
+    new_contents = contents->controller()->Clone()->active_contents();
     // If you duplicate a tab that is not selected, we need to make sure to
     // select the tab being duplicated so that DetermineInsertionIndex returns
     // the right index (if tab 5 is selected and we right-click tab 1 we want
@@ -1288,9 +1284,8 @@ void Browser::DuplicateContentsAt(int index) {
     browser->window()->Show();
 
     // The page transition below is only for the purpose of inserting the tab.
-    HWND parent_hwnd = reinterpret_cast<HWND>(window_->GetNativeHandle());
     new_contents = browser->AddTabWithNavigationController(
-        contents->controller()->Clone(parent_hwnd),
+        contents->controller()->Clone(),
         PageTransition::LINK);
   }
 
@@ -2189,9 +2184,7 @@ NavigationController* Browser::BuildRestoredNavigationController(
 
     // Create a NavigationController. This constructor creates the appropriate
     // set of TabContents.
-    HWND parent_hwnd = reinterpret_cast<HWND>(window_->GetNativeHandle());
-    return new NavigationController(
-        profile_, navigations, selected_navigation, parent_hwnd);
+    return new NavigationController(profile_, navigations, selected_navigation);
   } else {
     // No navigations. Create a tab with about:blank.
     TabContents* contents =
diff --git a/chrome/browser/debugger/debugger_view.cc b/chrome/browser/debugger/debugger_view.cc
index 127c987..169be19 100644
--- a/chrome/browser/debugger/debugger_view.cc
+++ b/chrome/browser/debugger/debugger_view.cc
@@ -92,8 +92,8 @@ void DebuggerView::OnInit() {
   // We can't create the WebContents until we've actually been put into a real
   // view hierarchy somewhere.
   Profile* profile = BrowserList::GetLastActive()->profile();
-  TabContents* tc = TabContents::CreateWithType(TAB_CONTENTS_DEBUGGER, 
-      ::GetDesktopWindow(), profile, NULL);
+  TabContents* tc = TabContents::CreateWithType(TAB_CONTENTS_DEBUGGER, profile,
+                                                NULL);
   web_contents_ = tc->AsWebContents();
   web_contents_->SetupController(profile);
   web_contents_->set_delegate(this);
diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc
index ea1f7e3..62aec65 100644
--- a/chrome/browser/external_tab_container.cc
+++ b/chrome/browser/external_tab_container.cc
@@ -57,8 +57,7 @@ bool ExternalTabContainer::Init(Profile* profile) {
       views::FocusManager::CreateFocusManager(m_hWnd, GetRootView());
   DCHECK(focus_manager);
   focus_manager->AddKeystrokeListener(this);
-  tab_contents_ = TabContents::CreateWithType(TAB_CONTENTS_WEB,
-                                              m_hWnd, profile, NULL);
+  tab_contents_ = TabContents::CreateWithType(TAB_CONTENTS_WEB, profile, NULL);
   if (!tab_contents_) {
     NOTREACHED();
     DestroyWindow();
diff --git a/chrome/browser/native_ui_contents.cc b/chrome/browser/native_ui_contents.cc
index 840bb91..0c87e8b 100644
--- a/chrome/browser/native_ui_contents.cc
+++ b/chrome/browser/native_ui_contents.cc
@@ -148,10 +148,9 @@ NativeUIContents::~NativeUIContents() {
                                        path_to_native_uis_.end());
 }
 
-void NativeUIContents::CreateView(HWND parent_hwnd,
-                                  const gfx::Rect& initial_bounds) {
+void NativeUIContents::CreateView() {
   set_delete_on_destroy(false);
-  ContainerWin::Init(parent_hwnd, initial_bounds, false);
+  ContainerWin::Init(GetDesktopWindow(), gfx::Rect(), false);
 }
 
 LRESULT NativeUIContents::OnCreate(LPCREATESTRUCT create_struct) {
diff --git a/chrome/browser/native_ui_contents.h b/chrome/browser/native_ui_contents.h
index 58e8283..6b15ec5 100644
--- a/chrome/browser/native_ui_contents.h
+++ b/chrome/browser/native_ui_contents.h
@@ -37,7 +37,7 @@ class NativeUIContents : public TabContents,
  public:
   explicit NativeUIContents(Profile* profile);
 
-  virtual void CreateView(HWND parent_hwnd, const gfx::Rect& initial_bounds);
+  virtual void CreateView();
   virtual HWND GetContainerHWND() const { return GetHWND(); }
   virtual void GetContainerBounds(gfx::Rect *out) const;
 
diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc
index c646b4a..18e270a 100644
--- a/chrome/browser/navigation_controller.cc
+++ b/chrome/browser/navigation_controller.cc
@@ -184,8 +184,7 @@ NavigationController::NavigationController(TabContents* contents,
 NavigationController::NavigationController(
     Profile* profile,
     const std::vector<TabNavigation>& navigations,
-    int selected_navigation,
-    HWND parent)
+    int selected_navigation)
     : profile_(profile),
       pending_entry_(NULL),
       last_committed_entry_index_(-1),
@@ -204,7 +203,7 @@ NavigationController::NavigationController(
   CreateNavigationEntriesFromTabNavigations(navigations, &entries_);
 
   // And finish the restore.
-  FinishRestore(parent, selected_navigation);
+  FinishRestore(selected_navigation);
 }
 
 NavigationController::~NavigationController() {
@@ -1024,10 +1023,7 @@ void NavigationController::NavigateToPendingEntry(bool reload) {
   if (from_contents && from_contents->type() != pending_entry_->tab_type())
     from_contents->set_is_active(false);
 
-  HWND parent =
-      from_contents ? GetParent(from_contents->GetContainerHWND()) : 0;
-  TabContents* contents =
-      GetTabContentsCreateIfNecessary(parent, *pending_entry_);
+  TabContents* contents = GetTabContentsCreateIfNecessary(*pending_entry_);
 
   contents->set_is_active(true);
   active_contents_ = contents;
@@ -1059,11 +1055,10 @@ void NavigationController::NotifyNavigationEntryCommitted(
 }
 
 TabContents* NavigationController::GetTabContentsCreateIfNecessary(
-    HWND parent,
     const NavigationEntry& entry) {
   TabContents* contents = GetTabContents(entry.tab_type());
   if (!contents) {
-    contents = TabContents::CreateWithType(entry.tab_type(), parent, profile_,
+    contents = TabContents::CreateWithType(entry.tab_type(), profile_,
                                            entry.site_instance());
     if (!contents->AsWebContents()) {
       // Update the max page id, otherwise the newly created TabContents may
@@ -1143,7 +1138,7 @@ void NavigationController::NotifyEntryChanged(const NavigationEntry* entry,
                                          Details<EntryChangedDetails>(&det));
 }
 
-NavigationController* NavigationController::Clone(HWND parent_hwnd) {
+NavigationController* NavigationController::Clone() {
   NavigationController* nc = new NavigationController(NULL, profile_);
 
   if (GetEntryCount() == 0)
@@ -1157,7 +1152,7 @@ NavigationController* NavigationController::Clone(HWND parent_hwnd) {
         new NavigationEntry(*GetEntryAtIndex(i))));
   }
 
-  nc->FinishRestore(parent_hwnd, last_committed_entry_index_);
+  nc->FinishRestore(last_committed_entry_index_);
 
   return nc;
 }
@@ -1203,7 +1198,7 @@ void NavigationController::CancelTabContentsCollection(TabContentsType t) {
   }
 }
 
-void NavigationController::FinishRestore(HWND parent_hwnd, int selected_index) {
+void NavigationController::FinishRestore(int selected_index) {
   DCHECK(selected_index >= 0 && selected_index < GetEntryCount());
   ConfigureEntriesForRestore(&entries_);
 
@@ -1212,8 +1207,7 @@ void NavigationController::FinishRestore(HWND parent_hwnd, int selected_index) {
   last_committed_entry_index_ = selected_index;
 
   // Callers assume we have an active_contents after restoring, so set it now.
-  active_contents_ =
-      GetTabContentsCreateIfNecessary(parent_hwnd, *entries_[selected_index]);
+  active_contents_ = GetTabContentsCreateIfNecessary(*entries_[selected_index]);
 }
 
 void NavigationController::DiscardNonCommittedEntriesInternal() {
diff --git a/chrome/browser/navigation_controller.h b/chrome/browser/navigation_controller.h
index cd156b5..74e3049 100644
--- a/chrome/browser/navigation_controller.h
+++ b/chrome/browser/navigation_controller.h
@@ -128,8 +128,7 @@ class NavigationController {
   NavigationController(
       Profile* profile,
       const std::vector<TabNavigation>& navigations,
-      int selected_navigation,
-      HWND parent);
+      int selected_navigation);
   ~NavigationController();
 
   // Begin the destruction sequence for this NavigationController and all its
@@ -141,8 +140,8 @@ class NavigationController {
   void Destroy();
 
   // Clone the receiving navigation controller. Only the active tab contents is
-  // duplicated. It is created as a child of the provided HWND.
-  NavigationController* Clone(HWND hwnd);
+  // duplicated.
+  NavigationController* Clone();
 
   // Returns the profile for this controller. It can never be NULL.
   Profile* profile() const {
@@ -434,8 +433,7 @@ class NavigationController {
   // Returns the TabContents for the |entry|'s type. If the TabContents
   // doesn't yet exist, it is created. If a new TabContents is created, its
   // parent is |parent|.  Becomes part of |entry|'s SiteInstance.
-  TabContents* GetTabContentsCreateIfNecessary(HWND parent,
-                                               const NavigationEntry& entry);
+  TabContents* GetTabContentsCreateIfNecessary(const NavigationEntry& entry);
 
   // Register the provided tab contents. This tab contents will be owned
   // and deleted by this navigation controller
@@ -463,7 +461,7 @@ class NavigationController {
   // Invoked after session/tab restore or cloning a tab. Resets the transition
   // type of the entries, updates the max page id and creates the active
   // contents.
-  void FinishRestore(HWND parent_hwnd, int selected_index);
+  void FinishRestore(int selected_index);
 
   // Inserts an entry after the current position, removing all entries after it.
   // The new entry will become the active one.
diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc
index 5c43b1b..98e9ad7 100644
--- a/chrome/browser/navigation_controller_unittest.cc
+++ b/chrome/browser/navigation_controller_unittest.cc
@@ -51,7 +51,7 @@ class NavigationControllerTest : public testing::Test,
 
     contents = new TestTabContents(type1());
     contents->set_delegate(this);
-    contents->CreateView(::GetDesktopWindow(), gfx::Rect());
+    contents->CreateView();
     contents->SetupController(profile);
   }
 
@@ -1232,7 +1232,7 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
   navigations.push_back(TabNavigation(0, url, GURL(), L"Title", "state",
                                       PageTransition::LINK));
   NavigationController* controller =
-      new NavigationController(profile, navigations, 0, NULL);
+      new NavigationController(profile, navigations, 0);
   controller->GoToIndex(0);
 
   // We should now have one entry, and it should be "pending".
diff --git a/chrome/browser/status_view.cc b/chrome/browser/status_view.cc
index 9039daa..30395fc 100644
--- a/chrome/browser/status_view.cc
+++ b/chrome/browser/status_view.cc
@@ -16,9 +16,8 @@ StatusView::~StatusView() {
     delete buttons_[i].button;
 }
 
-void StatusView::CreateView(HWND parent_hwnd,
-                            const gfx::Rect& initial_bounds) {
-  Create(parent_hwnd);
+void StatusView::CreateView() {
+  Create(GetDesktopWindow());
 }
 
 LRESULT StatusView::OnCreate(LPCREATESTRUCT create_struct) {
diff --git a/chrome/browser/status_view.h b/chrome/browser/status_view.h
index 7cdeec8..7184ba5 100644
--- a/chrome/browser/status_view.h
+++ b/chrome/browser/status_view.h
@@ -32,7 +32,7 @@ class StatusView : public TabContents,
     MSG_WM_SIZE(OnSize)
   END_MSG_MAP()
 
-  virtual void CreateView(HWND parent_hwnd, const gfx::Rect& initial_bounds);
+  virtual void CreateView();
   virtual HWND GetContainerHWND() const { return m_hWnd; }
 
   // Derived classes should implement the following functions
diff --git a/chrome/browser/tab_contents.h b/chrome/browser/tab_contents.h
index 6f7f592..ebbc47b 100644
--- a/chrome/browser/tab_contents.h
+++ b/chrome/browser/tab_contents.h
@@ -75,7 +75,6 @@ class TabContents : public PageNavigator {
   // Creates a new TabContents of the given type.  Will reuse the given
   // instance's renderer, if it is not null.
   static TabContents* CreateWithType(TabContentsType type,
-                                     HWND parent,
                                      Profile* profile,
                                      SiteInstance* instance);
 
@@ -340,7 +339,7 @@ class TabContents : public PageNavigator {
 
   // Tell the subclass to set up the view (e.g. create the container HWND if
   // applicable) and any other create-time setup.
-  virtual void CreateView(HWND parent_hwnd, const gfx::Rect& initial_bounds) {}
+  virtual void CreateView() {}
 
   // Returns the HWND associated with this TabContents. Outside of automation
   // in the context of the UI, this is required to be implemented.
diff --git a/chrome/browser/tab_contents_factory.cc b/chrome/browser/tab_contents_factory.cc
index 19b40569..2488222 100644
--- a/chrome/browser/tab_contents_factory.cc
+++ b/chrome/browser/tab_contents_factory.cc
@@ -38,7 +38,6 @@ TabContentsType TabContentsFactory::NextUnusedType() {
 
 /*static*/
 TabContents* TabContents::CreateWithType(TabContentsType type,
-                                         HWND parent,
                                          Profile* profile,
                                          SiteInstance* instance) {
   TabContents* contents;
@@ -89,7 +88,7 @@ TabContents* TabContents::CreateWithType(TabContentsType type,
   }
 
   if (contents)
-    contents->CreateView(parent, gfx::Rect());
+    contents->CreateView();
 
   return contents;
 }
diff --git a/chrome/browser/views/dom_view.cc b/chrome/browser/views/dom_view.cc
index ebb63e3..ba74fac 100644
--- a/chrome/browser/views/dom_view.cc
+++ b/chrome/browser/views/dom_view.cc
@@ -28,8 +28,8 @@ bool DOMView::Init(Profile* profile, SiteInstance* instance) {
   // a DOMUIHostFactory rather than TabContentsFactory, because DOMView's
   // should only be associated with instances of DOMUIHost.
   TabContentsType type = TabContents::TypeForURL(&contents_);
-  TabContents* tab_contents =  TabContents::CreateWithType(type,
-      GetContainer()->GetHWND(), profile, instance);
+  TabContents* tab_contents =  TabContents::CreateWithType(type, profile,
+                                                           instance);
   host_ = tab_contents->AsDOMUIHost();
   DCHECK(host_);
 
diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc
index dda65e1..a8fe7db 100644
--- a/chrome/browser/web_contents.cc
+++ b/chrome/browser/web_contents.cc
@@ -481,9 +481,8 @@ void WebContents::SetDownloadShelfVisible(bool visible) {
 }
 
 // Stupid view pass-throughs
-void WebContents::CreateView(HWND parent_hwnd,
-                             const gfx::Rect& initial_bounds) {
-  view_->CreateView(parent_hwnd, initial_bounds);
+void WebContents::CreateView() {
+  view_->CreateView();
 }
 HWND WebContents::GetContainerHWND() const {
   return view_->GetContainerHWND();
diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h
index 8b59d6b..71894de 100644
--- a/chrome/browser/web_contents.h
+++ b/chrome/browser/web_contents.h
@@ -103,7 +103,7 @@ class WebContents : public TabContents,
   // Retarded pass-throughs to the view.
   // TODO(brettw) fix this, tab contents shouldn't have these methods, probably
   // it should be killed altogether.
-  virtual void CreateView(HWND parent_hwnd, const gfx::Rect& initial_bounds);
+  virtual void CreateView();
   virtual HWND GetContainerHWND() const;
   virtual HWND GetContentHWND();
   virtual void GetContainerBounds(gfx::Rect *out) const;
diff --git a/chrome/browser/web_contents_view.h b/chrome/browser/web_contents_view.h
index e6b24c2..c253a7d 100644
--- a/chrome/browser/web_contents_view.h
+++ b/chrome/browser/web_contents_view.h
@@ -38,8 +38,7 @@ class WebContentsView : public RenderViewHostDelegate::View {
 
   virtual WebContents* GetWebContents() = 0;
 
-  virtual void CreateView(HWND parent_hwnd,
-                          const gfx::Rect& initial_bounds) = 0;
+  virtual void CreateView() = 0;
 
   // Sets up the View that holds the rendered web page, receives messages for
   // it and contains page plugins.
diff --git a/chrome/browser/web_contents_view_win.cc b/chrome/browser/web_contents_view_win.cc
index 90da9cd..30665ea 100644
--- a/chrome/browser/web_contents_view_win.cc
+++ b/chrome/browser/web_contents_view_win.cc
@@ -52,10 +52,12 @@ WebContents* WebContentsViewWin::GetWebContents() {
   return web_contents_;
 }
 
-void WebContentsViewWin::CreateView(HWND parent_hwnd,
-                                    const gfx::Rect& initial_bounds) {
+void WebContentsViewWin::CreateView() {
   set_delete_on_destroy(false);
-  ContainerWin::Init(parent_hwnd, initial_bounds, false);
+  // Since we create these windows parented to the desktop window initially, we
+  // don't want to create them initially visible.
+  set_window_style(WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS);
+  ContainerWin::Init(GetDesktopWindow(), gfx::Rect(), false);
 
   // Remove the root view drop target so we can register our own.
   RevokeDragDrop(GetHWND());
@@ -365,13 +367,7 @@ WebContents* WebContentsViewWin::CreateNewWindowInternal(
   new_contents->SetupController(web_contents_->profile());
   WebContentsView* new_view = new_contents->view();
 
-  // TODO(beng)
-  // The intention here is to create background tabs, which should ideally
-  // be parented to NULL. However doing that causes the corresponding view
-  // container windows to show up as overlapped windows, which causes
-  // other issues. We should fix this.
-  HWND new_view_parent_window = ::GetAncestor(GetContainerHWND(), GA_ROOT);
-  new_view->CreateView(new_view_parent_window, gfx::Rect());
+  new_view->CreateView();
 
   // TODO(brettw) it seems bogus that we have to call this function on the
   // newly created object and give it one of its own member variables.
diff --git a/chrome/browser/web_contents_view_win.h b/chrome/browser/web_contents_view_win.h
index ca03829..ee28e87 100644
--- a/chrome/browser/web_contents_view_win.h
+++ b/chrome/browser/web_contents_view_win.h
@@ -31,8 +31,7 @@ class WebContentsViewWin : public WebContentsView,
   // WebContentsView implementation --------------------------------------------
 
   virtual WebContents* GetWebContents();
-  virtual void CreateView(HWND parent_hwnd,
-                          const gfx::Rect& initial_bounds);
+  virtual void CreateView();
   virtual RenderWidgetHostViewWin* CreateViewForWidget(
       RenderWidgetHost* render_widget_host);
   virtual HWND GetContainerHWND() const;