Reland "Fix calculation of inner padding of OOBE dialog depending on outer margins"

This is a reland of 0908b290

Original change's description:
> Fix calculation of inner padding of OOBE dialog depending on outer margins
> 
> Bug: 1007294
> Change-Id: I99b7b3cc46e12c0fb060f76e086f5f31bd4cb812
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865216
> Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
> Auto-Submit: Denis Kuznetsov <antrim@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#707055}

Bug: 1007294
Change-Id: Iba03d0ed0f931ed50ced79b30dea18735fb127f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868990Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Commit-Queue: Denis Kuznetsov <antrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707541}

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

@@ -10,6 +10,7 @@ namespace chromeos {
 namespace {
 
 constexpr gfx::Size kMaxDialogSize{768, 768};
+constexpr int kDialogHeightForWidePadding = 640;
 constexpr gfx::Size kMinDialogSize{464, 464};
 constexpr gfx::Insets kMinMargins{48, 48};
 
@@ -17,7 +18,8 @@ constexpr gfx::Insets kMinMargins{48, 48};
 
 void CalculateOobeDialogBounds(const gfx::Rect& host_bounds,
                                int shelf_height,
-                               gfx::Rect* result) {
+                               gfx::Rect* result,
+                               OobeDialogPaddingMode* result_padding) {
   // Area to position dialog.
   gfx::Rect available_area = host_bounds;
   available_area.Inset(0, 0, 0, shelf_height);
@@ -36,6 +38,14 @@ void CalculateOobeDialogBounds(const gfx::Rect& host_bounds,
   // Center dialog within an available area.
   *result = available_area;
   result->ClampToCenteredSize(dialog_size);
+  if (!result_padding)
+    return;
+  if ((result->width() >= kMaxDialogSize.width()) &&
+      (result->height() >= kDialogHeightForWidePadding)) {
+    *result_padding = OobeDialogPaddingMode::PADDING_WIDE;
+  } else {
+    *result_padding = OobeDialogPaddingMode::PADDING_NARROW;
+  }
 }
 
 }  // namespace chromeos

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

@@ -10,12 +10,24 @@
 
 namespace chromeos {
 
+// Enum that specifies how inner padding of OOBE dialog should be calculated.
+enum class OobeDialogPaddingMode {
+  // Oobe dialog is displayed full screen, padding will be calculated
+  // via css depending on media size.
+  PADDING_AUTO,
+  // Oobe dialog have enough free space around and should use wide padding.
+  PADDING_WIDE,
+  // Oobe dialog is positioned in limited space and should use narrow padding.
+  PADDING_NARROW
+};
+
 // 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);
+                               gfx::Rect* result,
+                               OobeDialogPaddingMode* result_padding);
 
 }  // namespace chromeos
 

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

@@ -76,66 +76,82 @@ class OobeDialogSizeUtilsTest : public testing::Test {
 TEST_F(OobeDialogSizeUtilsTest, Chromebook) {
   gfx::Rect usual_device(1200, 800);
   gfx::Rect dialog;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(usual_device, kShelfHeight, &dialog);
+  CalculateOobeDialogBounds(usual_device, kShelfHeight, &dialog, &padding);
   ValidateDialog(SizeWithoutShelf(usual_device), dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_WIDE);
 }
 
 // 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;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(usual_device_with_keyboard, 0, &dialog);
+  CalculateOobeDialogBounds(usual_device_with_keyboard, 0, &dialog, &padding);
   ValidateDialog(usual_device_with_keyboard, dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_NARROW);
 }
 
 // Tablet device can have smaller screen size.
 TEST_F(OobeDialogSizeUtilsTest, TabletHorizontal) {
   gfx::Rect tablet_device(1080, 675);
   gfx::Rect dialog;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(tablet_device, kShelfHeight, &dialog);
+  CalculateOobeDialogBounds(tablet_device, kShelfHeight, &dialog, &padding);
   ValidateDialog(SizeWithoutShelf(tablet_device), dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_NARROW);
 }
 
-// Tablet device in horizontal mode with virtual keyboard have restricted space.
+// Tablet device in horizontal mode with virtual keyboard have restricted
+// vertical space.
 TEST_F(OobeDialogSizeUtilsTest, TabletHorizontalVirtualKeyboard) {
   gfx::Rect tablet_device(1080, 675 - kVirtualKeyboardHeight);
   gfx::Rect dialog;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(tablet_device, 0, &dialog);
+  CalculateOobeDialogBounds(tablet_device, 0, &dialog, &padding);
   ValidateDialog(tablet_device, dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_NARROW);
 }
 
-// Tablet device in horizontal mode with docked magnifier have restricted space.
+// Tablet device in horizontal mode with docked magnifier have restricted
+// vertical space.
 TEST_F(OobeDialogSizeUtilsTest, TabletHorizontalDockedMagnifier) {
   gfx::Rect tablet_device(0, 0, 1080, 675 - kDockedMagnifierHeight);
   gfx::Rect dialog;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(tablet_device, 0, &dialog);
+  CalculateOobeDialogBounds(tablet_device, 0, &dialog, &padding);
   ValidateDialog(tablet_device, dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_NARROW);
 }
 
 // Tablet device in horizontal mode with virtual keyboard and docked
-// magnifier results in very few space.
+// magnifier results in very few vertical space.
 TEST_F(OobeDialogSizeUtilsTest, TabletHorizontalVirtualKeyboardMagnifier) {
   gfx::Rect tablet_device(
       0, 0, 1080, 675 - kVirtualKeyboardHeight - kDockedMagnifierHeight);
 
   gfx::Rect dialog;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(tablet_device, 0, &dialog);
+  CalculateOobeDialogBounds(tablet_device, 0, &dialog, &padding);
   ValidateDialog(tablet_device, dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_NARROW);
 }
 
 // Tablet in vertical mode puts some strain on dialog width.
 TEST_F(OobeDialogSizeUtilsTest, TabletVertical) {
   gfx::Rect tablet_device(675, 1080);
   gfx::Rect dialog;
+  OobeDialogPaddingMode padding;
 
-  CalculateOobeDialogBounds(tablet_device, kShelfHeight, &dialog);
+  CalculateOobeDialogBounds(tablet_device, kShelfHeight, &dialog, &padding);
   ValidateDialog(SizeWithoutShelf(tablet_device), dialog);
+  EXPECT_EQ(padding, OobeDialogPaddingMode::PADDING_NARROW);
 }
 
 }  // namespace chromeos

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

@@ -20,6 +20,7 @@
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/login_screen_client.h"
 #include "chrome/browser/ui/webui/chrome_web_contents_handler.h"
+#include "chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
@@ -43,6 +44,18 @@ constexpr char kGaiaURL[] = "chrome://oobe/gaia-signin";
 constexpr char kAppLaunchBailout[] = "app_launch_bailout";
 constexpr char kCancel[] = "cancel";
 
+CoreOobeView::DialogPaddingMode ConvertDialogPaddingMode(
+    OobeDialogPaddingMode padding) {
+  switch (padding) {
+    case OobeDialogPaddingMode::PADDING_AUTO:
+      return CoreOobeView::DialogPaddingMode::MODE_AUTO;
+    case OobeDialogPaddingMode::PADDING_WIDE:
+      return CoreOobeView::DialogPaddingMode::MODE_WIDE;
+    case OobeDialogPaddingMode::PADDING_NARROW:
+      return CoreOobeView::DialogPaddingMode::MODE_NARROW;
+  }
+}
+
 }  // namespace
 
 class OobeWebDialogView : public views::WebDialogView {
@@ -124,6 +137,8 @@ class LayoutWidgetDelegateView : public views::WidgetDelegateView {
     Layout();
   }
 
+  OobeDialogPaddingMode padding() { return padding_; }
+
   // views::WidgetDelegateView:
   ui::ModalType GetModalType() const override { return ui::MODAL_TYPE_WINDOW; }
 
@@ -131,14 +146,18 @@ class LayoutWidgetDelegateView : public views::WidgetDelegateView {
 
   void Layout() override {
     if (fullscreen_) {
-      oobe_view_->SetBoundsRect(GetContentsBounds());
+      for (views::View* child : children()) {
+        child->SetBoundsRect(GetContentsBounds());
+      }
+      padding_ = OobeDialogPaddingMode::PADDING_AUTO;
       return;
     }
 
     gfx::Rect bounds;
     const int shelf_height =
         has_shelf_ ? ash::ShelfConfig::Get()->shelf_size() : 0;
-    CalculateOobeDialogBounds(GetContentsBounds(), shelf_height, &bounds);
+    CalculateOobeDialogBounds(GetContentsBounds(), shelf_height, &bounds,
+                              &padding_);
 
     for (views::View* child : children()) {
       child->SetBoundsRect(bounds);
@@ -155,6 +174,9 @@ class LayoutWidgetDelegateView : public views::WidgetDelegateView {
   // space).
   bool has_shelf_ = true;
 
+  // Tracks dialog margins after last size calculations.
+  OobeDialogPaddingMode padding_ = OobeDialogPaddingMode::PADDING_AUTO;
+
   DISALLOW_COPY_AND_ASSIGN(LayoutWidgetDelegateView);
 };
 
@@ -308,6 +330,8 @@ OobeUIDialogDelegate::OobeUIDialogDelegate(
   layout_view_->SetHasShelf(
       !ChromeKeyboardControllerClient::Get()->is_keyboard_visible());
 
+  dialog_view_->AddObserver(this);
+
   extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
       dialog_view_->web_contents());
 
@@ -321,6 +345,7 @@ OobeUIDialogDelegate::OobeUIDialogDelegate(
 }
 
 OobeUIDialogDelegate::~OobeUIDialogDelegate() {
+  dialog_view_->RemoveObserver(this);
   if (captive_portal_delegate_)
     captive_portal_delegate_->Close();
   if (controller_)
@@ -460,6 +485,13 @@ bool OobeUIDialogDelegate::AcceleratorPressed(
   return true;
 }
 
+void OobeUIDialogDelegate::OnViewBoundsChanged(views::View* observed_view) {
+  if (!widget_)
+    return;
+  GetOobeUI()->GetCoreOobeView()->SetDialogPaddingMode(
+      ConvertDialogPaddingMode(layout_view_->padding()));
+}
+
 void OobeUIDialogDelegate::OnKeyboardVisibilityChanged(bool visible) {
   if (!widget_)
     return;

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

@@ -16,6 +16,7 @@
 #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/views/view_observer.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
 
 namespace content {
@@ -27,6 +28,7 @@ class Accelerator;
 }
 
 namespace views {
+class View;
 class WebDialogView;
 class Widget;
 }  // namespace views
@@ -48,7 +50,8 @@ class OobeWebDialogView;
 //   clientView---->Widget's view hierarchy
 class OobeUIDialogDelegate : public ui::WebDialogDelegate,
                              public ChromeKeyboardControllerClient::Observer,
-                             public CaptivePortalWindowProxy::Observer {
+                             public CaptivePortalWindowProxy::Observer,
+                             public views::ViewObserver {
  public:
   explicit OobeUIDialogDelegate(base::WeakPtr<LoginDisplayHostMojo> controller);
   ~OobeUIDialogDelegate() override;
@@ -100,6 +103,9 @@ class OobeUIDialogDelegate : public ui::WebDialogDelegate,
   std::vector<ui::Accelerator> GetAccelerators() override;
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
 
+  // views::ViewObserver:
+  void OnViewBoundsChanged(views::View* observed_view) override;
+
   // ChromeKeyboardControllerClient::Observer:
   void OnKeyboardVisibilityChanged(bool visible) override;
 

chrome/browser/resources/chromeos/login/cr_ui.js

@@ -380,6 +380,14 @@ cr.define('cr.ui', function() {
     Oobe.getInstance().setShelfHeight(height);
   };
 
+  /**
+   * Sets the hint for calculating OOBE dialog margins.
+   * @param {OobeTypes.DialogPaddingMode} mode.
+   */
+  Oobe.setDialogPaddingMode = function(mode) {
+    Oobe.getInstance().setDialogPaddingMode(mode);
+  };
+
   /**
    * Get the primary display's name.
    *

chrome/browser/resources/chromeos/login/discover/discover_app.js

@@ -33,6 +33,7 @@ cr.define('cr.ui.Oobe', function() {
     setClientAreaSize: function(data) {},
     setLabelText: function(data) {},
     setShelfHeight: function(data) {},
+    setDialogPaddingMode: function(data) {},
     setVirtualKeyboardShown: function(data) {},
     showAPIKeysNotice: function(data) {},
     showOobeUI: function(data) {},

chrome/browser/resources/chromeos/login/oobe_types.js

@@ -134,3 +134,13 @@ OobeTypes.SecurityTokenPinDialogErrorType = {
  * }}
  */
 OobeTypes.SecurityTokenPinDialogParameters;
+
+/**
+ * Specifies the mechanism for calculating oobe-dialog inner padding.
+ * @enum {string}
+ */
+OobeTypes.DialogPaddingMode = {
+  AUTO: 'auto',
+  NARROW: 'narrow',
+  WIDE: 'wide',
+};

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

@@ -556,6 +556,25 @@ void CoreOobeHandler::UpdateClientAreaSize() {
   SetShelfHeight(ash::ShelfConfig::Get()->shelf_size());
 }
 
+void CoreOobeHandler::SetDialogPaddingMode(
+    CoreOobeView::DialogPaddingMode mode) {
+  std::string padding;
+  switch (mode) {
+    case CoreOobeView::DialogPaddingMode::MODE_AUTO:
+      padding = "auto";
+      break;
+    case CoreOobeView::DialogPaddingMode::MODE_NARROW:
+      padding = "narrow";
+      break;
+    case CoreOobeView::DialogPaddingMode::MODE_WIDE:
+      padding = "wide";
+      break;
+    default:
+      NOTREACHED();
+  }
+  CallJS("cr.ui.Oobe.setDialogPaddingMode", padding);
+}
+
 void CoreOobeHandler::OnOobeConfigurationChanged() {
   base::Value configuration(base::Value::Type::DICTIONARY);
   chromeos::configuration::FilterConfiguration(

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

@@ -39,6 +39,17 @@ namespace chromeos {
 
 class CoreOobeView {
  public:
+  // Enum that specifies how inner padding of OOBE dialog should be calculated.
+  enum class DialogPaddingMode {
+    // Oobe dialog is displayed full screen, padding will be calculated
+    // via css depending on media size.
+    MODE_AUTO,
+    // Oobe dialog have enough free space around and should use wide padding.
+    MODE_WIDE,
+    // Oobe dialog is positioned in limited space and should use narrow padding.
+    MODE_NARROW,
+  };
+
   virtual ~CoreOobeView() {}
 
   virtual void ShowSignInError(int login_attempts,
@@ -60,6 +71,7 @@ class CoreOobeView {
   virtual void SetVirtualKeyboardShown(bool shown) = 0;
   virtual void SetClientAreaSize(int width, int height) = 0;
   virtual void SetShelfHeight(int height) = 0;
+  virtual void SetDialogPaddingMode(DialogPaddingMode mode) = 0;
   virtual void ShowDeviceResetScreen() = 0;
   virtual void ShowEnableDebuggingScreen() = 0;
   virtual void InitDemoModeDetection() = 0;
@@ -140,6 +152,7 @@ class CoreOobeHandler : public BaseWebUIHandler,
   void SetVirtualKeyboardShown(bool displayed) override;
   void SetClientAreaSize(int width, int height) override;
   void SetShelfHeight(int height) override;
+  void SetDialogPaddingMode(CoreOobeView::DialogPaddingMode mode) override;
   void ShowDeviceResetScreen() override;
   void ShowEnableDebuggingScreen() override;
   void ShowActiveDirectoryPasswordChangeScreen(

ui/login/display_manager.js

@@ -318,6 +318,14 @@ cr.define('cr.ui.login', function() {
           '--shelf-area-height-base', height + 'px');
     },
 
+    /**
+     * Sets the hint for calculating OOBE dialog inner padding.
+     * @param {OobeTypes.DialogPaddingMode} mode.
+     */
+    setDialogPaddingMode: function(mode) {
+      document.documentElement.setAttribute('dialog-padding', mode);
+    },
+
     /**
      * Toggles background of main body between transparency and solid.
      * @param {boolean} solid Whether to show a solid background.

ui/login/oobe.css

@@ -56,6 +56,7 @@ html[screen=gaia-signin] {
   --oobe-dialog-side-margin: 0px;
 }
 
+
 @media screen and (max-width: 864px), (max-height: 736px) {
   html[screen=oobe] {
     --oobe-dialog-footer-height: 80px;
@@ -64,6 +65,12 @@ html[screen=gaia-signin] {
   }
 }
 
+html[screen=gaia-signin][dialog-padding=narrow] {
+  --oobe-dialog-footer-height: 80px;
+  --oobe-dialog-footer-padding: 24px;
+  --oobe-dialog-content-padding: 32px;
+}
+
 body.solid {
   background-color: white;
 }