Fixed view deletion on language switch.

Enhanced screen bounds determination.

BUGS=none
TEST=Launch with --login-manager --login-screen-size=1024,600 and verify
that wizard screen is centered within the background window and the window
is centered within the monitor. Launch with --login-manager
--login-screen=network and change language. Verify that nothing craches with
segmentation fault.

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

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

6 files changed, 72 insertions, 45 deletions

diff --git a/chrome/browser/chromeos/login/background_view.cc b/chrome/browser/chromeos/login/background_view.cc
index c4bf995..e935b2c 100644
--- a/chrome/browser/chromeos/login/background_view.cc
+++ b/chrome/browser/chromeos/login/background_view.cc
@@ -27,15 +27,17 @@ BackgroundView::BackgroundView() : status_area_(NULL) {
   InitStatusArea();
 }
 
-static gfx::Rect CalculateWindowBounds(const gfx::Rect& bounds) {
-  if (!bounds.IsEmpty())
-    return bounds;
+void BackgroundView::Init() {
+  InitStatusArea();
+}
 
-  return views::Screen::GetMonitorWorkAreaNearestWindow(NULL);
+void BackgroundView::Teardown() {
+  RemoveAllChildViews(true);
+  status_area_ = NULL;
 }
 
 static void ResetXCursor() {
-  // TOOD(sky): nuke this once new window manager is in place.
+  // TODO(sky): nuke this once new window manager is in place.
   // This gets rid of the ugly X default cursor.
   Display* display = x11_util::GetXDisplay();
   Cursor cursor = XCreateFontCursor(display, XC_left_ptr);
@@ -53,7 +55,7 @@ views::Widget* BackgroundView::CreateWindowContainingView(
 
   views::WidgetGtk* window =
       new views::WidgetGtk(views::WidgetGtk::TYPE_WINDOW);
-  window->Init(NULL, CalculateWindowBounds(bounds));
+  window->Init(NULL, bounds);
   chromeos::WmIpc::instance()->SetWindowType(
       window->GetNativeView(),
       chromeos::WmIpc::WINDOW_TYPE_LOGIN_BACKGROUND,
@@ -68,12 +70,6 @@ views::Widget* BackgroundView::CreateWindowContainingView(
   return window;
 }
 
-void BackgroundView::RecreateStatusArea() {
-  RemoveAllChildViews(true);
-  status_area_ = NULL;
-  InitStatusArea();
-}
-
 void BackgroundView::Layout() {
   int right_top_padding =
       chromeos::BorderDefinition::kWizardBorder.padding +
@@ -109,6 +105,7 @@ bool BackgroundView::IsButtonVisible(const views::View* button_view) const {
 }
 
 void BackgroundView::InitStatusArea() {
+  DCHECK(status_area_ == NULL);
   status_area_ = new StatusAreaView(this);
   status_area_->Init();
   AddChildView(status_area_);
diff --git a/chrome/browser/chromeos/login/background_view.h b/chrome/browser/chromeos/login/background_view.h
index b95391e..995bb58 100644
--- a/chrome/browser/chromeos/login/background_view.h
+++ b/chrome/browser/chromeos/login/background_view.h
@@ -22,15 +22,17 @@ class BackgroundView : public views::View, public StatusAreaHost {
  public:
   BackgroundView();
 
+  // Initializes child views of background view.
+  void Init();
+  // Deletes child views of background view.
+  void Teardown();
+
   // Creates a window containing an instance of WizardContentsView as the root
   // view. The caller is responsible for showing (and closing) the returned
   // widget. The BackgroundView is set in |view|.
   static views::Widget* CreateWindowContainingView(const gfx::Rect& bounds,
                                                    BackgroundView** view);
 
-  // Deletes the current status area and adds a new one.
-  void RecreateStatusArea();
-
   // Overridden from views::View:
   virtual void Layout();
 
diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc
index ac9b607..5fd0e65 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller.cc
@@ -35,8 +35,8 @@ const size_t kNoLogin = -1;
 
 ExistingUserController::ExistingUserController(
     const std::vector<UserManager::User>& users,
-    const gfx::Size& background_size)
-    : background_size_(background_size),
+    const gfx::Rect& background_bounds)
+    : background_bounds_(background_bounds),
       background_window_(NULL),
       background_view_(NULL),
       index_of_view_logging_in_(kNoLogin) {
@@ -45,8 +45,7 @@ ExistingUserController::ExistingUserController(
 
   // Caclulate the max number of users from available screen size.
   size_t max_users = kMaxUsers;
-  int screen_width =
-      views::Screen::GetMonitorAreaNearestPoint(gfx::Point(0, 0)).width();
+  int screen_width = background_bounds.width();
   if (screen_width > 0) {
     max_users = std::max(static_cast<size_t>(2), std::min(kMaxUsers,
         static_cast<size_t>((screen_width - UserController::kSize)
@@ -65,7 +64,7 @@ ExistingUserController::ExistingUserController(
 
 void ExistingUserController::Init() {
   background_window_ = BackgroundView::CreateWindowContainingView(
-      gfx::Rect(0, 0, background_size_.width(), background_size_.height()),
+      background_bounds_,
       &background_view_);
   background_window_->Show();
   for (size_t i = 0; i < controllers_.size(); ++i) {
@@ -99,7 +98,7 @@ void ExistingUserController::ProcessWmMessage(const WmIpc::Message& message,
 
   // WizardController takes care of deleting itself when done.
   WizardController* controller = new WizardController();
-  controller->Init(std::string(), false);
+  controller->Init(std::string(), background_bounds_, false);
   controller->Show();
 
   // Give the background window to the controller.
@@ -134,7 +133,7 @@ void ExistingUserController::Login(UserController* source,
 void ExistingUserController::OnLoginFailure(const std::string error) {
   LOG(INFO) << "OnLoginFailure";
 
-  // TODO: alert the user in some way to the failure.
+  // TODO(sky): alert the user in some way to the failure.
 
   controllers_[index_of_view_logging_in_]->SetPasswordEnabled(true);
 
diff --git a/chrome/browser/chromeos/login/existing_user_controller.h b/chrome/browser/chromeos/login/existing_user_controller.h
index f5e730a..ea25d99 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.h
+++ b/chrome/browser/chromeos/login/existing_user_controller.h
@@ -40,11 +40,10 @@ class ExistingUserController : public WmMessageListener::Observer,
                                public UserController::Delegate,
                                public LoginStatusConsumer {
  public:
-  // |login_size| is the size of the login window that is passed to the
-  // WizardWindow. |background_size| is not used if the user logs into using an
-  // existing user.
+  // Initializes views for known users. |background_bounds| determines the
+  // bounds of background view.
   ExistingUserController(const std::vector<UserManager::User>& users,
-                         const gfx::Size& background_size);
+                         const gfx::Rect& background_bounds);
 
   // Creates and shows the appropriate set of windows.
   void Init();
@@ -68,8 +67,8 @@ class ExistingUserController : public WmMessageListener::Observer,
   virtual void OnLoginFailure(const std::string error);
   virtual void OnLoginSuccess(const std::string username);
 
-  // Size of the background window.
-  const gfx::Size background_size_;
+  // Bounds of the background window.
+  const gfx::Rect background_bounds_;
 
   // Background window/view.
   views::Widget* background_window_;
diff --git a/chrome/browser/chromeos/login/wizard_controller.cc b/chrome/browser/chromeos/login/wizard_controller.cc
index f2d692d..815a1ce 100644
--- a/chrome/browser/chromeos/login/wizard_controller.cc
+++ b/chrome/browser/chromeos/login/wizard_controller.cc
@@ -9,6 +9,7 @@
 #include <sys/types.h>
 
 #include <string>
+#include <vector>
 
 #include "app/resource_bundle.h"
 #include "app/l10n_util.h"
@@ -86,6 +87,21 @@ class ContentView : public views::View {
   DISALLOW_COPY_AND_ASSIGN(ContentView);
 };
 
+
+// Returns bounds of the screen to use for login wizard.
+// The rect is centered within the default monitor and sized accordingly if
+// |size| is not empty. Otherwise the whole monitor is occupied.
+gfx::Rect CalculateScreenBounds(const gfx::Size& size) {
+  gfx::Rect bounds(views::Screen::GetMonitorWorkAreaNearestWindow(NULL));
+  if (!size.IsEmpty()) {
+    int horizontal_diff = bounds.width() - size.width();
+    int vertical_diff = bounds.height() - size.height();
+    bounds.Inset(horizontal_diff / 2, vertical_diff / 2);
+  }
+
+  return bounds;
+}
+
 }  // namespace
 
 // Initialize default controller.
@@ -117,17 +133,18 @@ WizardController::~WizardController() {
 }
 
 void WizardController::Init(const std::string& first_screen_name,
+                            const gfx::Rect& screen_bounds,
                             bool paint_background) {
   DCHECK(!contents_);
 
-  gfx::Rect screen_bounds =
-      views::Screen::GetMonitorWorkAreaNearestWindow(NULL);
-  int window_x = (screen_bounds.width() - kWizardScreenWidth) / 2;
-  int window_y = (screen_bounds.height() - kWizardScreenHeight) / 2;
+  int offset_x = (screen_bounds.width() - kWizardScreenWidth) / 2;
+  int offset_y = (screen_bounds.height() - kWizardScreenHeight) / 2;
+  int window_x = screen_bounds.x() + offset_x;
+  int window_y = screen_bounds.y() + offset_y;
 
-  contents_ = new ContentView(paint_background, window_x, window_y,
-                              screen_bounds.width(),
-                              screen_bounds.height());
+  contents_ = new ContentView(paint_background,
+                              offset_x, offset_y,
+                              screen_bounds.width(), screen_bounds.height());
 
   views::WidgetGtk* window =
       new views::WidgetGtk(views::WidgetGtk::TYPE_WINDOW);
@@ -159,10 +176,11 @@ void WizardController::Show() {
   widget_->Show();
 }
 
-void WizardController::ShowBackground(const gfx::Size& size) {
+void WizardController::ShowBackground(const gfx::Rect& bounds) {
   DCHECK(!background_widget_);
-  background_widget_ = chromeos::BackgroundView::CreateWindowContainingView(
-      gfx::Rect(0, 0, size.width(), size.height()), &background_view_);
+  background_widget_ =
+      chromeos::BackgroundView::CreateWindowContainingView(bounds,
+                                                           &background_view_);
   background_widget_->Show();
 }
 
@@ -240,13 +258,16 @@ void WizardController::OnUpdateCompleted() {
 ///////////////////////////////////////////////////////////////////////////////
 // WizardController, private:
 void WizardController::OnSwitchLanguage(const std::string& lang) {
-  // Delete all views that may may reference locale-specific data.
+  // Delete all views that may reference locale-specific data.
   SetCurrentScreen(NULL);
   network_screen_.reset();
   login_screen_.reset();
   account_screen_.reset();
   update_screen_.reset();
   contents_->RemoveAllChildViews(true);
+  // Can't delete background view since we don't know how to recreate it.
+  if (background_view_)
+    background_view_->Teardown();
 
   // Switch the locale.
   ResourceBundle::CleanupSharedInstance();
@@ -257,7 +278,7 @@ void WizardController::OnSwitchLanguage(const std::string& lang) {
 
   // Recreate view hierarchy and return to the wizard screen.
   if (background_view_)
-    background_view_->RecreateStatusArea();
+    background_view_->Init();
   OnExit(chromeos::ScreenObserver::LANGUAGE_CHANGED);
 }
 
@@ -333,6 +354,8 @@ namespace browser {
 // Declared in browser_dialogs.h so that others don't need to depend on our .h.
 void ShowLoginWizard(const std::string& first_screen_name,
                      const gfx::Size& size) {
+  gfx::Rect screen_bounds(CalculateScreenBounds(size));
+
   if (first_screen_name.empty() && chromeos::CrosLibrary::EnsureLoaded() &&
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableLoginImages)) {
@@ -340,13 +363,14 @@ void ShowLoginWizard(const std::string& first_screen_name,
         chromeos::UserManager::Get()->GetUsers();
     if (!users.empty()) {
       // ExistingUserController deletes itself.
-      (new chromeos::ExistingUserController(users, size))->Init();
+      (new chromeos::ExistingUserController(users, screen_bounds))->Init();
       return;
     }
   }
+
   WizardController* controller = new WizardController();
-  controller->ShowBackground(size);
-  controller->Init(first_screen_name, true);
+  controller->ShowBackground(screen_bounds);
+  controller->Init(first_screen_name, screen_bounds, true);
   controller->Show();
   if (chromeos::CrosLibrary::EnsureLoaded())
     chromeos::LoginLibrary::Get()->EmitLoginPromptReady();
diff --git a/chrome/browser/chromeos/login/wizard_controller.h b/chrome/browser/chromeos/login/wizard_controller.h
index 475d4465..13f877b 100644
--- a/chrome/browser/chromeos/login/wizard_controller.h
+++ b/chrome/browser/chromeos/login/wizard_controller.h
@@ -20,6 +20,10 @@ namespace chromeos {
 class BackgroundView;
 }
 
+namespace gfx {
+class Rect;
+}
+
 namespace views {
 class Views;
 class Widget;
@@ -41,8 +45,10 @@ class WizardController : public chromeos::ScreenObserver,
   // Shows the first screen defined by |first_screen_name| or by default
   // if the parameter is empty. |paint_background| indicates whether a
   // background should be painted. If |paint_background| is false, the window is
-  // made transparent.
+  // made transparent. |screen_bounds| are used to calculate position of the
+  // wizard screen.
   void Init(const std::string& first_screen_name,
+            const gfx::Rect& screen_bounds,
             bool paint_background);
 
   // Returns the view that contains all the other views.
@@ -52,7 +58,7 @@ class WizardController : public chromeos::ScreenObserver,
   void Show();
 
   // Creates and shows a background window.
-  void ShowBackground(const gfx::Size& size);
+  void ShowBackground(const gfx::Rect& bounds);
 
   // Takes ownership of the specified background widget and view.
   void OwnBackground(views::Widget* background_widget,