chromeos: Fix shelf appearing at login screen under mash

ash/common/wm_shell.h

 // Copyright 2016 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 ASH_COMMON_WM_SHELL_H_
 #define ASH_COMMON_WM_SHELL_H_
 
 #include <stdint.h>
 
 #include <memory>
 #include <vector>
 
 #include "ash/ash_export.h"
 #include "ash/common/metrics/gesture_action_type.h"
 #include "ash/common/metrics/user_metrics_action.h"
+#include "ash/common/session/session_state_observer.h"
 #include "ash/common/wm/lock_state_observer.h"
 #include "base/observer_list.h"
 #include "components/ui_devtools/devtools_server.h"
 #include "ui/base/ui_base_types.h"
 #include "ui/compositor/layer_type.h"
 #include "ui/wm/public/window_types.h"
 
 namespace app_list {
 class AppList;
 }
@@ skipping 67 matching lines @@
 class MaximizeModeEventHandler;
 class WindowState;
 }
 
 #if defined(OS_CHROMEOS)
 class LogoutConfirmationController;
 class VpnList;
 #endif
 
 // Similar to ash::Shell. Eventually the two will be merged.
-class ASH_EXPORT WmShell {
+class ASH_EXPORT WmShell : public SessionStateObserver {

[msw, 2016/12/16 16:26:20]

Hmm, this is a little unfortunate; how tough would it be to compose an object
which does this? (could ShelfController itself do this?)

[James Cook, 2016/12/20 04:02:42]

I don't have an elegant way to get rid of this.

I tried making ShelfController be a SessionStateObserver, but I don't like it.
(See next patchset for how I did it.) I have to inject the session_controller_
because WmShell isn't done initializing yet, and there's a not-very-obvious
observer order dependency between ShelfController (which must run first to
create the shelf) and WmShellAura (which has to create the keyboard).

I'm going to upload another patchset that keeps this a SessionStateObserver but
makes OnSessionStateChange() protected, like you mention below. Let me know
which approach you prefer.

[msw, 2016/12/20 15:37:30]

I guess it's okay for the shell to observe session state to initialize the
shelf. I'm just a little concerned about inviting the shelf to inherit other
base classes and take on responsibilities that should be encapsulated in
per-component objects, like ShelfController.

[James Cook, 2016/12/20 17:27:18]

Per offline discussion, keeping as-is.

We also might need to do other things in WmShell related to session state in the
future, like replace UpdateAfterLoginStatusChange().

  public:
   // This is necessary for a handful of places that is difficult to plumb
   // through context.
   static void Set(WmShell* instance);
   static WmShell* Get();
   static bool HasInstance() { return instance_ != nullptr; }
 
   void Initialize(const scoped_refptr<base::SequencedWorkerPool>& pool);
   virtual void Shutdown();
 
@@ skipping 222 matching lines @@
   CreateScopedDisableInternalMouseAndKeyboard() = 0;
 
   virtual std::unique_ptr<ImmersiveFullscreenController>
   CreateImmersiveFullscreenController() = 0;
 
   virtual std::unique_ptr<KeyEventWatcher> CreateKeyEventWatcher() = 0;
 
   // Initializes the appropriate shelves. Does nothing for any existing shelves.
   void CreateShelf();
 
-  // Show shelf view if it was created hidden (before session has started).
-  void ShowShelf();
-
   void CreateShelfDelegate();
 
   // Called after maximize mode has started, windows might still animate though.
   void OnMaximizeModeStarted();
 
   // Called after maximize mode has ended, windows might still be returning to
   // their original position.
   void OnMaximizeModeEnded();
 
   // Called when the overview mode is about to be started (before the windows
   // get re-arranged).
   virtual void OnOverviewModeStarting() = 0;
 
   // Called after overview mode has ended.
   virtual void OnOverviewModeEnded() = 0;
 
+  // Called after the initial user logs in and again after each additional user
+  // is added to the session. See session_manager::SessionState::ACTIVE.
+  virtual void OnUserSessionStateActive() = 0;

[msw, 2016/12/16 16:26:20]

This is a bit odd; if we do indeed keep WmShell as a SessionStateObserver, I
would prefer that the separate WmShell subclasses simply override
SessionStateChanged  and just check for ACTIVE (each subclass could inherit
SessionStateObserver, or the  override here could be protected instead of
private). Otherwise, this just seems like a less useful duplicate of the
observer function, with a slightly awkward name.

[James Cook, 2016/12/20 04:02:43]

Removed.

+
   // Called when the login status changes.
   // TODO(oshima): Investigate if we can merge this and |OnLoginStateChanged|.
   void UpdateAfterLoginStatusChange(LoginStatus status);
 
   // Notify observers that fullscreen mode has changed for |root_window|.
   void NotifyFullscreenStateChanged(bool is_fullscreen, WmWindow* root_window);
 
   // Notify observers that |pinned_window| changed its pinned window state.
   void NotifyPinnedStateChanged(WmWindow* pinned_window);
 
@@ skipping 64 matching lines @@
 
   // TODO(jamescook): Remove this when VirtualKeyboardController has been moved.
   virtual void ToggleIgnoreExternalKeyboard() = 0;
 
   // Enable or disable the laser pointer.
   virtual void SetLaserPointerEnabled(bool enabled) = 0;
 #endif
 
  protected:
   explicit WmShell(std::unique_ptr<ShellDelegate> shell_delegate);
-  virtual ~WmShell();
+  ~WmShell() override;
 
   base::ObserverList<ShellObserver>* shell_observers() {
     return &shell_observers_;
   }
 
   void SetKeyboardUI(std::unique_ptr<KeyboardUI> keyboard_ui);
 
   // Helpers to set (and initialize) or destroy various delegates.
   // TODO(msw|jamescook): Remove these once ShellDelegate, etc. are ported.
   void SetSystemTrayDelegate(std::unique_ptr<SystemTrayDelegate> delegate);
@@ skipping 13 matching lines @@
 
   void SetAcceleratorController(
       std::unique_ptr<AcceleratorController> accelerator_controller);
 
  private:
   friend class AcceleratorControllerTest;
   friend class ScopedRootWindowForNewWindows;
   friend class Shell;
   friend class WmShellTestApi;
 
+  // SessionStateObserver:
+  void SessionStateChanged(session_manager::SessionState state) override;
+
   static WmShell* instance_;
 
   base::ObserverList<ShellObserver> shell_observers_;
   std::unique_ptr<ShellDelegate> delegate_;
 
   std::unique_ptr<AcceleratorController> accelerator_controller_;
   std::unique_ptr<AccessibilityDelegate> accessibility_delegate_;
   std::unique_ptr<app_list::AppList> app_list_;
   std::unique_ptr<BrightnessControlDelegate> brightness_control_delegate_;
   std::unique_ptr<CastConfigController> cast_config_;
@@ skipping 35 matching lines @@
 
 #if defined(OS_CHROMEOS)
   std::unique_ptr<LogoutConfirmationController> logout_confirmation_controller_;
   std::unique_ptr<VpnList> vpn_list_;
 #endif
 };
 
 }  // namespace ash
 
 #endif  // ASH_COMMON_WM_SHELL_H_