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

ui/app_list/presenter/app_list_presenter_impl.cc

Index: ui/app_list/presenter/app_list_presenter_impl.cc
diff --git a/ui/app_list/presenter/app_list_presenter_impl.cc b/ui/app_list/presenter/app_list_presenter_impl.cc
index e6db16a76d1a001b89936e5dee210b80e819e52d..8e10c8eb636cf06bf0a26e39ee533e96aa57f65b 100644
--- a/ui/app_list/presenter/app_list_presenter_impl.cc
+++ b/ui/app_list/presenter/app_list_presenter_impl.cc
@@ -13,6 +13,8 @@
 #include "ui/aura/window.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
 #include "ui/views/widget/widget.h"
 
 namespace app_list {
@@ -49,14 +51,16 @@ AppListPresenterImpl::~AppListPresenterImpl() {
 }
 
 aura::Window* AppListPresenterImpl::GetWindow() {
-  return is_visible_ && view_ ? view_->GetWidget()->GetNativeWindow() : nullptr;
+  return (GetTargetVisibility() && view_)
+             ? view_->GetWidget()->GetNativeWindow()
+             : nullptr;
 }
 
 void AppListPresenterImpl::Show(int64_t display_id) {
-  if (is_visible_)
+  if (GetTargetVisibility())
     return;
 
-  is_visible_ = true;
+  target_display_id_ = display_id;
   if (view_) {
     ScheduleAnimation();
   } else {
@@ -73,13 +77,13 @@ void AppListPresenterImpl::Show(int64_t display_id) {
 }
 
 void AppListPresenterImpl::Dismiss() {
-  if (!is_visible_)
+  if (!GetTargetVisibility())
     return;
 
   // If the app list is currently visible, there should be an existing view.
   DCHECK(view_);
 
-  is_visible_ = false;
+  target_display_id_ = display::kInvalidDisplayId;
 
   // The dismissal may have occurred in response to the app list losing
   // activation. Otherwise, our widget is currently active. When the animation
@@ -102,20 +106,28 @@ void AppListPresenterImpl::ToggleAppList(int64_t display_id) {
   Show(display_id);
 }
 
-bool AppListPresenterImpl::IsVisible() const {
-  return view_ && view_->GetWidget()->IsVisible();
+bool AppListPresenterImpl::IsVisible(int64_t display_id) const {
+  views::Widget* widget = view_ ? view_->GetWidget() : nullptr;
+  return widget && widget->IsVisible() &&
+         (display_id == display::kInvalidDisplayId ||
+          display_id ==
+              display::Screen::GetScreen()
+                  ->GetDisplayNearestWindow(widget->GetNativeWindow())
+                  .id());
 }

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

this function might be easier to read with some early return false-es.

 
-bool AppListPresenterImpl::GetTargetVisibility() const {
-  return is_visible_;
+bool AppListPresenterImpl::GetTargetVisibility(int64_t display_id) const {
+  return target_display_id_ != display::kInvalidDisplayId &&

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

nit: This might be clearer as:

if (target_display_id_ == invalid)
  return false;  // Not shown anywhere.

return display_id == target_ || display_id == invalid;

or something like that.

+         (display_id == display::kInvalidDisplayId ||
+          display_id == target_display_id_);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListPresenterImpl, private:
 
 void AppListPresenterImpl::SetView(AppListView* view) {
-  DCHECK(view_ == nullptr);
-  DCHECK(is_visible_);
+  DCHECK_EQ(view_, nullptr);
+  DCHECK(GetTargetVisibility());
 
   view_ = view;
   views::Widget* widget = view_->GetWidget();
@@ -153,7 +165,7 @@ void AppListPresenterImpl::ScheduleAnimation() {
   gfx::Rect target_bounds;
   gfx::Vector2d offset = presenter_delegate_->GetVisibilityAnimationOffset(
       widget->GetNativeView()->GetRootWindow());
-  if (is_visible_) {
+  if (GetTargetVisibility()) {
     target_bounds = widget->GetWindowBoundsInScreen();
     gfx::Rect start_bounds = gfx::Rect(target_bounds);
     start_bounds.Offset(offset);
@@ -165,10 +177,10 @@ void AppListPresenterImpl::ScheduleAnimation() {
 
   ui::ScopedLayerAnimationSettings animation(layer->GetAnimator());
   animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(
-      is_visible_ ? 0 : kAnimationDurationMs));
+      GetTargetVisibility() ? 0 : kAnimationDurationMs));
   animation.AddObserver(this);
 
-  layer->SetOpacity(is_visible_ ? 1.0 : 0.0);
+  layer->SetOpacity(GetTargetVisibility() ? 1.0 : 0.0);
   widget->SetBounds(target_bounds);
 }
 
@@ -177,7 +189,7 @@ void AppListPresenterImpl::ScheduleAnimation() {
 
 void AppListPresenterImpl::OnWindowFocused(aura::Window* gained_focus,
                                            aura::Window* lost_focus) {
-  if (view_ && is_visible_) {
+  if (view_ && GetTargetVisibility()) {
     aura::Window* applist_window = view_->GetWidget()->GetNativeView();
     aura::Window* applist_container = applist_window->parent();
     if (applist_container->Contains(lost_focus) &&
@@ -201,7 +213,7 @@ void AppListPresenterImpl::OnWindowBoundsChanged(aura::Window* root,
 // AppListPresenterImpl, ui::ImplicitAnimationObserver implementation:
 
 void AppListPresenterImpl::OnImplicitAnimationsCompleted() {
-  if (is_visible_)
+  if (GetTargetVisibility())
     view_->GetWidget()->Activate();
   else
     view_->GetWidget()->Close();
@@ -212,7 +224,7 @@ void AppListPresenterImpl::OnImplicitAnimationsCompleted() {
 
 void AppListPresenterImpl::OnWidgetDestroying(views::Widget* widget) {
   DCHECK(view_->GetWidget() == widget);
-  if (is_visible_)
+  if (GetTargetVisibility())
     Dismiss();
   ResetView();
 }