Adapt OOBE UI Dialog sizing to UI specs.

Previously OOBE UI Dialog size was controlled from JS code.
This CL changes implementation so that dialog size is set according
to UI specifications in C++ code, and web-based ui should just fully
fit into given dialog.

Note that code in ash/wm/lock_layout_manager / lock_window_state
propagates bounds changes only to full-screen children, so
OobeUIDialogDelegate will create full-screen Widget that will
host WebUI dialog, receive all bound-changed events and update
hosted dialog size according to specs.

Bug: 1007294, 1007861
Change-Id: Ib81ec7f886cd406df98838de5af8401083de0b87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841953Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Denis Kuznetsov <antrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706966}

chrome/browser/chromeos/BUILD.gn

@@ -1486,6 +1486,8 @@ source_set("chromeos") {
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h",
     "login/ui/login_web_dialog.cc",
     "login/ui/login_web_dialog.h",
+    "login/ui/oobe_dialog_size_utils.cc",
+    "login/ui/oobe_dialog_size_utils.h",
     "login/ui/oobe_ui_dialog_delegate.cc",
     "login/ui/oobe_ui_dialog_delegate.h",
     "login/ui/simple_web_view_dialog.cc",
@@ -2695,6 +2697,7 @@ source_set("unit_tests") {
     "login/supervised/supervised_user_authentication_unittest.cc",
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate_unittest.cc",
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view_unittest.cc",
+    "login/ui/oobe_dialog_size_utils_unittest.cc",
     "login/users/affiliation_unittest.cc",
     "login/users/multi_profile_user_controller_unittest.cc",
     "login/users/user_manager_unittest.cc",

chrome/browser/chromeos/login/ui/fake_login_display_host.cc

@@ -117,8 +117,6 @@ void FakeLoginDisplayHost::ShowGaiaDialog(bool can_close,
 
 void FakeLoginDisplayHost::HideOobeDialog() {}
 
-void FakeLoginDisplayHost::UpdateOobeDialogSize(int width, int height) {}
-
 void FakeLoginDisplayHost::UpdateOobeDialogState(ash::OobeDialogState state) {}
 
 const user_manager::UserList FakeLoginDisplayHost::GetUsers() {

chrome/browser/chromeos/login/ui/fake_login_display_host.h

@@ -56,7 +56,6 @@ class FakeLoginDisplayHost : public LoginDisplayHost {
   void ShowGaiaDialog(bool can_close,
                       const AccountId& prefilled_account) override;
   void HideOobeDialog() override;
-  void UpdateOobeDialogSize(int width, int height) override;
   void UpdateOobeDialogState(ash::OobeDialogState state) override;
   const user_manager::UserList GetUsers() override;
   void CancelPasswordChangedFlow() override;

chrome/browser/chromeos/login/ui/login_display_host.h

@@ -139,9 +139,6 @@ class LoginDisplayHost {
   // Hide any visible oobe dialog.
   virtual void HideOobeDialog() = 0;
 
-  // Update the size of the oobe dialog.
-  virtual void UpdateOobeDialogSize(int width, int height) = 0;
-
   // Update the state of the oobe dialog.
   virtual void UpdateOobeDialogState(ash::OobeDialogState state) = 0;
 

chrome/browser/chromeos/login/ui/login_display_host_mojo.cc

@@ -288,11 +288,6 @@ void LoginDisplayHostMojo::HideOobeDialog() {
   HideDialog();
 }
 
-void LoginDisplayHostMojo::UpdateOobeDialogSize(int width, int height) {
-  if (dialog_)
-    dialog_->UpdateSizeAndPosition(width, height);
-}
-
 void LoginDisplayHostMojo::UpdateOobeDialogState(ash::OobeDialogState state) {
   if (dialog_)
     dialog_->SetState(state);

chrome/browser/chromeos/login/ui/login_display_host_mojo.h

@@ -87,7 +87,6 @@ class LoginDisplayHostMojo : public LoginDisplayHostCommon,
   void ShowGaiaDialog(bool can_close,
                       const AccountId& prefilled_account) override;
   void HideOobeDialog() override;
-  void UpdateOobeDialogSize(int width, int height) override;
   void UpdateOobeDialogState(ash::OobeDialogState state) override;
   const user_manager::UserList GetUsers() override;
   void OnCancelPasswordChangedFlow() override;
@@ -176,7 +175,7 @@ class LoginDisplayHostMojo : public LoginDisplayHostCommon,
 
   // Called after host deletion.
   std::vector<base::OnceClosure> completion_callbacks_;
-  OobeUIDialogDelegate* dialog_ = nullptr;
+  OobeUIDialogDelegate* dialog_ = nullptr;  // Not owned.
   bool can_close_dialog_ = true;
   std::unique_ptr<WizardController> wizard_controller_;
 

chrome/browser/chromeos/login/ui/login_display_host_webui.cc

@@ -963,10 +963,6 @@ void LoginDisplayHostWebUI::HideOobeDialog() {
   NOTREACHED();
 }
 
-void LoginDisplayHostWebUI::UpdateOobeDialogSize(int width, int height) {
-  NOTREACHED();
-}
-
 void LoginDisplayHostWebUI::UpdateOobeDialogState(ash::OobeDialogState state) {
   NOTREACHED();
 }

chrome/browser/chromeos/login/ui/login_display_host_webui.h

@@ -84,7 +84,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
   void ShowGaiaDialog(bool can_close,
                       const AccountId& prefilled_account) override;
   void HideOobeDialog() override;
-  void UpdateOobeDialogSize(int width, int height) override;
   void UpdateOobeDialogState(ash::OobeDialogState state) override;
   const user_manager::UserList GetUsers() override;
   void ShowFeedback() override;

chrome/browser/chromeos/login/ui/mock_login_display_host.h

@@ -53,7 +53,6 @@ class MockLoginDisplayHost : public LoginDisplayHost {
   MOCK_METHOD1(StartArcKiosk, void(const AccountId&));
   MOCK_METHOD2(ShowGaiaDialog, void(bool, const AccountId&));
   MOCK_METHOD0(HideOobeDialog, void());
-  MOCK_METHOD2(UpdateOobeDialogSize, void(int, int));
   MOCK_METHOD1(UpdateOobeDialogState, void(ash::OobeDialogState state));
   MOCK_METHOD0(GetUsers, const user_manager::UserList(void));
 

chrome/browser/chromeos/login/ui/oobe_dialog_size_utils.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/login/ui/oobe_dialog_size_utils.h"
+#include "ui/gfx/geometry/insets.h"
+
+namespace chromeos {
+
+namespace {
+
+constexpr gfx::Size kMaxDialogSize{768, 768};
+constexpr gfx::Size kMinDialogSize{464, 464};
+constexpr gfx::Insets kMinMargins{48, 48};
+
+}  // namespace
+
+void CalculateOobeDialogBounds(const gfx::Rect& host_bounds,
+                               int shelf_height,
+                               gfx::Rect* result) {
+  // Area to position dialog.
+  gfx::Rect available_area = host_bounds;
+  available_area.Inset(0, 0, 0, shelf_height);
+
+  // Inset minimum margin on each side of area.
+  gfx::Rect area_no_margins = available_area;
+  area_no_margins.Inset(kMinMargins);
+
+  gfx::Size dialog_size = area_no_margins.size();
+  dialog_size.SetToMin(kMaxDialogSize);
+  dialog_size.SetToMax(kMinDialogSize);
+
+  // Still, dialog should fit into available area.
+  dialog_size.SetToMin(available_area.size());
+
+  // Center dialog within an available area.
+  *result = available_area;
+  result->ClampToCenteredSize(dialog_size);
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/ui/oobe_dialog_size_utils.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_UI_OOBE_DIALOG_SIZE_UTILS_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_UI_OOBE_DIALOG_SIZE_UTILS_H_
+
+#include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace chromeos {
+
+// Position OOBE dialog according to specs inside |host_bounds| excluding shelf.
+// |host_bounds| is in coordinates of oobe dialog widget. |result| is
+// in the same coordinates of |host_bounds|.
+void CalculateOobeDialogBounds(const gfx::Rect& host_bounds,
+                               int shelf_height,
+                               gfx::Rect* result);
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_UI_OOBE_DIALOG_SIZE_UTILS_H_

chrome/browser/chromeos/login/ui/oobe_dialog_size_utils_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/login/ui/oobe_dialog_size_utils.h"
+
+#include <stddef.h>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace chromeos {
+
+namespace {
+
+constexpr int kGaiaDialogMaxSize = 768;
+constexpr int kGaiaDialogMinMargin = 48;
+constexpr int kGaiaDialogMinSize = 464;
+
+constexpr int kShelfHeight = 56;
+constexpr int kVirtualKeyboardHeight = 280;
+constexpr int kDockedMagnifierHeight = 235;
+
+}  // namespace
+
+class OobeDialogSizeUtilsTest : public testing::Test {
+ public:
+  OobeDialogSizeUtilsTest() = default;
+
+  ~OobeDialogSizeUtilsTest() override = default;
+
+  void ValidateDialog(const gfx::Rect& area, const gfx::Rect& dialog) {
+    // Dialog should fully fit into the area.
+    EXPECT_TRUE(area.Contains(dialog));
+
+    EXPECT_GE(dialog.x(), area.x());
+    EXPECT_LE(dialog.right(), area.right());
+    EXPECT_GE(dialog.y(), area.y());
+    EXPECT_LE(dialog.bottom(), area.bottom());
+
+    // Dialog is centered in area.
+    EXPECT_EQ(area.CenterPoint(), dialog.CenterPoint());
+
+    EXPECT_LE(dialog.width(), kGaiaDialogMaxSize);
+    EXPECT_LE(dialog.height(), kGaiaDialogMaxSize);
+    // If there is at least some space, we should have margins.
+    if (dialog.width() > kGaiaDialogMinSize) {
+      EXPECT_GE(dialog.x(), kGaiaDialogMinMargin);
+      EXPECT_GE(area.right() - dialog.right(), kGaiaDialogMinMargin);
+    }
+    if (dialog.height() > kGaiaDialogMinSize) {
+      EXPECT_TRUE(dialog.y() >= kGaiaDialogMinMargin);
+      EXPECT_TRUE(area.bottom() - dialog.bottom() >= kGaiaDialogMinMargin);
+    }
+    // If dialog size is lesser than minimum size, there should be no margins
+    if (dialog.width() < kGaiaDialogMinSize) {
+      EXPECT_EQ(dialog.x(), area.x());
+      EXPECT_EQ(dialog.right(), area.right());
+    }
+    if (dialog.height() < kGaiaDialogMinSize) {
+      EXPECT_EQ(dialog.y(), area.y());
+      EXPECT_EQ(dialog.bottom(), area.bottom());
+    }
+  }
+
+  gfx::Rect SizeWithoutShelf(const gfx::Rect& area) const {
+    return gfx::Rect(area.width(), area.height() - kShelfHeight);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(OobeDialogSizeUtilsTest);
+};
+
+// We have plenty of space on the screen.
+TEST_F(OobeDialogSizeUtilsTest, Chromebook) {
+  gfx::Rect usual_device(1200, 800);
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(usual_device, kShelfHeight, &dialog);
+  ValidateDialog(SizeWithoutShelf(usual_device), dialog);
+}
+
+// We have plenty of space on the screen, but virtual keyboard takes some space.
+TEST_F(OobeDialogSizeUtilsTest, ChromebookVirtualKeyboard) {
+  gfx::Rect usual_device_with_keyboard(1200, 800 - kVirtualKeyboardHeight);
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(usual_device_with_keyboard, 0, &dialog);
+  ValidateDialog(usual_device_with_keyboard, dialog);
+}
+
+// Tablet device can have smaller screen size.
+TEST_F(OobeDialogSizeUtilsTest, TabletHorizontal) {
+  gfx::Rect tablet_device(1080, 675);
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(tablet_device, kShelfHeight, &dialog);
+  ValidateDialog(SizeWithoutShelf(tablet_device), dialog);
+}
+
+// Tablet device in horizontal mode with virtual keyboard have restricted space.
+TEST_F(OobeDialogSizeUtilsTest, TabletHorizontalVirtualKeyboard) {
+  gfx::Rect tablet_device(1080, 675 - kVirtualKeyboardHeight);
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(tablet_device, 0, &dialog);
+  ValidateDialog(tablet_device, dialog);
+}
+
+// Tablet device in horizontal mode with docked magnifier have restricted space.
+TEST_F(OobeDialogSizeUtilsTest, TabletHorizontalDockedMagnifier) {
+  gfx::Rect tablet_device(0, 0, 1080, 675 - kDockedMagnifierHeight);
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(tablet_device, 0, &dialog);
+  ValidateDialog(tablet_device, dialog);
+}
+
+// Tablet device in horizontal mode with virtual keyboard and docked
+// magnifier results in very few space.
+TEST_F(OobeDialogSizeUtilsTest, TabletHorizontalVirtualKeyboardMagnifier) {
+  gfx::Rect tablet_device(
+      0, 0, 1080, 675 - kVirtualKeyboardHeight - kDockedMagnifierHeight);
+
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(tablet_device, 0, &dialog);
+  ValidateDialog(tablet_device, dialog);
+}
+
+// Tablet in vertical mode puts some strain on dialog width.
+TEST_F(OobeDialogSizeUtilsTest, TabletVertical) {
+  gfx::Rect tablet_device(675, 1080);
+  gfx::Rect dialog;
+
+  CalculateOobeDialogBounds(tablet_device, kShelfHeight, &dialog);
+  ValidateDialog(SizeWithoutShelf(tablet_device), dialog);
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.h

@@ -8,8 +8,6 @@
 #include <string>
 
 #include "ash/public/cpp/login_types.h"
-#include "ash/public/cpp/tablet_mode.h"
-#include "ash/public/cpp/tablet_mode_observer.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
@@ -18,8 +16,6 @@
 #include "chrome/browser/ui/ash/keyboard/chrome_keyboard_controller_client.h"
 #include "chrome/browser/ui/chrome_web_modal_dialog_manager_delegate.h"
 #include "components/web_modal/web_contents_modal_dialog_host.h"
-#include "ui/display/display_observer.h"
-#include "ui/display/screen.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
 
 namespace content {
@@ -38,8 +34,10 @@ class Widget;
 namespace chromeos {
 
 class CaptivePortalDialogDelegate;
+class LayoutWidgetDelegateView;
 class LoginDisplayHostMojo;
 class OobeUI;
+class OobeWebDialogView;
 
 // This class manages the behavior of the Oobe UI dialog.
 // And its lifecycle is managed by the widget created in Show().
@@ -48,9 +46,7 @@ class OobeUI;
 //         |
 //         V
 //   clientView---->Widget's view hierarchy
-class OobeUIDialogDelegate : public display::DisplayObserver,
-                             public ash::TabletModeObserver,
-                             public ui::WebDialogDelegate,
+class OobeUIDialogDelegate : public ui::WebDialogDelegate,
                              public ChromeKeyboardControllerClient::Observer,
                              public CaptivePortalWindowProxy::Observer {
  public:
@@ -80,20 +76,10 @@ class OobeUIDialogDelegate : public display::DisplayObserver,
 
   content::WebContents* GetWebContents();
 
-  void UpdateSizeAndPosition(int width, int height);
   OobeUI* GetOobeUI() const;
   gfx::NativeWindow GetNativeWindow() const;
 
  private:
-  // display::DisplayObserver:
-  void OnDisplayMetricsChanged(const display::Display& display,
-                               uint32_t changed_metrics) override;
-  // ash::TabletModeObserver:
-  void OnTabletModeStarted() override;
-  void OnTabletModeEnded() override;
-
-  void OnTabletModeToggled(bool enabled);
-
   // ui::WebDialogDelegate:
   ui::ModalType GetDialogModalType() const override;
   base::string16 GetDialogTitle() const override;
@@ -103,7 +89,8 @@ class OobeUIDialogDelegate : public display::DisplayObserver,
   void GetDialogSize(gfx::Size* size) const override;
   bool CanResizeDialog() const override;
   std::string GetDialogArgs() const override;
-  // NOTE: This function deletes this object at the end.
+  // NOTE: This function starts cleanup sequence that would call FinishCleanup
+  // and delete this object in the end.
   void OnDialogClosed(const std::string& json_retval) override;
   void OnCloseContents(content::WebContents* source,
                        bool* out_close_dialog) override;
@@ -124,17 +111,14 @@ class OobeUIDialogDelegate : public display::DisplayObserver,
 
   base::WeakPtr<CaptivePortalDialogDelegate> captive_portal_delegate_;
 
-  // This is owned by the underlying native widget.
-  // Before its deletion, onDialogClosed will get called and delete this object.
-  views::Widget* dialog_widget_ = nullptr;
-  views::WebDialogView* dialog_view_ = nullptr;
-  gfx::Size size_;
-  bool showing_fullscreen_ = false;
-
-  ScopedObserver<display::Screen, display::DisplayObserver> display_observer_{
-      this};
-  ScopedObserver<ash::TabletMode, ash::TabletModeObserver>
-      tablet_mode_observer_{this};
+  // Root widget. It is assumed that widget is placed as a full-screen inside
+  // LockContainer.
+  views::Widget* widget_ = nullptr;
+  // Reference to view owned by widget_.
+  LayoutWidgetDelegateView* layout_view_ = nullptr;
+  // Reference to dialog view stored in widget_.
+  OobeWebDialogView* dialog_view_ = nullptr;
+
   ScopedObserver<ChromeKeyboardControllerClient,
                  ChromeKeyboardControllerClient::Observer>
       keyboard_observer_{this};

chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc

@@ -716,8 +716,6 @@ void GaiaScreenHandler::RegisterMessages() {
   AddRawCallback("showAddUser", &GaiaScreenHandler::HandleShowAddUser);
   AddCallback("getIsSamlUserPasswordless",
               &GaiaScreenHandler::HandleGetIsSamlUserPasswordless);
-  AddCallback("updateOobeDialogSize",
-              &GaiaScreenHandler::HandleUpdateOobeDialogSize);
   AddCallback("hideOobeDialog", &GaiaScreenHandler::HandleHideOobeDialog);
   AddCallback("updateSigninUIState",
               &GaiaScreenHandler::HandleUpdateSigninUIState);
@@ -1004,11 +1002,6 @@ void GaiaScreenHandler::HandleGaiaUIReady() {
   }
 }
 
-void GaiaScreenHandler::HandleUpdateOobeDialogSize(int width, int height) {
-  if (LoginDisplayHost::default_host())
-    LoginDisplayHost::default_host()->UpdateOobeDialogSize(width, height);
-}
-
 void GaiaScreenHandler::HandleHideOobeDialog() {
   if (LoginDisplayHost::default_host())
     LoginDisplayHost::default_host()->HideOobeDialog();

chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h

@@ -224,7 +224,6 @@ class GaiaScreenHandler : public BaseScreenHandler,
   void HandleIdentifierEntered(const std::string& account_identifier);
 
   void HandleAuthExtensionLoaded();
-  void HandleUpdateOobeDialogSize(int width, int height);
   void HandleHideOobeDialog();
   void HandleShowAddUser(const base::ListValue* args);
   void HandleGetIsSamlUserPasswordless(const std::string& callback_id,

ui/login/display_manager.js

@@ -786,10 +786,6 @@ cr.define('cr.ui.login', function() {
         screen.style.height = height + 'px';
         screen.style.margin = 'auto';
       }
-
-      if (this.showingViewsLogin) {
-        chrome.send('updateOobeDialogSize', [width, height]);
-      }
     },
 
     /**