ChromeShellDelegate: Cleanup: move/remove some methods

This CL:
* Removes IsRunningInForcedAppMode from ChromeShellDelegate
  (ash.mojom.SessionInfo.is_running_in_app_mode is used instead).
* Moves enabling of MagnifierKeyScroller and SpokenFeedbackToggler
  to ash::Shell.
* Eliminates unused PreShutdown method.
* Replaces call back into ash::Shell (ugh!) to show settings with
  direct call.

Bug: 665064

Change-Id: I31e68366be2f6efab3845be6d00ebc3e967c99c7
Reviewed-on: https://chromium-review.googlesource.com/1070771Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561635}

ash/accelerators/accelerator_controller.cc

@@ -1660,7 +1660,7 @@ AcceleratorController::GetAcceleratorProcessingRestriction(int action) const {
       !base::ContainsKey(actions_allowed_at_power_menu_, action)) {
     return RESTRICTION_PREVENT_PROCESSING;
   }
-  if (Shell::Get()->shell_delegate()->IsRunningInForcedAppMode() &&
+  if (Shell::Get()->session_controller()->IsRunningInAppMode() &&
       actions_allowed_in_app_mode_.find(action) ==
           actions_allowed_in_app_mode_.end()) {
     return RESTRICTION_PREVENT_PROCESSING;

ash/session/session_controller.cc

@@ -332,6 +332,11 @@ void SessionController::SetUserSessionOrder(
   if (user_sessions_[0]->session_id != active_session_id_) {
     active_session_id_ = user_sessions_[0]->session_id;
 
+    if (!last_active_account_id.is_valid()) {
+      for (auto& observer : observers_)
+        observer.OnFirstSessionStarted();
+    }
+
     session_activation_observer_holder_.NotifyActiveSessionChanged(
         last_active_account_id, user_sessions_[0]->user_info->account_id);
 

ash/session/session_observer.h

@@ -24,6 +24,9 @@ class ASH_EXPORT SessionObserver {
   // Called when a user session gets added to the existing session.
   virtual void OnUserSessionAdded(const AccountId& account_id) {}
 
+  // Called once the first time a user session starts.
+  virtual void OnFirstSessionStarted() {}
+
   // Called when a user session is updated, such as avatar change.
   virtual void OnUserSessionUpdated(const AccountId& account_id) {}
 

ash/shell.cc

@@ -729,8 +729,6 @@ Shell::~Shell() {
 
   user_metrics_recorder_->OnShellShuttingDown();
 
-  shell_delegate_->PreShutdown();
-
   cros_display_config_.reset();
   display_configuration_observer_.reset();
   display_prefs_.reset();
@@ -1461,13 +1459,23 @@ void Shell::OnWindowActivated(
     root_window_for_new_windows_ = gained_active->GetRootWindow();
 }
 
+void Shell::OnFirstSessionStarted() {
+  // Enable magnifier scroll keys as there may be no mouse cursor in kiosk mode.
+  MagnifierKeyScroller::SetEnabled(session_controller_->IsRunningInAppMode());
+
+  // Enable long press action to toggle spoken feedback with hotrod remote
+  // which can't handle shortcuts.
+  SpokenFeedbackToggler::SetEnabled(session_controller_->IsRunningInAppMode());
+}
+
 void Shell::OnSessionStateChanged(session_manager::SessionState state) {
   // Initialize the |shelf_window_watcher_| when a session becomes active.
   // Shelf itself is initialized in RootWindowController.
   if (state == session_manager::SessionState::ACTIVE) {
-    if (!shelf_window_watcher_)
+    if (!shelf_window_watcher_) {
       shelf_window_watcher_ =
           std::make_unique<ShelfWindowWatcher>(shelf_model());
+    }
   }
 
   // NOTE: keyboard::IsKeyboardEnabled() is false in mash, but may not be in

ash/shell.h

@@ -699,6 +699,7 @@ class ASH_EXPORT Shell : public SessionObserver,
                          aura::Window* lost_active) override;
 
   // SessionObserver:
+  void OnFirstSessionStarted() override;
   void OnSessionStateChanged(session_manager::SessionState state) override;
   void OnLoginStatusChanged(LoginStatus login_status) override;
   void OnLockStateChanged(bool locked) override;

ash/shell/shell_delegate_impl.cc

@@ -33,18 +33,12 @@ ShellDelegateImpl::~ShellDelegateImpl() = default;
   return nullptr;
 }
 
-bool ShellDelegateImpl::IsRunningInForcedAppMode() const {
-  return false;
-}
-
 bool ShellDelegateImpl::CanShowWindowForUser(aura::Window* window) const {
   return true;
 }
 
 void ShellDelegateImpl::PreInit() {}
 
-void ShellDelegateImpl::PreShutdown() {}
-
 std::unique_ptr<keyboard::KeyboardUI> ShellDelegateImpl::CreateKeyboardUI() {
   return std::make_unique<TestKeyboardUI>();
 }

ash/shell/shell_delegate_impl.h

@@ -25,10 +25,8 @@ class ShellDelegateImpl : public ShellDelegate {
 
   // ShellDelegate:
   ::service_manager::Connector* GetShellConnector() const override;
-  bool IsRunningInForcedAppMode() const override;
   bool CanShowWindowForUser(aura::Window* window) const override;
   void PreInit() override;
-  void PreShutdown() override;
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;

ash/shell_delegate.h

@@ -45,9 +45,6 @@ class ASH_EXPORT ShellDelegate {
   // Returns the connector for the mojo service manager. Returns null in tests.
   virtual service_manager::Connector* GetShellConnector() const = 0;
 
-  // Returns true if we're running in forced app mode.
-  virtual bool IsRunningInForcedAppMode() const = 0;
-
   // Returns true if |window| can be shown for the delegate's concept of current
   // user.
   virtual bool CanShowWindowForUser(aura::Window* window) const = 0;
@@ -56,10 +53,6 @@ class ASH_EXPORT ShellDelegate {
   // can perform tasks necessary before the shell is initialized.
   virtual void PreInit() = 0;
 
-  // Called at the beginninig of Shell destructor so that
-  // delegate can use Shell instance to perform cleanup tasks.
-  virtual void PreShutdown() = 0;
-
   // Create a shell-specific keyboard::KeyboardUI.
   virtual std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() = 0;
 

ash/shell_delegate_mus.cc

@@ -51,11 +51,6 @@ service_manager::Connector* ShellDelegateMus::GetShellConnector() const {
   return connector_;
 }
 
-bool ShellDelegateMus::IsRunningInForcedAppMode() const {
-  NOTIMPLEMENTED_LOG_ONCE();
-  return false;
-}
-
 bool ShellDelegateMus::CanShowWindowForUser(aura::Window* window) const {
   NOTIMPLEMENTED_LOG_ONCE();
   return true;
@@ -65,10 +60,6 @@ void ShellDelegateMus::PreInit() {
   NOTIMPLEMENTED_LOG_ONCE();
 }
 
-void ShellDelegateMus::PreShutdown() {
-  NOTIMPLEMENTED_LOG_ONCE();
-}
-
 std::unique_ptr<keyboard::KeyboardUI> ShellDelegateMus::CreateKeyboardUI() {
   NOTIMPLEMENTED_LOG_ONCE();
   return nullptr;

ash/shell_delegate_mus.h

@@ -24,10 +24,8 @@ class ShellDelegateMus : public ShellDelegate {
 
   // ShellDelegate:
   service_manager::Connector* GetShellConnector() const override;
-  bool IsRunningInForcedAppMode() const override;
   bool CanShowWindowForUser(aura::Window* window) const override;
   void PreInit() override;
-  void PreShutdown() override;
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;

ash/test_shell_delegate.cc

@@ -21,18 +21,12 @@ TestShellDelegate::~TestShellDelegate() = default;
   return nullptr;
 }
 
-bool TestShellDelegate::IsRunningInForcedAppMode() const {
-  return false;
-}
-
 bool TestShellDelegate::CanShowWindowForUser(aura::Window* window) const {
   return true;
 }
 
 void TestShellDelegate::PreInit() {}
 
-void TestShellDelegate::PreShutdown() {}
-
 std::unique_ptr<keyboard::KeyboardUI> TestShellDelegate::CreateKeyboardUI() {
   return std::make_unique<TestKeyboardUI>();
 }

ash/test_shell_delegate.h

@@ -19,10 +19,8 @@ class TestShellDelegate : public ShellDelegate {
 
   // Overridden from ShellDelegate:
   ::service_manager::Connector* GetShellConnector() const override;
-  bool IsRunningInForcedAppMode() const override;
   bool CanShowWindowForUser(aura::Window* window) const override;
   void PreInit() override;
-  void PreShutdown() override;
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;

chrome/browser/app_mode/app_mode_utils.h

@@ -12,10 +12,11 @@ namespace chrome {
 // Returns true if the given browser command is allowed in app mode.
 bool IsCommandAllowedInAppMode(int command_id);
 
-// Return true if browser process is run in kiosk or forced app mode.
+// Returns true if the browser process is run in kiosk or forced app mode.
 bool IsRunningInAppMode();
 
-// Return true if browser process is run in forced app mode.
+// Returns true if the browser process is run in forced app mode. Note: On
+// Chrome OS devices this is functionally equivalent to IsRunningInAppMode.
 bool IsRunningInForcedAppMode();
 
 // Returns true if browser process is run in forced app mode for Chrome app

chrome/browser/chromeos/accessibility/DEPS

 include_rules = [
-  # TODO(crbug.com/678705): Convert to using mojo interfaces in
+  # TODO(crbug.com/756054): Convert to using mojo interfaces in
   # //ash/public/interfaces and eliminate this.
   "!ash",
   # Public interfaces are OK.

chrome/browser/ui/ash/chrome_shell_delegate.cc

@@ -9,12 +9,9 @@
 #include <limits>
 #include <vector>
 
-#include "ash/accelerators/magnifier_key_scroller.h"
-#include "ash/accelerators/spoken_feedback_toggler.h"
 #include "ash/accessibility/accessibility_delegate.h"
 #include "ash/public/cpp/accessibility_types.h"
 #include "ash/shell.h"
-#include "ash/system/tray/system_tray_controller.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "base/command_line.h"
 #include "base/macros.h"
@@ -80,13 +77,6 @@ void InitAfterFirstSessionStart() {
       ash::Shell::Get()->mru_window_tracker()->BuildMruWindowList();
   if (!mru_list.empty())
     mru_list.front()->Focus();
-
-  // Enable magnifier scroll keys as there may be no mouse cursor in kiosk mode.
-  ash::MagnifierKeyScroller::SetEnabled(chrome::IsRunningInForcedAppMode());
-
-  // Enable long press action to toggle spoken feedback with hotrod
-  // remote which can't handle shortcut.
-  ash::SpokenFeedbackToggler::SetEnabled(chrome::IsRunningInForcedAppMode());
 }
 
 class AccessibilityDelegateImpl : public ash::AccessibilityDelegate {
@@ -140,10 +130,6 @@ service_manager::Connector* ChromeShellDelegate::GetShellConnector() const {
   return content::ServiceManagerConnection::GetForProcess()->GetConnector();
 }
 
-bool ChromeShellDelegate::IsRunningInForcedAppMode() const {
-  return chrome::IsRunningInForcedAppMode();
-}
-
 bool ChromeShellDelegate::CanShowWindowForUser(aura::Window* window) const {
   return ::CanShowWindowForUser(window, base::Bind(&GetActiveBrowserContext));
 }
@@ -160,9 +146,6 @@ void ChromeShellDelegate::PreInit() {
   new policy::DisplayRotationDefaultHandler();
 }
 
-void ChromeShellDelegate::PreShutdown() {
-}
-
 void ChromeShellDelegate::OpenUrlFromArc(const GURL& url) {
   if (!url.is_valid())
     return;
@@ -179,7 +162,8 @@ void ChromeShellDelegate::OpenUrlFromArc(const GURL& url) {
   if (url_to_open.GetContent() == "settings" &&
       (url_to_open.SchemeIs(url::kAboutScheme) ||
        url_to_open.SchemeIs(content::kChromeUIScheme))) {
-    ash::Shell::Get()->system_tray_controller()->ShowSettings();
+    chrome::ShowSettingsSubPageForProfile(
+        ProfileManager::GetActiveUserProfile(), "");
     return;
   }
 

chrome/browser/ui/ash/chrome_shell_delegate.h

@@ -26,10 +26,8 @@ class ChromeShellDelegate : public ash::ShellDelegate,
 
   // ash::ShellDelegate overrides;
   service_manager::Connector* GetShellConnector() const override;
-  bool IsRunningInForcedAppMode() const override;
   bool CanShowWindowForUser(aura::Window* window) const override;
   void PreInit() override;
-  void PreShutdown() override;
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   ash::NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;