Fix shelf auto-hide calculation for app-list visibility.

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/media_delegate.h"
 #include "ash/common/metrics/gesture_action_type.h"
 #include "ash/common/metrics/user_metrics_action.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/display/types/display_constants.h"
 #include "ui/wm/public/window_types.h"
 
 namespace base {
 class SequencedWorkerPool;
 }
 
 namespace display {
 class Display;
 class ManagedDisplayInfo;
 }
@@ skipping 236 matching lines @@
   void ShowAppList();
 
   // Dismisses the app list.
   void DismissAppList();
 
   // Shows the app list if it's not visible. Dismisses it otherwise.
   void ToggleAppList();
 
   // Returns app list actual visibility. This might differ from
   // GetAppListTargetVisibility() when hiding animation is still in flight.
-  bool IsApplistVisible() const;
+  // Pass a valid display id to check target visibility for a specific display.

[James Cook, 2016/11/30 23:27:19]

If you don't pass a valid id what does it check? Just primary display or all
displays?

+  bool IsApplistVisible(int64_t display_id = display::kInvalidDisplayId) const;
 
   // Returns app list target visibility.
-  bool GetAppListTargetVisibility() const;
+  // Pass a valid display id to check visibility for a specific display.
+  bool GetAppListTargetVisibility(
+      int64_t display_id = display::kInvalidDisplayId) const;

[James Cook, 2016/11/30 23:27:19]

optional: I wonder if these methods would be clearer if they didn't use default
parameters, but had two explicit versions of each method. Then it would be
clearer that IsAppListVisible() means visible-anywhere and
IsAppListVisible(display_id) means just that display. The fact that you're using
kInvalidDisplayId to mean "any display" is kind of an implementation detail.

(And I see there are 55 callers of GetAppListTargetVisibility() so it's quite
reasonable not to change all the callers to pass an explicit display id.)

 
   // Returns true if a window is currently pinned.
   virtual bool IsPinned() = 0;
 
   // Sets/Unsets the |window| to as a pinned window. If this is called with a
   // window with WINDOW_STATE_TYPE_PINNED state, then this sets the |window|
   // as a pinned window. Otherwise, this unsets it.
   // For setting, a caller needs to guarantee that no windows are set
   // as pinned window. For unsetting, a caller needs to guarantee that the
   // |window| is the one which is currently set as a pinned window via previous
@@ skipping 224 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_