chromeos: Fix shelf appearing at login screen under mash

ash/common/wm_shell.h

Index: ash/common/wm_shell.h
diff --git a/ash/common/wm_shell.h b/ash/common/wm_shell.h
index e0267f04a05c795cba5f7bd8aed92f5414640c69..38e8d726efdf8baf81b3b8de4d0115295f9f4e24 100644
--- a/ash/common/wm_shell.h
+++ b/ash/common/wm_shell.h
@@ -13,6 +13,7 @@
 #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"
@@ -100,7 +101,7 @@ 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.
@@ -343,9 +344,6 @@ class ASH_EXPORT WmShell {
   // 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.
@@ -362,6 +360,10 @@ class ASH_EXPORT WmShell {
   // 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);
@@ -446,7 +448,7 @@ class ASH_EXPORT WmShell {
 
  protected:
   explicit WmShell(std::unique_ptr<ShellDelegate> shell_delegate);
-  virtual ~WmShell();
+  ~WmShell() override;
 
   base::ObserverList<ShellObserver>* shell_observers() {
     return &shell_observers_;
@@ -480,6 +482,9 @@ class ASH_EXPORT WmShell {
   friend class Shell;
   friend class WmShellTestApi;
 
+  // SessionStateObserver:
+  void SessionStateChanged(session_manager::SessionState state) override;
+
   static WmShell* instance_;
 
   base::ObserverList<ShellObserver> shell_observers_;