cros: Move keep_alive and drag'n'drop to LoginDisplayHostCommon.

- keep_alive ensures we do not exit/shutdown.
- drag'n'drop disables drag and drop.

Bug: 784495
Change-Id: I65525cf5065db48bc14db93af3f3b6e86bc715c7
Reviewed-on: https://chromium-review.googlesource.com/871835
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534836}

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

@@ -4,7 +4,9 @@
 
 #include "chrome/browser/chromeos/login/ui/login_display_host_common.h"
 
+#include "ash/shell.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/app_launch_controller.h"
 #include "chrome/browser/chromeos/login/arc_kiosk_controller.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_app_launcher.h"
@@ -13,7 +15,12 @@
 #include "chrome/browser/chromeos/mobile_config.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/system/device_disabling_manager.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/wallpaper_controller_client.h"
+#include "chrome/browser/ui/webui/chromeos/internet_detail_dialog.h"
+#include "components/keep_alive_registry/keep_alive_types.h"
+#include "components/keep_alive_registry/scoped_keep_alive.h"
+#include "ui/wm/public/scoped_drag_drop_disabler.h"
 
 namespace chromeos {
 namespace {
@@ -25,10 +32,36 @@ constexpr int64_t kPolicyServiceInitializationDelayMilliseconds = 100;
 
 }  // namespace
 
-LoginDisplayHostCommon::LoginDisplayHostCommon() : weak_factory_(this) {}
+LoginDisplayHostCommon::LoginDisplayHostCommon() : weak_factory_(this) {
+  keep_alive_.reset(
+      new ScopedKeepAlive(KeepAliveOrigin::LOGIN_DISPLAY_HOST_WEBUI,
+                          KeepAliveRestartOption::DISABLED));
+
+  // Disable Drag'n'Drop for the login session.
+  // ash::Shell may be null in tests.
+  if (ash::Shell::HasInstance() && !ash_util::IsRunningInMash()) {
+    scoped_drag_drop_disabler_.reset(
+        new wm::ScopedDragDropDisabler(ash::Shell::GetPrimaryRootWindow()));
+  } else {
+    NOTIMPLEMENTED();
+  }
+
+  // Close the login screen on NOTIFICATION_APP_TERMINATING.
+  registrar_.Add(this, chrome::NOTIFICATION_APP_TERMINATING,
+                 content::NotificationService::AllSources());
+  // NOTIFICATION_BROWSER_OPENED is issued after browser is created, but
+  // not shown yet. Lock window has to be closed at this point so that
+  // a browser window exists and the window can acquire input focus.
+  registrar_.Add(this, chrome::NOTIFICATION_BROWSER_OPENED,
+                 content::NotificationService::AllSources());
+}
 
 LoginDisplayHostCommon::~LoginDisplayHostCommon() {}
 
+void LoginDisplayHostCommon::BeforeSessionStart() {
+  session_starting_ = true;
+}
+
 AppLaunchController* LoginDisplayHostCommon::GetAppLaunchController() {
   return app_launch_controller_.get();
 }
@@ -127,10 +160,6 @@ void LoginDisplayHostCommon::StartArcKiosk(const AccountId& account_id) {
   OnStartArcKiosk();
 }
 
-void LoginDisplayHostCommon::OnAuthPrewarmDone() {
-  auth_prewarmer_.reset();
-}
-
 void LoginDisplayHostCommon::CompleteLogin(const UserContext& user_context) {
   ExistingUserController* controller =
       ExistingUserController::current_controller();
@@ -177,4 +206,34 @@ bool LoginDisplayHostCommon::IsUserWhitelisted(const AccountId& account_id) {
   return controller->IsUserWhitelisted(account_id);
 }
 
+void LoginDisplayHostCommon::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  if (type == chrome::NOTIFICATION_APP_TERMINATING) {
+    ShutdownDisplayHost();
+  } else if (type == chrome::NOTIFICATION_BROWSER_OPENED && session_starting_) {
+    // Browsers created before session start (windows opened by extensions, for
+    // example) are ignored.
+    OnBrowserCreated();
+    registrar_.Remove(this, chrome::NOTIFICATION_APP_TERMINATING,
+                      content::NotificationService::AllSources());
+    registrar_.Remove(this, chrome::NOTIFICATION_BROWSER_OPENED,
+                      content::NotificationService::AllSources());
+  }
+}
+
+void LoginDisplayHostCommon::OnAuthPrewarmDone() {
+  auth_prewarmer_.reset();
+}
+
+void LoginDisplayHostCommon::ShutdownDisplayHost() {
+  if (shutting_down_)
+    return;
+
+  shutting_down_ = true;
+  registrar_.RemoveAll();
+  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
+}
+
 }  // namespace chromeos

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

@@ -10,8 +10,14 @@
 #include <vector>
 
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
+#include "content/public/browser/notification_observer.h"
 
 class AccountId;
+class ScopedKeepAlive;
+
+namespace wm {
+class ScopedDragDropDisabler;
+}
 
 namespace chromeos {
 
@@ -21,12 +27,14 @@ class DemoAppLauncher;
 // LoginDisplayHostCommon contains code which is not specific to a particular UI
 // implementation - the goal is to reduce code duplication between
 // LoginDisplayHostViews and LoginDisplayHostWebUI.
-class LoginDisplayHostCommon : public LoginDisplayHost {
+class LoginDisplayHostCommon : public LoginDisplayHost,
+                               public content::NotificationObserver {
  public:
   LoginDisplayHostCommon();
   ~LoginDisplayHostCommon() override;
 
   // LoginDisplayHost:
+  void BeforeSessionStart() final;
   AppLaunchController* GetAppLaunchController() final;
   void StartSignInScreen(const LoginScreenContext& context) final;
   void PrewarmAuthentication() final;
@@ -44,14 +52,23 @@ class LoginDisplayHostCommon : public LoginDisplayHost {
   void LoadSigninWallpaper() final;
   bool IsUserWhitelisted(const AccountId& account_id) final;
 
+  // content::NotificationObserver:
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) override;
+
  protected:
   virtual void OnStartSignInScreen(const LoginScreenContext& context) = 0;
   virtual void OnStartAppLaunch() = 0;
   virtual void OnStartArcKiosk() = 0;
+  virtual void OnBrowserCreated() = 0;
 
   // Deletes |auth_prewarmer_|.
   void OnAuthPrewarmDone();
 
+  // Marks display host for deletion.
+  void ShutdownDisplayHost();
+
   // Active instance of authentication prewarmer.
   std::unique_ptr<AuthPrewarmer> auth_prewarmer_;
 
@@ -64,7 +81,24 @@ class LoginDisplayHostCommon : public LoginDisplayHost {
   // ARC kiosk controller.
   std::unique_ptr<ArcKioskController> arc_kiosk_controller_;
 
+  content::NotificationRegistrar registrar_;
+
  private:
+  // True if session start is in progress.
+  bool session_starting_ = false;
+
+  // Has ShutdownDisplayHost() already been called?  Used to avoid posting our
+  // own deletion to the message loop twice if the user logs out while we're
+  // still in the process of cleaning up after login (http://crbug.com/134463).
+  bool shutting_down_ = false;
+
+  // Make sure chrome won't exit while we are at login/oobe screen.
+  std::unique_ptr<ScopedKeepAlive> keep_alive_;
+
+  // Keeps a copy of the old Drag'n'Drop client, so that it would be disabled
+  // during a login session and restored afterwards.
+  std::unique_ptr<wm::ScopedDragDropDisabler> scoped_drag_drop_disabler_;
+
   base::WeakPtrFactory<LoginDisplayHostCommon> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginDisplayHostCommon);

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

@@ -56,10 +56,6 @@ WebUILoginView* LoginDisplayHostViews::GetWebUILoginView() const {
   return nullptr;
 }
 
-void LoginDisplayHostViews::BeforeSessionStart() {
-  NOTIMPLEMENTED();
-}
-
 void LoginDisplayHostViews::Finalize(base::OnceClosure completion_callback) {
   completion_callbacks_.push_back(std::move(completion_callback));
   base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
@@ -123,6 +119,10 @@ void LoginDisplayHostViews::OnStartArcKiosk() {
   NOTIMPLEMENTED();
 }
 
+void LoginDisplayHostViews::OnBrowserCreated() {
+  NOTIMPLEMENTED();
+}
+
 void LoginDisplayHostViews::StartVoiceInteractionOobe() {
   NOTIMPLEMENTED();
 }

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

@@ -33,7 +33,6 @@ class LoginDisplayHostViews : public LoginDisplayHostCommon,
   gfx::NativeWindow GetNativeWindow() const override;
   OobeUI* GetOobeUI() const override;
   WebUILoginView* GetWebUILoginView() const override;
-  void BeforeSessionStart() override;
   void Finalize(base::OnceClosure completion_callback) override;
   void SetStatusAreaVisible(bool visible) override;
   void StartWizard(OobeScreen first_screen) override;
@@ -44,6 +43,7 @@ class LoginDisplayHostViews : public LoginDisplayHostCommon,
   void OnPreferencesChanged() override;
   void OnStartAppLaunch() override;
   void OnStartArcKiosk() override;
+  void OnBrowserCreated() override;
   void StartVoiceInteractionOobe() override;
   bool IsVoiceInteractionOobe() override;
 

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

@@ -72,8 +72,6 @@
 #include "chromeos/settings/cros_settings_provider.h"
 #include "chromeos/settings/timezone_settings.h"
 #include "chromeos/timezone/timezone_resolver.h"
-#include "components/keep_alive_registry/keep_alive_types.h"
-#include "components/keep_alive_registry/scoped_keep_alive.h"
 #include "components/language/core/common/locale_util.h"
 #include "components/prefs/pref_service.h"
 #include "components/session_manager/core/session_manager.h"
@@ -434,24 +432,10 @@ LoginDisplayHostWebUI::LoginDisplayHostWebUI(const gfx::Rect& wallpaper_bounds)
 
   ui::InputDeviceManager::GetInstance()->AddObserver(this);
 
-  // Close the login screen on NOTIFICATION_APP_TERMINATING.
-  registrar_.Add(this, chrome::NOTIFICATION_APP_TERMINATING,
-                 content::NotificationService::AllSources());
-
-  // NOTIFICATION_BROWSER_OPENED is issued after browser is created, but
-  // not shown yet. Lock window has to be closed at this point so that
-  // a browser window exists and the window can acquire input focus.
-  registrar_.Add(this, chrome::NOTIFICATION_BROWSER_OPENED,
-                 content::NotificationService::AllSources());
-
   // Login screen is moved to lock screen container when user logs in.
   registrar_.Add(this, chrome::NOTIFICATION_LOGIN_USER_CHANGED,
                  content::NotificationService::AllSources());
 
-  keep_alive_.reset(
-      new ScopedKeepAlive(KeepAliveOrigin::LOGIN_DISPLAY_HOST_WEBUI,
-                          KeepAliveRestartOption::DISABLED));
-
   bool zero_delay_enabled = WizardController::IsZeroDelayEnabled();
   // Mash always runs login screen with zero delay
   if (ash_util::IsRunningInMash())
@@ -505,14 +489,6 @@ LoginDisplayHostWebUI::LoginDisplayHostWebUI(const gfx::Rect& wallpaper_bounds)
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
   manager->Initialize(chromeos::SOUND_STARTUP,
                       bundle.GetRawDataResource(IDR_SOUND_STARTUP_WAV));
-
-  // Disable Drag'n'Drop for the login session.
-  if (!ash_util::IsRunningInMash()) {
-    scoped_drag_drop_disabler_.reset(
-        new wm::ScopedDragDropDisabler(ash::Shell::GetPrimaryRootWindow()));
-  } else {
-    NOTIMPLEMENTED();
-  }
 }
 
 LoginDisplayHostWebUI::~LoginDisplayHostWebUI() {
@@ -542,8 +518,6 @@ LoginDisplayHostWebUI::~LoginDisplayHostWebUI() {
 
   ScheduleCompletionCallbacks(std::move(completion_callbacks_));
 
-  keep_alive_.reset();
-
   // TODO(tengs): This should be refactored. See crbug.com/314934.
   if (user_manager::UserManager::Get()->IsCurrentUserNew()) {
     // DriveOptInController will delete itself when finished.
@@ -569,10 +543,6 @@ WebUILoginView* LoginDisplayHostWebUI::GetWebUILoginView() const {
   return login_view_;
 }
 
-void LoginDisplayHostWebUI::BeforeSessionStart() {
-  session_starting_ = true;
-}
-
 void LoginDisplayHostWebUI::Finalize(base::OnceClosure completion_callback) {
   DVLOG(1) << "Finalizing LoginDisplayHost. User session starting";
 
@@ -816,6 +786,8 @@ void LoginDisplayHostWebUI::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
+  LoginDisplayHostCommon::Observe(type, source, details);
+
   if (chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE == type ||
       chrome::NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN == type) {
     VLOG(1) << "Login WebUI >> WEBUI_VISIBLE";
@@ -828,16 +800,6 @@ void LoginDisplayHostWebUI::Observe(
                       content::NotificationService::AllSources());
     registrar_.Remove(this, chrome::NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN,
                       content::NotificationService::AllSources());
-  } else if (type == chrome::NOTIFICATION_APP_TERMINATING) {
-    ShutdownDisplayHost();
-  } else if (type == chrome::NOTIFICATION_BROWSER_OPENED && session_starting_) {
-    // Browsers created before session start (windows opened by extensions, for
-    // example) are ignored.
-    OnBrowserCreated();
-    registrar_.Remove(this, chrome::NOTIFICATION_APP_TERMINATING,
-                      content::NotificationService::AllSources());
-    registrar_.Remove(this, chrome::NOTIFICATION_BROWSER_OPENED,
-                      content::NotificationService::AllSources());
   } else if (type == chrome::NOTIFICATION_LOGIN_USER_CHANGED &&
              user_manager::UserManager::Get()->IsCurrentUserNew()) {
     registrar_.Remove(this, chrome::NOTIFICATION_LOGIN_USER_CHANGED,
@@ -967,15 +929,6 @@ void LoginDisplayHostWebUI::OnUserSwitchAnimationFinished() {
 ////////////////////////////////////////////////////////////////////////////////
 // LoginDisplayHostWebUI, private
 
-void LoginDisplayHostWebUI::ShutdownDisplayHost() {
-  if (shutting_down_)
-    return;
-
-  shutting_down_ = true;
-  registrar_.RemoveAll();
-  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
-}
-
 void LoginDisplayHostWebUI::ScheduleWorkspaceAnimation() {
   if (ash_util::IsRunningInMash()) {
     NOTIMPLEMENTED();

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

@@ -29,9 +29,6 @@
 #include "ui/events/devices/input_device_event_observer.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/views/widget/widget_removals_observer.h"
-#include "ui/wm/public/scoped_drag_drop_disabler.h"
-
-class ScopedKeepAlive;
 
 namespace ash {
 class FocusRingController;
@@ -45,7 +42,6 @@ class WebUILoginView;
 // An implementation class for OOBE/login WebUI screen host.
 // It encapsulates controllers, wallpaper integration and flow.
 class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
-                              public content::NotificationObserver,
                               public content::WebContentsObserver,
                               public chromeos::SessionManagerClient::Observer,
                               public chromeos::CrasAudioHandler::AudioObserver,
@@ -62,7 +58,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
   gfx::NativeWindow GetNativeWindow() const override;
   OobeUI* GetOobeUI() const override;
   WebUILoginView* GetWebUILoginView() const override;
-  void BeforeSessionStart() override;
   void Finalize(base::OnceClosure completion_callback) override;
   void SetStatusAreaVisible(bool visible) override;
   void StartWizard(OobeScreen first_screen) override;
@@ -75,13 +70,11 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
   void OnStartArcKiosk() override;
   bool IsVoiceInteractionOobe() override;
   void StartVoiceInteractionOobe() override;
+  void OnBrowserCreated() override;
 
   // Creates WizardController instance.
   WizardController* CreateWizardController();
 
-  // Called when the first browser window is created, but before it's shown.
-  void OnBrowserCreated();
-
   const gfx::Rect& wallpaper_bounds() const { return wallpaper_bounds_; }
 
   // Trace id for ShowLoginWebUI event (since there exists at most one login
@@ -96,32 +89,32 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
  protected:
   class KeyboardDrivenOobeKeyHandler;
 
-  // content::NotificationObserver implementation:
+  // LoginDisplayHost:
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // Overridden from content::WebContentsObserver:
+  // content::WebContentsObserver:
   void RenderProcessGone(base::TerminationStatus status) override;
 
-  // Overridden from chromeos::SessionManagerClient::Observer:
+  // chromeos::SessionManagerClient::Observer:
   void EmitLoginPromptVisibleCalled() override;
 
-  // Overridden from chromeos::CrasAudioHandler::AudioObserver:
+  // chromeos::CrasAudioHandler::AudioObserver:
   void OnActiveOutputNodeChanged() override;
 
-  // Overridden from display::DisplayObserver:
+  // display::DisplayObserver:
   void OnDisplayAdded(const display::Display& new_display) override;
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override;
 
-  // Overridden from ui::InputDeviceEventObserver
+  // ui::InputDeviceEventObserver
   void OnTouchscreenDeviceConfigurationChanged() override;
 
-  // Overridden from views::WidgetRemovalsObserver:
+  // views::WidgetRemovalsObserver:
   void OnWillRemoveView(views::Widget* widget, views::View* view) override;
 
-  // Overridden from chrome::MultiUserWindowManager::Observer:
+  // chrome::MultiUserWindowManager::Observer:
   void OnUserSwitchAnimationFinished() override;
 
  private:
@@ -145,9 +138,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
                           // adding a user into multi-profile session.
   };
 
-  // Marks display host for deletion.
-  void ShutdownDisplayHost();
-
   // Schedules workspace transition animation.
   void ScheduleWorkspaceAnimation();
 
@@ -184,8 +174,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
   // Used to calculate position of the screens and wallpaper.
   gfx::Rect wallpaper_bounds_;
 
-  content::NotificationRegistrar registrar_;
-
   // Sign in screen controller.
   std::unique_ptr<ExistingUserController> existing_user_controller_;
 
@@ -194,20 +182,9 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
 
   std::unique_ptr<SignInScreenController> signin_screen_controller_;
 
-  // Make sure chrome won't exit while we are at login/oobe screen.
-  std::unique_ptr<ScopedKeepAlive> keep_alive_;
-
-  // Has ShutdownDisplayHost() already been called?  Used to avoid posting our
-  // own deletion to the message loop twice if the user logs out while we're
-  // still in the process of cleaning up after login (http://crbug.com/134463).
-  bool shutting_down_ = false;
-
   // Whether progress bar is shown on the OOBE page.
   bool oobe_progress_bar_visible_ = false;
 
-  // True if session start is in progress.
-  bool session_starting_ = false;
-
   // Container of the screen we are displaying.
   views::Widget* login_window_ = nullptr;
 
@@ -272,10 +249,6 @@ class LoginDisplayHostWebUI : public LoginDisplayHostCommon,
   // After OOBE is completed, this is always initialized with true.
   bool oobe_startup_sound_played_ = false;
 
-  // Keeps a copy of the old Drag'n'Drop client, so that it would be disabled
-  // during a login session and restored afterwards.
-  std::unique_ptr<wm::ScopedDragDropDisabler> scoped_drag_drop_disabler_;
-
   bool is_voice_interaction_oobe_ = false;
 
   base::WeakPtrFactory<LoginDisplayHostWebUI> animation_weak_ptr_factory_;