Re-do the way browser windows are shown:

- Remove the path from WinMain to the Browser object passing the show_command. For the Browser object, this is a problem since this value isn't portable. For the code in general it involves a lot of ugly wiring. It's completely unnecessary since the value is obtainable via GetStartupInfo.
- Remove show_command plumbing from all over the place (session restore, web app launcher, etc)

Change the way browser windows are constructed:
- The browser constructor now takes just a type and a profile, and simply initializes the object.
- Some configuration that used to be part of the constructor that was only used in one or two use cases (initial bounds, maximized state, web app name) are split into separate setters.
- Window creation is split out into a separate step to be called post configuration.
- Assorted static helper functions added to Browser to make construction of common types easy.
- Remove Browser::Show in favor of BrowserWindow::Show
- Adjust all callers to use the new helpers.

Change the way ChromeViews restores window placement:
- Split restored size determination from restored maximized determination. They are needed by the code at different times. Size restoration happens when the window is constructed and Window::SetInitialBounds is called. Maximized state restoration happens when the window is shown for the first time and SW_SHOWMAXIMIZED or SW_SHOWNORMAL is needed. Thus, replace WindowDelegate::RestoreWindowPosition with WindowDelegate::RestoreWindowBounds and WindowDelegate::RestoreMaximizedState.
- Window::SetInitialBounds calls WindowDelegate::RestoreWindowBounds
- Window::Show calls WindowDelegate::RestoreMaximizedState
- Adjusts all WindowDelegate implementations that override RestoreWindowPosition to implement these new methods instead.
- Move "playback/record" mode window size setting from browser_init to Browser::RestoreWindowBounds.
- Provide a virtual function on Window called GetShowState that determines the default show state to be used when Window::Show is called. For most windows and dialogs this is SW_SHOWNORMAL. AeroGlassFrame/OpaqueFrame (the browser window frames) override this since they're the app's main windows to return the value provided by GetStartupInfo which gives the value from the app shortcut.

http://crbug.com/3557

Review URL: http://codereview.chromium.org/10896

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

9 files changed, 100 insertions, 59 deletions

diff --git a/chrome/browser/views/frame/aero_glass_frame.cc b/chrome/browser/views/frame/aero_glass_frame.cc
index 14170b8..0c5846a 100644
--- a/chrome/browser/views/frame/aero_glass_frame.cc
+++ b/chrome/browser/views/frame/aero_glass_frame.cc
@@ -45,8 +45,8 @@ AeroGlassFrame::AeroGlassFrame(BrowserView* browser_view)
 AeroGlassFrame::~AeroGlassFrame() {
 }
 
-void AeroGlassFrame::Init(const gfx::Rect& bounds) {
-  Window::Init(NULL, bounds);
+void AeroGlassFrame::Init() {
+  Window::Init(NULL, gfx::Rect());
 }
 
 int AeroGlassFrame::GetMinimizeButtonOffset() const {
@@ -91,7 +91,16 @@ views::Window* AeroGlassFrame::GetWindow() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-// AeroGlassFrame, views::ContainerWin implementation:
+// AeroGlassFrame, views::ContainerWin overrides:
+
+bool AeroGlassFrame::AcceleratorPressed(views::Accelerator* accelerator) {
+  return browser_view_->AcceleratorPressed(*accelerator);
+}
+
+bool AeroGlassFrame::GetAccelerator(int cmd_id,
+                                    views::Accelerator* accelerator) {
+  return browser_view_->GetAccelerator(cmd_id, accelerator);
+}
 
 void AeroGlassFrame::OnInitMenuPopup(HMENU menu, UINT position,
                                      BOOL is_system_menu) {
@@ -179,15 +188,10 @@ LRESULT AeroGlassFrame::OnNCHitTest(const CPoint& pt) {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-// AeroGlassFrame, views::ContainerWin overrides:
+// AeroGlassFrame, views::CustomFrameWindow overrides:
 
-bool AeroGlassFrame::AcceleratorPressed(views::Accelerator* accelerator) {
-  return browser_view_->AcceleratorPressed(*accelerator);
-}
-
-bool AeroGlassFrame::GetAccelerator(int cmd_id,
-                                    views::Accelerator* accelerator) {
-  return browser_view_->GetAccelerator(cmd_id, accelerator);
+int AeroGlassFrame::GetShowState() const {
+  return browser_view_->GetShowState();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/views/frame/aero_glass_frame.h b/chrome/browser/views/frame/aero_glass_frame.h
index 0859b46..86df734 100644
--- a/chrome/browser/views/frame/aero_glass_frame.h
+++ b/chrome/browser/views/frame/aero_glass_frame.h
@@ -24,7 +24,7 @@ class AeroGlassFrame : public BrowserFrame,
   explicit AeroGlassFrame(BrowserView* browser_view);
   virtual ~AeroGlassFrame();
 
-  void Init(const gfx::Rect& bounds);
+  void Init();
 
   // Determine the distance of the left edge of the minimize button from the
   // right edge of the window. Used in our Non-Client View's Layout.
@@ -38,12 +38,10 @@ class AeroGlassFrame : public BrowserFrame,
   virtual void UpdateThrobber(bool running);
   virtual views::Window* GetWindow();
 
+ protected:
   // Overridden from views::ContainerWin:
   virtual bool AcceleratorPressed(views::Accelerator* accelerator);
   virtual bool GetAccelerator(int cmd_id, views::Accelerator* accelerator);
-
- protected:
-  // Overridden from views::ContainerWin:
   virtual void OnInitMenuPopup(HMENU menu, UINT position, BOOL is_system_menu);
   virtual void OnEndSession(BOOL ending, UINT logoff);
   virtual LRESULT OnMouseActivate(HWND window,
@@ -55,6 +53,9 @@ class AeroGlassFrame : public BrowserFrame,
   virtual LRESULT OnNCCalcSize(BOOL mode, LPARAM l_param);
   virtual LRESULT OnNCHitTest(const CPoint& pt);
 
+  // Overridden from views::CustomFrameWindow:
+  virtual int GetShowState() const;
+
  private:
   // Updates the DWM with the frame bounds.
   void UpdateDWMFrame();
diff --git a/chrome/browser/views/frame/browser_frame.h b/chrome/browser/views/frame/browser_frame.h
index 7aa62bc..c0b18af 100644
--- a/chrome/browser/views/frame/browser_frame.h
+++ b/chrome/browser/views/frame/browser_frame.h
@@ -56,9 +56,7 @@ class BrowserFrame {
   // Creates a BrowserFrame instance for the specified FrameType and
   // BrowserView.
   static BrowserFrame* CreateForBrowserView(FrameType type,
-                                            BrowserView* browser_view,
-                                            const gfx::Rect& bounds,
-                                            int show_command);
+                                            BrowserView* browser_view);
 
 };
 
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 41088fe..17127ba 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -123,6 +123,14 @@ BrowserView::~BrowserView() {
   ticker_.UnregisterTickHandler(&hung_window_detector_);
 }
 
+int BrowserView::GetShowState() const {
+  STARTUPINFO si = {0};
+  si.cb = sizeof(si);
+  si.dwFlags = STARTF_USESHOWWINDOW;
+  GetStartupInfo(&si);
+  return si.wShowWindow;
+}
+
 void BrowserView::WindowMoved() {
   // Cancel any tabstrip animations, some of them may be invalidated by the
   // window being repositioned.
@@ -320,8 +328,31 @@ void BrowserView::Init() {
   InitSystemMenu();
 }
 
-void BrowserView::Show(int command, bool adjust_to_fit) {
-  frame_->GetWindow()->Show(command);
+void BrowserView::Show() {
+  // If the window is already visible, just activate it.
+  if (frame_->GetWindow()->IsVisible()) {
+    frame_->GetWindow()->Activate();
+    return;
+  }
+
+  // Setting the focus doesn't work when the window is invisible, so any focus
+  // initialization that happened before this will be lost.
+  //
+  // We really "should" restore the focus whenever the window becomes unhidden,
+  // but I think initializing is the only time where this can happen where
+  // there is some focus change we need to pick up, and this is easier than
+  // plumbing through an un-hide message all the way from the frame.
+  //
+  // If we do find there are cases where we need to restore the focus on show,
+  // that should be added and this should be removed.
+  TabContents* selected_tab_contents = GetSelectedTabContents();
+  if (selected_tab_contents)
+    selected_tab_contents->RestoreFocus();
+
+  frame_->GetWindow()->Show();
+  int show_state = frame_->GetWindow()->GetShowState();
+  if (show_state == SW_SHOWNORMAL || show_state == SW_SHOWMAXIMIZED)
+    frame_->GetWindow()->Activate();
 }
 
 void BrowserView::Close() {
@@ -378,7 +409,7 @@ void BrowserView::ValidateThrobber() {
   }
 }
 
-gfx::Rect BrowserView::GetNormalBounds() {
+gfx::Rect BrowserView::GetNormalBounds() const {
   WINDOWPLACEMENT wp;
   wp.length = sizeof(wp);
   const bool ret = !!GetWindowPlacement(frame_->GetWindow()->GetHWND(), &wp);
@@ -562,18 +593,13 @@ bool BrowserView::ExecuteWindowsCommand(int command_id) {
   return false;
 }
 
-void BrowserView::SaveWindowPosition(const CRect& bounds,
-                                     bool maximized,
-                                     bool always_on_top) {
-  browser_->SaveWindowPosition(gfx::Rect(bounds), maximized);
+void BrowserView::SaveWindowPlacement(const gfx::Rect& bounds,
+                                      bool maximized,
+                                      bool always_on_top) {
+  browser_->SaveWindowPlacement(bounds, maximized);
 }
 
-bool BrowserView::RestoreWindowPosition(CRect* bounds,
-                                        bool* maximized,
-                                        bool* always_on_top) {
-  DCHECK(bounds && maximized && always_on_top);
-  *always_on_top = false;
-
+bool BrowserView::GetSavedWindowBounds(gfx::Rect* bounds) const {
   if (browser_->type() == BrowserType::BROWSER) {
     // We are a popup window. The value passed in |bounds| represents two
     // pieces of information:
@@ -587,12 +613,12 @@ bool BrowserView::RestoreWindowPosition(CRect* bounds,
       // its desired height, since the toolbar is considered part of the
       // window's client area as far as GetWindowBoundsForClientBounds is
       // concerned...
-      bounds->bottom += toolbar_->GetPreferredSize().height();
+      bounds->set_height(
+          bounds->height() + toolbar_->GetPreferredSize().height());
     }
 
-    gfx::Rect window_rect =
-        frame_->GetWindowBoundsForClientBounds(gfx::Rect(*bounds));
-    window_rect.set_origin(gfx::Point(bounds->left, bounds->top));
+    gfx::Rect window_rect = frame_->GetWindowBoundsForClientBounds(*bounds);
+    window_rect.set_origin(gfx::Point(bounds->x(), bounds->y()));
 
     // When we are given x/y coordinates of 0 on a created popup window,
     // assume none were given by the window.open() command.
@@ -603,13 +629,9 @@ bool BrowserView::RestoreWindowPosition(CRect* bounds,
       window_rect.set_origin(origin);
     }
 
-    *bounds = window_rect.ToRECT();
-    *maximized = false;
+    *bounds = window_rect;
   } else {
-    // TODO(beng): (http://b/1317622) Make these functions take gfx::Rects.
-    gfx::Rect b(*bounds);
-    browser_->RestoreWindowPosition(&b, maximized);
-    *bounds = b.ToRECT();
+    *bounds = browser_->GetSavedWindowBounds();
   }
 
   // We return true because we can _always_ locate reasonable bounds using the
@@ -619,6 +641,11 @@ bool BrowserView::RestoreWindowPosition(CRect* bounds,
   return true;
 }
 
+bool BrowserView::GetSavedMaximizedState(bool* maximized) const {
+  *maximized = browser_->GetSavedMaximizedState();
+  return true;
+}
+
 void BrowserView::WindowClosing() {
 }
 
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index 73ba719..549cbd6 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -43,6 +43,10 @@ class BrowserView : public BrowserWindow,
 
   void set_frame(BrowserFrame* frame) { frame_ = frame; }
 
+  // Returns the show flag that should be used to show the frame containing
+  // this view.
+  int GetShowState() const;
+
   // Called by the frame to notify the BrowserView that it was moved, and that
   // any dependent popup windows should be repositioned.
   void WindowMoved();
@@ -137,7 +141,7 @@ class BrowserView : public BrowserWindow,
 
   // Overridden from BrowserWindow:
   virtual void Init();
-  virtual void Show(int command, bool adjust_to_fit);
+  virtual void Show();
   virtual void Close();
   virtual void Activate();
   virtual void FlashFrame();
@@ -147,7 +151,7 @@ class BrowserView : public BrowserWindow,
   virtual void SelectedTabToolbarSizeChanged(bool is_animating);
   virtual void UpdateTitleBar();
   virtual void ValidateThrobber();
-  virtual gfx::Rect GetNormalBounds();
+  virtual gfx::Rect GetNormalBounds() const;
   virtual bool IsMaximized();
   virtual ToolbarStarToggle* GetStarButton() const;
   virtual LocationBarView* GetLocationBarView() const;
@@ -182,12 +186,11 @@ class BrowserView : public BrowserWindow,
   virtual SkBitmap GetWindowIcon();
   virtual bool ShouldShowWindowIcon() const;
   virtual bool ExecuteWindowsCommand(int command_id);
-  virtual void SaveWindowPosition(const CRect& bounds,
-                                  bool maximized,
-                                  bool always_on_top);
-  virtual bool RestoreWindowPosition(CRect* bounds,
-                                     bool* maximized,
-                                     bool* always_on_top);
+  virtual void SaveWindowPlacement(const gfx::Rect& bounds,
+                                   bool maximized,
+                                   bool always_on_top);
+  virtual bool GetSavedWindowBounds(gfx::Rect* bounds) const;
+  virtual bool GetSavedMaximizedState(bool* maximized) const;
   virtual void WindowClosing();
   virtual views::View* GetContentsView();
   virtual views::ClientView* CreateClientView(views::Window* window);
diff --git a/chrome/browser/views/frame/browser_window_factory.cc b/chrome/browser/views/frame/browser_window_factory.cc
index 69c61bb..59a8d9d 100644
--- a/chrome/browser/views/frame/browser_window_factory.cc
+++ b/chrome/browser/views/frame/browser_window_factory.cc
@@ -17,12 +17,10 @@
 // BrowserWindow, public:
 
 // static
-BrowserWindow* BrowserWindow::CreateBrowserWindow(Browser* browser,
-                                                  const gfx::Rect& bounds,
-                                                  int show_command) {
+BrowserWindow* BrowserWindow::CreateBrowserWindow(Browser* browser) {
   BrowserView* browser_view = new BrowserView(browser);
   BrowserFrame::CreateForBrowserView(BrowserFrame::GetActiveFrameType(),
-                                     browser_view, bounds, show_command);
+                                     browser_view);
   return browser_view;
 }
 
@@ -37,16 +35,14 @@ BrowserFrame::FrameType BrowserFrame::GetActiveFrameType() {
 
 // static
 BrowserFrame* BrowserFrame::CreateForBrowserView(BrowserFrame::FrameType type,
-                                                 BrowserView* browser_view,
-                                                 const gfx::Rect& bounds,
-                                                 int show_command) {
+                                                 BrowserView* browser_view) {
   if (type == FRAMETYPE_OPAQUE) {
     OpaqueFrame* frame = new OpaqueFrame(browser_view);
-    frame->Init(NULL, bounds);
+    frame->Init();
     return frame;
   } else if (type == FRAMETYPE_AERO_GLASS) {
     AeroGlassFrame* frame = new AeroGlassFrame(browser_view);
-    frame->Init(bounds);
+    frame->Init();
     return frame;
   }
   NOTREACHED() << "Unsupported frame type";
diff --git a/chrome/browser/views/frame/opaque_frame.cc b/chrome/browser/views/frame/opaque_frame.cc
index 1d1ce8f..3158af7 100644
--- a/chrome/browser/views/frame/opaque_frame.cc
+++ b/chrome/browser/views/frame/opaque_frame.cc
@@ -23,6 +23,10 @@ OpaqueFrame::OpaqueFrame(BrowserView* browser_view)
 OpaqueFrame::~OpaqueFrame() {
 }
 
+void OpaqueFrame::Init() {
+  CustomFrameWindow::Init(NULL, gfx::Rect());
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueFrame, BrowserFrame implementation:
 
@@ -62,6 +66,10 @@ void OpaqueFrame::UpdateWindowIcon() {
   GetOpaqueNonClientView()->UpdateWindowIcon();
 }
 
+int OpaqueFrame::GetShowState() const {
+  return browser_view_->GetShowState();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueFrame, views::ContainerWin overrides:
 
diff --git a/chrome/browser/views/frame/opaque_frame.h b/chrome/browser/views/frame/opaque_frame.h
index 0a6d1ca..f28d782 100644
--- a/chrome/browser/views/frame/opaque_frame.h
+++ b/chrome/browser/views/frame/opaque_frame.h
@@ -29,6 +29,8 @@ class OpaqueFrame : public BrowserFrame,
   explicit OpaqueFrame(BrowserView* browser_view);
   virtual ~OpaqueFrame();
 
+  void Init();
+
  protected:
   // Overridden from BrowserFrame:
   virtual gfx::Rect GetWindowBoundsForClientBounds(
@@ -40,6 +42,7 @@ class OpaqueFrame : public BrowserFrame,
 
   // Overridden from views::CustomFrameWindow:
   virtual void UpdateWindowIcon();
+  virtual int GetShowState() const;
 
   // Overridden from views::ContainerWin:
   virtual bool AcceleratorPressed(views::Accelerator* accelerator);
diff --git a/chrome/browser/views/frame/opaque_non_client_view.cc b/chrome/browser/views/frame/opaque_non_client_view.cc
index 877b8b5..460a1f8 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.cc
+++ b/chrome/browser/views/frame/opaque_non_client_view.cc
@@ -467,7 +467,8 @@ gfx::Rect OpaqueNonClientView::GetBoundsForTabStrip(TabStrip* tabstrip) {
   int tabstrip_width = minimize_button_->x() - tabstrip_x;
   if (frame_->IsMaximized())
     tabstrip_width -= kNewTabIconWindowControlsSpacing;
-  return gfx::Rect(tabstrip_x, 0, tabstrip_width, tabstrip_height);
+  return gfx::Rect(tabstrip_x, 0, std::max(0, tabstrip_width),
+                   tabstrip_height);
 }
 
 void OpaqueNonClientView::UpdateWindowIcon() {