Implement hiding of close button for LoginScreenExtensionUi window

Background: The chrome.loginScreenUi extension API can open windows while
in the ChromeOS signin screen. The API uses a views::WebDialogView to
show the window. One of the parameters of the API sets whether the window
can be closed by the user. Currently, setting the parameter to false
prevents the user from closing the window, but the close button is still
interactive (animates when hovered and when pressed).

ash::NonClientFrameViewAsh and DefaultCaptionButtonModel treats the close
button as always visible and always enabled. This CL changes
FrameCaptionButtonContainerView to not show the close button depending on
views::WidgetDelegate::ShouldShowCloseButton().

In addition, this CL also makes the LoginScreenExtensionUi window not
draggable.

BUG=985754

Change-Id: I191cfad13aeb2764fecaa8867cf344a16aeb0e6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709586
Commit-Queue: Jit Yao Yap <jityao@google.com>
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682727}

ash/frame/caption_buttons/frame_caption_button_container_view_unittest.cc

@@ -22,17 +22,24 @@ namespace {
 
 class TestWidgetDelegate : public views::WidgetDelegateView {
  public:
-  TestWidgetDelegate(bool can_maximize, bool can_minimize)
-      : can_maximize_(can_maximize), can_minimize_(can_minimize) {}
+  TestWidgetDelegate(bool can_maximize,
+                     bool can_minimize,
+                     bool close_button_visible)
+      : can_maximize_(can_maximize),
+        can_minimize_(can_minimize),
+        close_button_visible_(close_button_visible) {}
   ~TestWidgetDelegate() override = default;
 
   bool CanMaximize() const override { return can_maximize_; }
 
   bool CanMinimize() const override { return can_minimize_; }
 
+  bool ShouldShowCloseButton() const override { return close_button_visible_; }
+
  private:
   bool can_maximize_;
   bool can_minimize_;
+  bool close_button_visible_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWidgetDelegate);
 };
@@ -45,6 +52,8 @@ class FrameCaptionButtonContainerViewTest : public AshTestBase {
 
   enum MinimizeAllowed { MINIMIZE_ALLOWED, MINIMIZE_DISALLOWED };
 
+  enum CloseButtonVisible { CLOSE_BUTTON_VISIBLE, CLOSE_BUTTON_NOT_VISIBLE };
+
   FrameCaptionButtonContainerViewTest() = default;
 
   ~FrameCaptionButtonContainerViewTest() override = default;
@@ -52,13 +61,15 @@ class FrameCaptionButtonContainerViewTest : public AshTestBase {
   // Creates a widget which allows maximizing based on |maximize_allowed|.
   // The caller takes ownership of the returned widget.
   views::Widget* CreateTestWidget(MaximizeAllowed maximize_allowed,
-                                  MinimizeAllowed minimize_allowed)
+                                  MinimizeAllowed minimize_allowed,
+                                  CloseButtonVisible close_button_visible)
       WARN_UNUSED_RESULT {
     views::Widget* widget = new views::Widget;
     views::Widget::InitParams params;
     params.delegate =
         new TestWidgetDelegate(maximize_allowed == MAXIMIZE_ALLOWED,
-                               minimize_allowed == MINIMIZE_ALLOWED);
+                               minimize_allowed == MINIMIZE_ALLOWED,
+                               close_button_visible == CLOSE_BUTTON_VISIBLE);
     params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
     params.context = CurrentContext();
     widget->Init(params);
@@ -106,8 +117,8 @@ class FrameCaptionButtonContainerViewTest : public AshTestBase {
 TEST_F(FrameCaptionButtonContainerViewTest, ButtonVisibility) {
   // All the buttons should be visible when minimizing and maximizing are
   // allowed.
-  FrameCaptionButtonContainerView container1(
-      CreateTestWidget(MAXIMIZE_ALLOWED, MINIMIZE_ALLOWED));
+  FrameCaptionButtonContainerView container1(CreateTestWidget(
+      MAXIMIZE_ALLOWED, MINIMIZE_ALLOWED, CLOSE_BUTTON_VISIBLE));
   InitContainer(&container1);
   container1.Layout();
   FrameCaptionButtonContainerView::TestApi t1(&container1);
@@ -119,8 +130,8 @@ TEST_F(FrameCaptionButtonContainerViewTest, ButtonVisibility) {
 
   // The minimize button should be visible when minimizing is allowed but
   // maximizing is disallowed.
-  FrameCaptionButtonContainerView container2(
-      CreateTestWidget(MAXIMIZE_DISALLOWED, MINIMIZE_ALLOWED));
+  FrameCaptionButtonContainerView container2(CreateTestWidget(
+      MAXIMIZE_DISALLOWED, MINIMIZE_ALLOWED, CLOSE_BUTTON_VISIBLE));
   InitContainer(&container2);
   container2.Layout();
   FrameCaptionButtonContainerView::TestApi t2(&container2);
@@ -132,8 +143,8 @@ TEST_F(FrameCaptionButtonContainerViewTest, ButtonVisibility) {
 
   // Neither the minimize button nor the size button should be visible when
   // neither minimizing nor maximizing are allowed.
-  FrameCaptionButtonContainerView container3(
-      CreateTestWidget(MAXIMIZE_DISALLOWED, MINIMIZE_DISALLOWED));
+  FrameCaptionButtonContainerView container3(CreateTestWidget(
+      MAXIMIZE_DISALLOWED, MINIMIZE_DISALLOWED, CLOSE_BUTTON_VISIBLE));
   InitContainer(&container3);
   container3.Layout();
   FrameCaptionButtonContainerView::TestApi t3(&container3);
@@ -148,8 +159,8 @@ TEST_F(FrameCaptionButtonContainerViewTest, ButtonVisibility) {
 // correct placement of the buttons.
 TEST_F(FrameCaptionButtonContainerViewTest,
        TestUpdateSizeButtonVisibilityAnimation) {
-  FrameCaptionButtonContainerView container(
-      CreateTestWidget(MAXIMIZE_ALLOWED, MINIMIZE_ALLOWED));
+  FrameCaptionButtonContainerView container(CreateTestWidget(
+      MAXIMIZE_ALLOWED, MINIMIZE_ALLOWED, CLOSE_BUTTON_VISIBLE));
 
   // Add an extra button to the left of the size button to verify that it is
   // repositioned similarly to the minimize button. This simulates the PWA menu
@@ -213,4 +224,28 @@ TEST_F(FrameCaptionButtonContainerViewTest,
             initial_container_bounds.width());
 }
 
+// Test that the close button is visible when
+// |WidgetDelegate::ShouldShowCloseButton()| returns true.
+TEST_F(FrameCaptionButtonContainerViewTest, ShouldShowCloseButtonTrue) {
+  FrameCaptionButtonContainerView container(CreateTestWidget(
+      MAXIMIZE_ALLOWED, MINIMIZE_ALLOWED, CLOSE_BUTTON_VISIBLE));
+  InitContainer(&container);
+  container.Layout();
+  FrameCaptionButtonContainerView::TestApi testApi(&container);
+  EXPECT_TRUE(testApi.close_button()->GetVisible());
+  EXPECT_TRUE(testApi.close_button()->GetEnabled());
+}
+
+// Test that the close button is not visible when
+// |WidgetDelegate::ShouldShowCloseButton()| returns false.
+TEST_F(FrameCaptionButtonContainerViewTest, ShouldShowCloseButtonFalse) {
+  FrameCaptionButtonContainerView container(CreateTestWidget(
+      MAXIMIZE_ALLOWED, MINIMIZE_ALLOWED, CLOSE_BUTTON_NOT_VISIBLE));
+  InitContainer(&container);
+  container.Layout();
+  FrameCaptionButtonContainerView::TestApi testApi(&container);
+  EXPECT_FALSE(testApi.close_button()->GetVisible());
+  EXPECT_TRUE(testApi.close_button()->GetEnabled());
+}
+
 }  // namespace ash

ash/frame/non_client_frame_view_ash_unittest.cc

@@ -581,11 +581,15 @@ TEST_F(NonClientFrameViewAshTest, CustomButtonModel) {
   FrameCaptionButtonContainerView::TestApi test_api(
       header_view->caption_button_container());
 
-  // CLOSE buttion is always visible and enabled.
+  // CLOSE button is always enabled.
   EXPECT_TRUE(test_api.close_button());
-  EXPECT_TRUE(test_api.close_button()->GetVisible());
+  EXPECT_FALSE(test_api.close_button()->GetVisible());
   EXPECT_TRUE(test_api.close_button()->GetEnabled());
 
+  model_ptr->SetVisible(views::CAPTION_BUTTON_ICON_CLOSE, true);
+  non_client_frame_view->SizeConstraintsChanged();
+  EXPECT_TRUE(test_api.close_button()->GetVisible());
+
   EXPECT_FALSE(test_api.minimize_button()->GetVisible());
   EXPECT_FALSE(test_api.size_button()->GetVisible());
   EXPECT_FALSE(test_api.menu_button()->GetVisible());

ash/public/cpp/caption_buttons/frame_caption_button_container_view.cc

@@ -101,6 +101,19 @@ double CapAnimationValue(double value) {
   return std::min(1.0, std::max(0.0, value));
 }
 
+// Returns a |views::BoxLayout| layout manager with the settings needed by
+// FrameCaptionButtonContainerView.
+std::unique_ptr<views::BoxLayout> MakeBoxLayoutManager(
+    int minimum_cross_axis_size) {
+  std::unique_ptr<views::BoxLayout> layout = std::make_unique<views::BoxLayout>(
+      views::BoxLayout::Orientation::kHorizontal);
+  layout->set_cross_axis_alignment(
+      views::BoxLayout::CrossAxisAlignment::kCenter);
+  layout->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kEnd);
+  layout->set_minimum_cross_axis_size(minimum_cross_axis_size);
+  return layout;
+}
+
 // A default CaptionButtonModel that uses the widget delegate's state
 // to determine if each button should be visible and enabled.
 class DefaultCaptionButtonModel : public CaptionButtonModel {
@@ -120,7 +133,7 @@ class DefaultCaptionButtonModel : public CaptionButtonModel {
       case views::CAPTION_BUTTON_ICON_RIGHT_SNAPPED:
         return frame_->widget_delegate()->CanResize();
       case views::CAPTION_BUTTON_ICON_CLOSE:
-        return true;
+        return frame_->widget_delegate()->ShouldShowCloseButton();
 
       // No back or menu button by default.
       case views::CAPTION_BUTTON_ICON_BACK:
@@ -154,12 +167,7 @@ FrameCaptionButtonContainerView::FrameCaptionButtonContainerView(
     : views::AnimationDelegateViews(frame->GetRootView()),
       frame_(frame),
       model_(std::make_unique<DefaultCaptionButtonModel>(frame)) {
-  auto layout = std::make_unique<views::BoxLayout>(
-      views::BoxLayout::Orientation::kHorizontal);
-  layout->set_cross_axis_alignment(
-      views::BoxLayout::CrossAxisAlignment::kCenter);
-  layout->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kEnd);
-  SetLayoutManager(std::move(layout));
+  SetLayoutManager(MakeBoxLayoutManager(/*minimum_cross_axis_size=*/0));
   tablet_mode_animation_.reset(new gfx::SlideAnimation(this));
   tablet_mode_animation_->SetTweenType(gfx::Tween::LINEAR);
 
@@ -262,6 +270,8 @@ void FrameCaptionButtonContainerView::UpdateCaptionButtonState(bool animate) {
       model_->IsEnabled(views::CAPTION_BUTTON_ICON_MINIMIZE));
   menu_button_->SetVisible(model_->IsVisible(views::CAPTION_BUTTON_ICON_MENU));
   menu_button_->SetEnabled(model_->IsEnabled(views::CAPTION_BUTTON_ICON_MENU));
+  close_button_->SetVisible(
+      model_->IsVisible(views::CAPTION_BUTTON_ICON_CLOSE));
 }
 
 void FrameCaptionButtonContainerView::SetButtonSize(const gfx::Size& size) {
@@ -269,6 +279,8 @@ void FrameCaptionButtonContainerView::SetButtonSize(const gfx::Size& size) {
   minimize_button_->SetPreferredSize(size);
   size_button_->SetPreferredSize(size);
   close_button_->SetPreferredSize(size);
+
+  SetLayoutManager(MakeBoxLayoutManager(size.height()));
 }
 
 void FrameCaptionButtonContainerView::SetModel(
@@ -284,10 +296,14 @@ void FrameCaptionButtonContainerView::Layout() {
   if (tablet_mode_animation_->is_animating())
     AnimationProgressed(tablet_mode_animation_.get());
 
+#if DCHECK_IS_ON()
+  if (close_button_->GetVisible()) {
     // The top right corner must be occupied by the close button for easy mouse
     // access. This check is agnostic to RTL layout.
     DCHECK_EQ(close_button_->y(), 0);
     DCHECK_EQ(close_button_->bounds().right(), width());
+  }
+#endif  // DCHECK_IS_ON()
 }
 
 const char* FrameCaptionButtonContainerView::GetClassName() const {

chrome/browser/chromeos/BUILD.gn

@@ -1410,8 +1410,12 @@ source_set("chromeos") {
     "login/ui/login_display_webui.h",
     "login/ui/login_feedback.cc",
     "login/ui/login_feedback.h",
+    "login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.cc",
+    "login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h",
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.cc",
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h",
+    "login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.cc",
+    "login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.h",
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_window.cc",
     "login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h",
     "login/ui/login_web_dialog.cc",
@@ -2544,6 +2548,7 @@ source_set("unit_tests") {
     "login/signin_partition_manager_unittest.cc",
     "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/users/affiliation_unittest.cc",
     "login/users/multi_profile_user_controller_unittest.cc",
     "login/users/user_manager_unittest.cc",

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler.cc

@@ -7,6 +7,7 @@
 #include "ash/public/cpp/login_screen.h"
 #include "ash/public/cpp/login_screen_model.h"
 #include "ash/public/cpp/login_types.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
 #include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/ui/ash/login_screen_client.h"
@@ -90,7 +91,7 @@ bool LoginScreenExtensionUiHandler::Show(const extensions::Extension* extension,
         ash::OobeDialogState::EXTENSION_LOGIN);
   }
 
-  LoginScreenExtensionUiWindow::CreateOptions create_options(
+  LoginScreenExtensionUiCreateOptions create_options(
       extension->short_name(), extension->GetResourceURL(resource_path),
       can_be_closed_by_user,
       base::BindOnce(
@@ -166,4 +167,14 @@ void LoginScreenExtensionUiHandler::OnExtensionUninstalled(
   RemoveWindowForExtension(extension->id());
 }
 
+LoginScreenExtensionUiWindow*
+LoginScreenExtensionUiHandler::GetWindowForTesting(
+    const std::string& extension_id) {
+  WindowMap::iterator it = windows_.find(extension_id);
+  if (it == windows_.end())
+    return nullptr;
+
+  return it->second.get();
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler.h

@@ -67,6 +67,9 @@ class LoginScreenExtensionUiHandler
   // session_manager::SessionManagerObserver
   void OnSessionStateChanged() override;
 
+  LoginScreenExtensionUiWindow* GetWindowForTesting(
+      const std::string& extension_id);
+
  private:
   using WindowMap =
       std::map<std::string, std::unique_ptr<LoginScreenExtensionUiWindow>>;

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/test/gtest_util.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
 #include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h"
 #include "chrome/browser/ui/ash/test_login_screen.h"
 #include "chrome/common/chrome_constants.h"
@@ -50,7 +51,7 @@ class FakeLoginScreenExtensionUiWindowFactory
   ~FakeLoginScreenExtensionUiWindowFactory() override = default;
 
   std::unique_ptr<LoginScreenExtensionUiWindow> Create(
-      LoginScreenExtensionUiWindow::CreateOptions* create_options) override {
+      LoginScreenExtensionUiCreateOptions* create_options) override {
     create_was_called_ = true;
     last_extension_name_ = create_options->extension_name;
     last_content_url_ = create_options->content_url;

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_ui_apitest.cc

@@ -7,16 +7,22 @@
 
 #include "chrome/browser/chromeos/extensions/login_screen/login_screen_apitest_base.h"
 #include "chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h"
 #include "chrome/browser/chromeos/policy/signin_profile_extensions_policy_test_base.h"
 #include "components/version_info/version_info.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_delegate.h"
 
 namespace {
 
-const char kCanOpenWindow[] = "LoginScreenUiCanOpenWindow";
-const char kCannotOpenMultipleWindows[] =
+constexpr char kCanOpenWindow[] = "LoginScreenUiCanOpenWindow";
+constexpr char kCannotOpenMultipleWindows[] =
     "LoginScreenUiCannotOpenMultipleWindows";
-const char kCanOpenAndCloseWindow[] = "LoginScreenUiCanOpenAndCloseWindow";
-const char kCannotCloseNoWindow[] = "LoginScreenUiCannotCloseNoWindow";
+constexpr char kCanOpenAndCloseWindow[] = "LoginScreenUiCanOpenAndCloseWindow";
+constexpr char kCannotCloseNoWindow[] = "LoginScreenUiCannotCloseNoWindow";
+constexpr char kUserCanCloseWindow[] = "LoginScreenUiUserCanCloseWindow";
+constexpr char kUserCannotCloseWindow[] = "LoginScreenUiUserCannotCloseWindow";
 
 }  // namespace
 
@@ -28,11 +34,31 @@ class LoginScreenUiApitest : public LoginScreenApitestBase {
 
   ~LoginScreenUiApitest() override = default;
 
-  bool HasOpenWindow() {
-    LoginScreenExtensionUiHandler* ui_handler =
-        LoginScreenExtensionUiHandler::Get(false);
-    CHECK(ui_handler);
-    return ui_handler->HasOpenWindow(extension_id_);
+  bool HasOpenWindow() const {
+    return LoginScreenExtensionUiHandler::Get(false)->HasOpenWindow(
+        extension_id_);
+  }
+
+  bool CanCloseDialog() const {
+    return LoginScreenExtensionUiHandler::Get(false)
+        ->GetWindowForTesting(extension_id_)
+        ->GetDialogDelegateForTesting()
+        ->CanCloseDialog();
+  }
+
+  bool ShouldShowCloseButton() const {
+    return LoginScreenExtensionUiHandler::Get(false)
+        ->GetWindowForTesting(extension_id_)
+        ->GetDialogWidgetForTesting()
+        ->widget_delegate()
+        ->ShouldShowCloseButton();
+  }
+
+  bool IsMovementDisabled() const {
+    return LoginScreenExtensionUiHandler::Get(false)
+        ->GetWindowForTesting(extension_id_)
+        ->GetDialogWidgetForTesting()
+        ->movement_disabled();
   }
 
  private:
@@ -41,7 +67,11 @@ class LoginScreenUiApitest : public LoginScreenApitestBase {
 
 IN_PROC_BROWSER_TEST_F(LoginScreenUiApitest, ExtensionCanOpenWindow) {
   SetUpExtensionAndRunTest(kCanOpenWindow);
-  EXPECT_TRUE(HasOpenWindow());
+  ASSERT_TRUE(HasOpenWindow());
+  // userCanClose defaults to false
+  EXPECT_TRUE(IsMovementDisabled());
+  EXPECT_FALSE(CanCloseDialog());
+  EXPECT_FALSE(ShouldShowCloseButton());
 }
 
 IN_PROC_BROWSER_TEST_F(LoginScreenUiApitest,
@@ -60,4 +90,20 @@ IN_PROC_BROWSER_TEST_F(LoginScreenUiApitest, ExtensionCannotCloseNoWindow) {
   EXPECT_FALSE(HasOpenWindow());
 }
 
+IN_PROC_BROWSER_TEST_F(LoginScreenUiApitest, UserCanCloseWindow) {
+  SetUpExtensionAndRunTest(kUserCanCloseWindow);
+  ASSERT_TRUE(HasOpenWindow());
+  EXPECT_TRUE(IsMovementDisabled());
+  EXPECT_TRUE(CanCloseDialog());
+  EXPECT_TRUE(ShouldShowCloseButton());
+}
+
+IN_PROC_BROWSER_TEST_F(LoginScreenUiApitest, UserCannotCloseWindow) {
+  SetUpExtensionAndRunTest(kUserCannotCloseWindow);
+  ASSERT_TRUE(HasOpenWindow());
+  EXPECT_TRUE(IsMovementDisabled());
+  EXPECT_FALSE(CanCloseDialog());
+  EXPECT_FALSE(ShouldShowCloseButton());
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.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/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
+
+#include <memory>
+
+namespace chromeos {
+
+LoginScreenExtensionUiCreateOptions::LoginScreenExtensionUiCreateOptions(
+    const std::string& extension_name,
+    const GURL& content_url,
+    bool can_be_closed_by_user,
+    base::OnceClosure close_callback)
+    : extension_name(extension_name),
+      content_url(content_url),
+      can_be_closed_by_user(can_be_closed_by_user),
+      close_callback(std::move(close_callback)) {}
+
+LoginScreenExtensionUiCreateOptions::~LoginScreenExtensionUiCreateOptions() =
+    default;
+
+}  // namespace chromeos

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.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_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_CREATE_OPTIONS_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_CREATE_OPTIONS_H_
+
+#include <string>
+
+#include "base/callback.h"
+#include "url/gurl.h"
+
+namespace chromeos {
+
+struct LoginScreenExtensionUiCreateOptions {
+  LoginScreenExtensionUiCreateOptions(const std::string& extension_name,
+                                      const GURL& content_url,
+                                      bool can_be_closed_by_user,
+                                      base::OnceClosure close_callback);
+  ~LoginScreenExtensionUiCreateOptions();
+
+  const std::string extension_name;
+  const GURL content_url;
+  bool can_be_closed_by_user;
+  base::OnceClosure close_callback;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_CREATE_OPTIONS_H_

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.cc

@@ -5,10 +5,14 @@
 #include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_ui_message_handler.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
+#include "ui/gfx/geometry/size.h"
 
 namespace chromeos {
 
@@ -18,7 +22,7 @@ const double kRelativeScreenHeight = 0.8;
 }  // namespace
 
 LoginScreenExtensionUiDialogDelegate::LoginScreenExtensionUiDialogDelegate(
-    LoginScreenExtensionUiWindow::CreateOptions* create_options)
+    LoginScreenExtensionUiCreateOptions* create_options)
     : extension_name_(create_options->extension_name),
       content_url_(create_options->content_url),
       can_close_(create_options->can_be_closed_by_user),

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h

@@ -6,19 +6,34 @@
 #define CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_DIALOG_DELEGATE_H_
 
 #include <string>
+#include <vector>
 
-#include "base/callback_forward.h"
-#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h"
+#include "base/callback.h"
+#include "base/strings/string16.h"
+#include "ui/base/ui_base_types.h"
+#include "ui/gfx/native_widget_types.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
+#include "url/gurl.h"
+
+namespace content {
+class WebContents;
+class WebUIMessageHandler;
+}  // namespace content
+
+namespace gfx {
+class Size;
+}  // namespace gfx
 
 namespace chromeos {
 
+struct LoginScreenExtensionUiCreateOptions;
+
 // This class is used to provide data from a chrome.loginScreenUi API call to
 // the WebDialog.
 class LoginScreenExtensionUiDialogDelegate : public ui::WebDialogDelegate {
  public:
   explicit LoginScreenExtensionUiDialogDelegate(
-      LoginScreenExtensionUiWindow::CreateOptions* create_options);
+      LoginScreenExtensionUiCreateOptions* create_options);
   ~LoginScreenExtensionUiDialogDelegate() override;
 
   void set_can_close(bool can_close) { can_close_ = can_close; }

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate_unittest.cc

@@ -10,7 +10,7 @@
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler.h"
-#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/chrome_ash_test_base.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -41,7 +41,7 @@ TEST_F(LoginScreenExtensionUiDialogDelegateUnittest, Test) {
 
   base::RunLoop close_callback_wait;
 
-  LoginScreenExtensionUiWindow::CreateOptions create_options(
+  LoginScreenExtensionUiCreateOptions create_options(
       extension->short_name(), extension->GetResourceURL(kResourcePath),
       false /*can_be_closed_by_user*/, close_callback_wait.QuitClosure());
 

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.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/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.h"
+
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h"
+#include "content/public/browser/browser_context.h"
+
+namespace chromeos {
+
+LoginScreenExtensionUiWebDialogView::LoginScreenExtensionUiWebDialogView(
+    content::BrowserContext* context,
+    LoginScreenExtensionUiDialogDelegate* delegate,
+    std::unique_ptr<ui::WebDialogWebContentsDelegate::WebContentsHandler>
+        handler)
+    : views::WebDialogView(context, delegate, std::move(handler)),
+      delegate_(delegate) {}
+
+LoginScreenExtensionUiWebDialogView::~LoginScreenExtensionUiWebDialogView() =
+    default;
+
+bool LoginScreenExtensionUiWebDialogView::ShouldShowCloseButton() const {
+  return !delegate_ || delegate_->CanCloseDialog();
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.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_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_WEB_DIALOG_VIEW_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_WEB_DIALOG_VIEW_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "ui/views/controls/webview/web_dialog_view.h"
+#include "ui/web_dialogs/web_dialog_web_contents_delegate.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace chromeos {
+
+class LoginScreenExtensionUiDialogDelegate;
+
+// A WebDialogView used by chrome.loginScreenUi API calls. It hides the close
+// button if |LoginScreenExtensionUiDialogDelegate::CanCloseDialog()| is false.
+class LoginScreenExtensionUiWebDialogView : public views::WebDialogView {
+ public:
+  explicit LoginScreenExtensionUiWebDialogView(
+      content::BrowserContext* context,
+      LoginScreenExtensionUiDialogDelegate* delegate,
+      std::unique_ptr<ui::WebDialogWebContentsDelegate::WebContentsHandler>
+          handler);
+  ~LoginScreenExtensionUiWebDialogView() override;
+
+  bool ShouldShowCloseButton() const override;
+
+ private:
+  LoginScreenExtensionUiDialogDelegate* delegate_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(LoginScreenExtensionUiWebDialogView);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_WEB_DIALOG_VIEW_H_

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view_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/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.h"
+
+#include <memory>
+
+#include "base/bind_helpers.h"
+#include "base/macros.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h"
+#include "chrome/browser/ui/webui/chrome_web_contents_handler.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace chromeos {
+
+class LoginScreenExtensionUiWebDialogViewUnittest : public testing::Test {
+ public:
+  LoginScreenExtensionUiWebDialogViewUnittest() = default;
+  ~LoginScreenExtensionUiWebDialogViewUnittest() override = default;
+
+ protected:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfile profile;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LoginScreenExtensionUiWebDialogViewUnittest);
+};
+
+TEST_F(LoginScreenExtensionUiWebDialogViewUnittest, ShouldShowCloseButton) {
+  LoginScreenExtensionUiCreateOptions create_options(
+      "extension_name", GURL(),
+      /*can_be_closed_by_user=*/true, base::DoNothing());
+
+  std::unique_ptr<LoginScreenExtensionUiDialogDelegate> dialog_delegate =
+      std::make_unique<LoginScreenExtensionUiDialogDelegate>(&create_options);
+
+  std::unique_ptr<LoginScreenExtensionUiWebDialogView> dialog_view =
+      std::make_unique<LoginScreenExtensionUiWebDialogView>(
+          &profile, dialog_delegate.get(),
+          std::make_unique<ChromeWebContentsHandler>());
+
+  ASSERT_TRUE(dialog_view->ShouldShowCloseButton());
+}
+
+TEST_F(LoginScreenExtensionUiWebDialogViewUnittest, ShouldNotShowCloseButton) {
+  LoginScreenExtensionUiCreateOptions create_options(
+      "extension_name", GURL(),
+      /*can_be_closed_by_user=*/false, base::DoNothing());
+
+  std::unique_ptr<LoginScreenExtensionUiDialogDelegate> dialog_delegate =
+      std::make_unique<LoginScreenExtensionUiDialogDelegate>(&create_options);
+
+  std::unique_ptr<LoginScreenExtensionUiWebDialogView> dialog_view =
+      std::make_unique<LoginScreenExtensionUiWebDialogView>(
+          &profile, dialog_delegate.get(),
+          std::make_unique<ChromeWebContentsHandler>());
+
+  ASSERT_FALSE(dialog_view->ShouldShowCloseButton());
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.cc

@@ -5,7 +5,9 @@
 #include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h"
 
 #include "ash/public/cpp/shell_window_ids.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_create_options.h"
 #include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_dialog_delegate.h"
+#include "chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_web_dialog_view.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/webui/chrome_web_contents_handler.h"
@@ -14,18 +16,6 @@
 
 namespace chromeos {
 
-LoginScreenExtensionUiWindow::CreateOptions::CreateOptions(
-    const std::string& extension_name,
-    const GURL& content_url,
-    bool can_be_closed_by_user,
-    base::OnceClosure close_callback)
-    : extension_name(extension_name),
-      content_url(content_url),
-      can_be_closed_by_user(can_be_closed_by_user),
-      close_callback(std::move(close_callback)) {}
-
-LoginScreenExtensionUiWindow::CreateOptions::~CreateOptions() = default;
-
 LoginScreenExtensionUiWindowFactory::LoginScreenExtensionUiWindowFactory() =
     default;
 LoginScreenExtensionUiWindowFactory::~LoginScreenExtensionUiWindowFactory() =
@@ -33,24 +23,25 @@ LoginScreenExtensionUiWindowFactory::~LoginScreenExtensionUiWindowFactory() =
 
 std::unique_ptr<LoginScreenExtensionUiWindow>
 LoginScreenExtensionUiWindowFactory::Create(
-    LoginScreenExtensionUiWindow::CreateOptions* create_options) {
+    LoginScreenExtensionUiCreateOptions* create_options) {
   return std::make_unique<LoginScreenExtensionUiWindow>(create_options);
 }
 
 LoginScreenExtensionUiWindow::LoginScreenExtensionUiWindow(
-    CreateOptions* create_options) {
-  dialog_delegate_ = new LoginScreenExtensionUiDialogDelegate(create_options);
-
-  dialog_view_ = new views::WebDialogView(
-      ProfileHelper::GetSigninProfile(), dialog_delegate_,
-      std::make_unique<ChromeWebContentsHandler>());
-
+    LoginScreenExtensionUiCreateOptions* create_options)
+    : dialog_delegate_(
+          new LoginScreenExtensionUiDialogDelegate(create_options)),
+      dialog_view_(new LoginScreenExtensionUiWebDialogView(
+          ProfileHelper::GetSigninProfile(),
+          dialog_delegate_,
+          std::make_unique<ChromeWebContentsHandler>())) {
   dialog_widget_ = new views::Widget;
   views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW);
   params.delegate = dialog_view_;
   ash_util::SetupWidgetInitParamsForContainer(
       &params, ash::kShellWindowId_LockScreenContainer);
   dialog_widget_->Init(params);
+  dialog_widget_->set_movement_disabled(true);
   dialog_delegate_->set_native_window(dialog_widget_->GetNativeWindow());
   dialog_widget_->Show();
 }
@@ -60,4 +51,13 @@ LoginScreenExtensionUiWindow::~LoginScreenExtensionUiWindow() {
   dialog_widget_->Close();
 }
 
+LoginScreenExtensionUiDialogDelegate*
+LoginScreenExtensionUiWindow::GetDialogDelegateForTesting() {
+  return dialog_delegate_;
+}
+
+views::Widget* LoginScreenExtensionUiWindow::GetDialogWidgetForTesting() {
+  return dialog_widget_;
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/login/ui/login_screen_extension_ui/login_screen_extension_ui_window.h

@@ -5,44 +5,37 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_WINDOW_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_UI_LOGIN_SCREEN_EXTENSION_UI_LOGIN_SCREEN_EXTENSION_UI_WINDOW_H_
 
-#include <string>
+#include <memory>
 
-#include "base/callback_forward.h"
-#include "ui/web_dialogs/web_dialog_delegate.h"
+#include "base/macros.h"
 
 namespace views {
-class WebDialogView;
 class Widget;
 }  // namespace views
 
 namespace chromeos {
 
+struct LoginScreenExtensionUiCreateOptions;
 class LoginScreenExtensionUiDialogDelegate;
+class LoginScreenExtensionUiWebDialogView;
 
 // This class represents the window that can be created by the
 // chrome.loginScreenUi API. It manages the window's widget, view and delegate,
 // which are all automatically deleted when the widget closes.
+// The window is not draggable, and has a close button which is not visible
+// if |create_options.can_be_closed_by_user| is false.
 class LoginScreenExtensionUiWindow {
  public:
-  struct CreateOptions {
-    CreateOptions(const std::string& extension_name,
-                  const GURL& content_url,
-                  bool can_be_closed_by_user,
-                  base::OnceClosure close_callback);
-    ~CreateOptions();
-
-    const std::string extension_name;
-    const GURL content_url;
-    bool can_be_closed_by_user;
-    base::OnceClosure close_callback;
-  };
-
-  explicit LoginScreenExtensionUiWindow(CreateOptions* create_options);
+  explicit LoginScreenExtensionUiWindow(
+      LoginScreenExtensionUiCreateOptions* create_options);
   ~LoginScreenExtensionUiWindow();
 
+  LoginScreenExtensionUiDialogDelegate* GetDialogDelegateForTesting();
+  views::Widget* GetDialogWidgetForTesting();
+
  private:
   LoginScreenExtensionUiDialogDelegate* dialog_delegate_ = nullptr;
-  views::WebDialogView* dialog_view_ = nullptr;
+  LoginScreenExtensionUiWebDialogView* dialog_view_ = nullptr;
   views::Widget* dialog_widget_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(LoginScreenExtensionUiWindow);
@@ -54,7 +47,7 @@ class LoginScreenExtensionUiWindowFactory {
   virtual ~LoginScreenExtensionUiWindowFactory();
 
   virtual std::unique_ptr<LoginScreenExtensionUiWindow> Create(
-      LoginScreenExtensionUiWindow::CreateOptions* create_options);
+      LoginScreenExtensionUiCreateOptions* create_options);
 
   DISALLOW_COPY_AND_ASSIGN(LoginScreenExtensionUiWindowFactory);
 };

chrome/test/data/extensions/api_test/login_screen_apis/extension/background.js

@@ -42,6 +42,20 @@ const tests = {
       chrome.test.succeed();
     });
   },
+  'LoginScreenUiUserCanCloseWindow': () => {
+    chrome.loginScreenUi.show({url: 'some/path.html',
+        userCanClose: true}, () => {
+      chrome.test.assertNoLastError();
+      chrome.test.succeed();
+    });
+  },
+  'LoginScreenUiUserCannotCloseWindow': () => {
+    chrome.loginScreenUi.show({url: 'some/path.html',
+        userCanClose: false}, () => {
+      chrome.test.assertNoLastError();
+      chrome.test.succeed();
+    });
+  },
 
   /* Storage ******************************************************************/
   'StorageCannotAccessLocalStorage': () => {

chrome/test/data/extensions/api_test/login_screen_apis/extension/manifest.json

 {
   "name": "Login screen APIs test extension",
-  "version": "1.17",
+  "version": "1.18",
   "manifest_version": 2,
   "description": "Login screen APIs test extension",
   "background": {

chrome/test/data/extensions/api_test/login_screen_apis/update_manifest.xml

@@ -7,6 +7,6 @@
   <app appid='oclffehlkdgibkainkilopaalpdobkan'>
     <updatecheck
         codebase='http://mock.http/extensions/api_test/login_screen_apis/extension.crx'
-        version='1.17' />
+        version='1.18' />
    </app>
 </gupdate>