cros: Make the captive portal dialog closeable.

This CL moves the dialog host to its own class and widget which fills
the entire screen. This allows the captive portal dialog to be
interactible in any location it could be rendered in. We also add an
observer to CaptivePortalWindowProxy to allow OobeUIDialogDelegate to
show/hide the host widget when the captive portal dialog is about to
be shown or has just been hidden.

Bug: 870864
Change-Id: I898342795a3ecc950c6e4c273582962a2c70122b
Reviewed-on: https://chromium-review.googlesource.com/1187512
Commit-Queue: Quan Nguyen <qnnguyen@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586428}

chrome/browser/chromeos/login/screens/error_screen.cc

@@ -110,12 +110,8 @@ void ErrorScreen::AllowOfflineLogin(bool allowed) {
 }
 
 void ErrorScreen::FixCaptivePortal() {
-  if (!captive_portal_window_proxy_.get()) {
-    content::WebContents* web_contents =
-        LoginDisplayHost::default_host()->GetOobeWebContents();
-    captive_portal_window_proxy_.reset(new CaptivePortalWindowProxy(
-        network_state_informer_.get(), web_contents));
-  }
+  MaybeInitCaptivePortalWindowProxy(
+      LoginDisplayHost::default_host()->GetOobeWebContents());
   captive_portal_window_proxy_->ShowIfRedirected();
 }
 
@@ -180,6 +176,14 @@ ErrorScreen::RegisterConnectRequestCallback(const base::Closure& callback) {
   return connect_request_callbacks_.Add(callback);
 }
 
+void ErrorScreen::MaybeInitCaptivePortalWindowProxy(
+    content::WebContents* web_contents) {
+  if (!captive_portal_window_proxy_.get()) {
+    captive_portal_window_proxy_ = std::make_unique<CaptivePortalWindowProxy>(
+        network_state_informer_.get(), web_contents);
+  }
+}
+
 void ErrorScreen::Show() {
   if (!on_hide_callback_) {
     SetHideCallback(base::Bind(&ErrorScreen::DefaultHideCallback,

chrome/browser/chromeos/login/screens/error_screen.h

@@ -45,6 +45,10 @@ class ErrorScreen : public BaseScreen,
   ErrorScreen(BaseScreenDelegate* base_screen_delegate, NetworkErrorView* view);
   ~ErrorScreen() override;
 
+  CaptivePortalWindowProxy* captive_portal_window_proxy() {
+    return captive_portal_window_proxy_.get();
+  }
+
   // Toggles the guest sign-in prompt.
   void AllowGuestSignin(bool allowed);
 
@@ -95,6 +99,10 @@ class ErrorScreen : public BaseScreen,
   ConnectRequestCallbackSubscription RegisterConnectRequestCallback(
       const base::Closure& callback);
 
+  // Creates an instance of CaptivePortalWindowProxy, if one has not already
+  // been created.
+  void MaybeInitCaptivePortalWindowProxy(content::WebContents* web_contents);
+
   // BaseScreen overrides:
   void Show() override;
   void Hide() override;

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

@@ -72,6 +72,9 @@ void CaptivePortalWindowProxy::Show() {
   if (GetState() == STATE_DISPLAYED)  // Dialog is already shown, do nothing.
     return;
 
+  for (auto& observer : observers_)
+    observer.OnBeforeCaptivePortalShown();
+
   InitCaptivePortalView();
 
   CaptivePortalView* portal = captive_portal_view_.release();
@@ -90,6 +93,9 @@ void CaptivePortalWindowProxy::Close() {
     widget_->Close();
   captive_portal_view_.reset();
   captive_portal_view_for_testing_ = NULL;
+
+  for (auto& observer : observers_)
+    observer.OnAfterCaptivePortalHidden();
 }
 
 void CaptivePortalWindowProxy::OnRedirected() {
@@ -108,12 +114,23 @@ void CaptivePortalWindowProxy::OnOriginalURLLoaded() {
   Close();
 }
 
+void CaptivePortalWindowProxy::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void CaptivePortalWindowProxy::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void CaptivePortalWindowProxy::OnWidgetClosing(views::Widget* widget) {
   DCHECK(GetState() == STATE_DISPLAYED);
   DCHECK(widget == widget_);
 
   DetachFromWidget(widget);
 
+  for (auto& observer : observers_)
+    observer.OnAfterCaptivePortalHidden();
+
   DCHECK(GetState() == STATE_IDLE);
 }
 

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

@@ -9,6 +9,8 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
 #include "base/time/time.h"
 #include "ui/views/widget/widget_observer.h"
 
@@ -37,6 +39,19 @@ class CaptivePortalWindowProxyDelegate {
 // Proxy which manages showing of the window for CaptivePortal sign-in.
 class CaptivePortalWindowProxy : public views::WidgetObserver {
  public:
+  // Observer interface for CaptivePortalWindowProxy that gets notified when the
+  // CaptivePortal widget is shown or hidden/closed.
+  class Observer : public base::CheckedObserver {
+   public:
+    Observer() = default;
+    ~Observer() override = default;
+    virtual void OnBeforeCaptivePortalShown() {}
+    virtual void OnAfterCaptivePortalHidden() {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Observer);
+  };
+
   typedef CaptivePortalWindowProxyDelegate Delegate;
 
   CaptivePortalWindowProxy(Delegate* delegate,
@@ -64,6 +79,9 @@ class CaptivePortalWindowProxy : public views::WidgetObserver {
   // redirections.
   void OnOriginalURLLoaded();
 
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   // Overridden from views::WidgetObserver:
   void OnWidgetClosing(views::Widget* widget) override;
   void OnWidgetDestroying(views::Widget* widget) override;
@@ -118,6 +136,8 @@ class CaptivePortalWindowProxy : public views::WidgetObserver {
 
   base::Time started_loading_at_;
 
+  base::ObserverList<Observer> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(CaptivePortalWindowProxy);
 };
 

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

@@ -109,7 +109,7 @@ void LoginDisplayHostMojo::HandleDisplayCaptivePortal() {
   if (dialog_->IsVisible())
     GetOobeUI()->GetErrorScreen()->FixCaptivePortal();
   else
-    should_display_captive_portal_ = true;
+    dialog_->SetShouldDisplayCaptivePortal(true);
 }
 
 LoginDisplay* LoginDisplayHostMojo::GetLoginDisplay() {
@@ -278,11 +278,6 @@ void LoginDisplayHostMojo::ShowGaiaDialog(
   }
 
   dialog_->Show();
-
-  if (should_display_captive_portal_) {
-    GetOobeUI()->GetErrorScreen()->FixCaptivePortal();
-    should_display_captive_portal_ = false;
-  }
 }
 
 void LoginDisplayHostMojo::HideOobeDialog() {

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

@@ -162,10 +162,6 @@ class LoginDisplayHostMojo : public LoginDisplayHostCommon,
   // first OnStartSigninScreen and remains true afterward.
   bool signin_screen_started_ = false;
 
-  // Whether the captive portal screen should be shown the next time the Gaia
-  // dialog is opened.
-  bool should_display_captive_portal_ = false;
-
   base::WeakPtrFactory<LoginDisplayHostMojo> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginDisplayHostMojo);

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

@@ -62,13 +62,113 @@ class OobeWebDialogView : public views::WebDialogView {
   }
 };
 
+class CaptivePortalDialogDelegate
+    : public ui::WebDialogDelegate,
+      public ChromeWebModalDialogManagerDelegate,
+      public web_modal::WebContentsModalDialogHost {
+ public:
+  explicit CaptivePortalDialogDelegate(content::WebContents* web_contents)
+      : web_contents_(web_contents) {
+    view_ = new views::WebDialogView(ProfileHelper::GetSigninProfile(), this,
+                                     new ChromeWebContentsHandler);
+    view_->SetVisible(false);
+
+    views::Widget::InitParams params(
+        views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
+    params.delegate = view_;
+    params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
+    ash_util::SetupWidgetInitParamsForContainer(
+        &params, ash::kShellWindowId_LockSystemModalContainer);
+
+    widget_ = new views::Widget;
+    widget_->Init(params);
+    widget_->SetBounds(display::Screen::GetScreen()
+                           ->GetDisplayNearestWindow(widget_->GetNativeWindow())
+                           .work_area());
+    widget_->SetOpacity(0);
+
+    // Set this as the web modal delegate so that captive portal dialog can
+    // appear.
+    web_modal::WebContentsModalDialogManager::CreateForWebContents(
+        web_contents_);
+    web_modal::WebContentsModalDialogManager::FromWebContents(web_contents_)
+        ->SetDelegate(this);
+  }
+
+  void Show() { widget_->Show(); }
+
+  void Hide() { widget_->Hide(); }
+
+  web_modal::WebContentsModalDialogHost* GetWebContentsModalDialogHost()
+      override {
+    return this;
+  }
+
+  // web_modal::WebContentsModalDialogHost:
+  gfx::NativeView GetHostView() const override {
+    return widget_->GetNativeWindow();
+  }
+
+  gfx::Point GetDialogPosition(const gfx::Size& size) override {
+    // Center the dialog in the screen.
+    gfx::Size host_size = GetHostView()->GetBoundsInScreen().size();
+    return gfx::Point(host_size.width() / 2 - size.width() / 2,
+                      host_size.height() / 2 - size.height() / 2);
+  }
+
+  gfx::Size GetMaximumDialogSize() override {
+    return display::Screen::GetScreen()
+        ->GetDisplayNearestWindow(widget_->GetNativeWindow())
+        .work_area()
+        .size();
+  }
+
+  void AddObserver(web_modal::ModalDialogHostObserver* observer) override {}
+
+  void RemoveObserver(web_modal::ModalDialogHostObserver* observer) override {}
+
+  // ui::WebDialogDelegate:
+  ui::ModalType GetDialogModalType() const override {
+    return ui::ModalType::MODAL_TYPE_SYSTEM;
+  }
+
+  base::string16 GetDialogTitle() const override { return base::string16(); }
+
+  GURL GetDialogContentURL() const override { return GURL(); }
+
+  void GetWebUIMessageHandlers(
+      std::vector<content::WebUIMessageHandler*>* handlers) const override {}
+
+  void GetDialogSize(gfx::Size* size) const override {
+    *size = display::Screen::GetScreen()
+                ->GetDisplayNearestWindow(widget_->GetNativeWindow())
+                .work_area()
+                .size();
+  }
+
+  std::string GetDialogArgs() const override { return std::string(); }
+
+  void OnDialogClosed(const std::string& json_retval) override { delete this; }
+
+  void OnCloseContents(content::WebContents* source,
+                       bool* out_close_dialog) override {
+    *out_close_dialog = false;
+  }
+
+  bool ShouldShowDialogTitle() const override { return false; }
+
+ private:
+  views::Widget* widget_ = nullptr;
+  views::WebDialogView* view_ = nullptr;
+  content::WebContents* web_contents_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(CaptivePortalDialogDelegate);
+};
+
 OobeUIDialogDelegate::OobeUIDialogDelegate(
     base::WeakPtr<LoginDisplayHostMojo> controller)
     : controller_(controller),
-      size_(gfx::Size(kGaiaDialogWidth, kGaiaDialogHeight)),
-      display_observer_(this),
-      tablet_mode_observer_(this),
-      keyboard_observer_(this) {
+      size_(gfx::Size(kGaiaDialogWidth, kGaiaDialogHeight)) {
   display_observer_.Add(display::Screen::GetScreen());
   tablet_mode_observer_.Add(TabletModeClient::Get());
   // TODO(crbug.com/646565): Support virtual keyboard under MASH. There is no
@@ -102,12 +202,13 @@ OobeUIDialogDelegate::OobeUIDialogDelegate(
   extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
       dialog_view_->web_contents());
 
-  // Set this as the web modal delegate so that captive portal dialog can
-  // appear.
-  web_modal::WebContentsModalDialogManager::CreateForWebContents(
-      GetWebContents());
-  web_modal::WebContentsModalDialogManager::FromWebContents(GetWebContents())
-      ->SetDelegate(this);
+  captive_portal_delegate_ =
+      new CaptivePortalDialogDelegate(dialog_view_->web_contents());
+
+  GetOobeUI()->GetErrorScreen()->MaybeInitCaptivePortalWindowProxy(
+      dialog_view_->web_contents());
+  captive_portal_observer_.Add(
+      GetOobeUI()->GetErrorScreen()->captive_portal_window_proxy());
 }
 
 OobeUIDialogDelegate::~OobeUIDialogDelegate() {
@@ -123,9 +224,16 @@ bool OobeUIDialogDelegate::IsVisible() {
   return dialog_widget_->IsVisible();
 }
 
+void OobeUIDialogDelegate::SetShouldDisplayCaptivePortal(bool should_display) {
+  should_display_captive_portal_ = should_display;
+}
+
 void OobeUIDialogDelegate::Show() {
   dialog_widget_->Show();
   SetState(ash::mojom::OobeDialogState::GAIA_SIGNIN);
+
+  if (should_display_captive_portal_)
+    GetOobeUI()->GetErrorScreen()->FixCaptivePortal();
 }
 
 void OobeUIDialogDelegate::ShowFullScreen() {
@@ -207,35 +315,6 @@ gfx::NativeWindow OobeUIDialogDelegate::GetNativeWindow() const {
   return dialog_widget_ ? dialog_widget_->GetNativeWindow() : nullptr;
 }
 
-web_modal::WebContentsModalDialogHost*
-OobeUIDialogDelegate::GetWebContentsModalDialogHost() {
-  return this;
-}
-
-gfx::NativeView OobeUIDialogDelegate::GetHostView() const {
-  return GetNativeWindow();
-}
-
-gfx::Point OobeUIDialogDelegate::GetDialogPosition(const gfx::Size& size) {
-  // Center the dialog in the screen.
-  gfx::Size host_size = GetHostView()->GetBoundsInScreen().size();
-  return gfx::Point(host_size.width() / 2 - size.width() / 2,
-                    host_size.height() / 2 - size.height() / 2);
-}
-
-gfx::Size OobeUIDialogDelegate::GetMaximumDialogSize() {
-  return display::Screen::GetScreen()
-      ->GetDisplayNearestWindow(GetNativeWindow())
-      .work_area()
-      .size();
-}
-
-void OobeUIDialogDelegate::AddObserver(
-    web_modal::ModalDialogHostObserver* observer) {}
-
-void OobeUIDialogDelegate::RemoveObserver(
-    web_modal::ModalDialogHostObserver* observer) {}
-
 void OobeUIDialogDelegate::OnDisplayMetricsChanged(
     const display::Display& display,
     uint32_t changed_metrics) {
@@ -328,4 +407,19 @@ void OobeUIDialogDelegate::OnKeyboardVisibilityStateChanged(bool is_visible) {
   UpdateSizeAndPosition(size_.width(), size_.height());
 }
 
+void OobeUIDialogDelegate::OnBeforeCaptivePortalShown() {
+  should_display_captive_portal_ = false;
+
+  captive_portal_delegate_->Show();
+}
+
+void OobeUIDialogDelegate::OnAfterCaptivePortalHidden() {
+  // If the captive portal state went from hidden -> visible -> back to hidden
+  // while the OOBE dialog was not shown, we should not attempt to load the
+  // captive portal next time the OOBE dialog pops up.
+  should_display_captive_portal_ = false;
+
+  captive_portal_delegate_->Hide();
+}
+
 }  // namespace chromeos

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

@@ -44,6 +44,8 @@ namespace chromeos {
 class LoginDisplayHostMojo;
 class OobeUI;
 
+class CaptivePortalDialogDelegate;
+
 // This class manages the behavior of the Oobe UI dialog.
 // And its lifecycle is managed by the widget created in Show().
 //   WebDialogView<----delegate_----OobeUIDialogDelegate
@@ -54,9 +56,8 @@ class OobeUI;
 class OobeUIDialogDelegate : public display::DisplayObserver,
                              public TabletModeClientObserver,
                              public ui::WebDialogDelegate,
-                             public ChromeWebModalDialogManagerDelegate,
-                             public web_modal::WebContentsModalDialogHost,
-                             public keyboard::KeyboardControllerObserver {
+                             public keyboard::KeyboardControllerObserver,
+                             public CaptivePortalWindowProxy::Observer {
  public:
   explicit OobeUIDialogDelegate(base::WeakPtr<LoginDisplayHostMojo> controller);
   ~OobeUIDialogDelegate() override;
@@ -79,23 +80,15 @@ class OobeUIDialogDelegate : public display::DisplayObserver,
   // Update the oobe state of the dialog.
   void SetState(ash::mojom::OobeDialogState state);
 
+  // Tell the dialog whether to call FixCaptivePortal next time it is shown.
+  void SetShouldDisplayCaptivePortal(bool should_display);
+
   content::WebContents* GetWebContents();
 
   void UpdateSizeAndPosition(int width, int height);
   OobeUI* GetOobeUI() const;
   gfx::NativeWindow GetNativeWindow() const;
 
-  // ChromeWebModalDialogManagerDelegate:
-  web_modal::WebContentsModalDialogHost* GetWebContentsModalDialogHost()
-      override;
-
-  // web_modal::WebContentsModalDialogHost:
-  gfx::NativeView GetHostView() const override;
-  gfx::Point GetDialogPosition(const gfx::Size& size) override;
-  gfx::Size GetMaximumDialogSize() override;
-  void AddObserver(web_modal::ModalDialogHostObserver* observer) override;
-  void RemoveObserver(web_modal::ModalDialogHostObserver* observer) override;
-
  private:
   // display::DisplayObserver:
   void OnDisplayMetricsChanged(const display::Display& display,
@@ -124,8 +117,14 @@ class OobeUIDialogDelegate : public display::DisplayObserver,
   // keyboard::KeyboardControllerObserver:
   void OnKeyboardVisibilityStateChanged(bool is_visible) override;
 
+  // CaptivePortalWindowProxy::Observer:
+  void OnBeforeCaptivePortalShown() override;
+  void OnAfterCaptivePortalHidden() override;
+
   base::WeakPtr<LoginDisplayHostMojo> controller_;
 
+  CaptivePortalDialogDelegate* captive_portal_delegate_ = nullptr;
+
   // This is owned by the underlying native widget.
   // Before its deletion, onDialogClosed will get called and delete this object.
   views::Widget* dialog_widget_ = nullptr;
@@ -133,15 +132,22 @@ class OobeUIDialogDelegate : public display::DisplayObserver,
   gfx::Size size_;
   bool showing_fullscreen_ = false;
 
-  ScopedObserver<display::Screen, display::DisplayObserver> display_observer_;
+  ScopedObserver<display::Screen, display::DisplayObserver> display_observer_{
+      this};
   ScopedObserver<TabletModeClient, TabletModeClientObserver>
-      tablet_mode_observer_;
+      tablet_mode_observer_{this};
   ScopedObserver<keyboard::KeyboardController, KeyboardControllerObserver>
-      keyboard_observer_;
+      keyboard_observer_{this};
+  ScopedObserver<CaptivePortalWindowProxy, OobeUIDialogDelegate>
+      captive_portal_observer_{this};
 
   std::map<ui::Accelerator, std::string> accel_map_;
   ash::mojom::OobeDialogState state_ = ash::mojom::OobeDialogState::HIDDEN;
 
+  // Whether the captive portal screen should be shown the next time the Gaia
+  // dialog is opened.
+  bool should_display_captive_portal_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(OobeUIDialogDelegate);
 };