Split AppListController::SetVisible into Show and Dismiss.

ash/wm/app_list_controller.cc

 // Copyright (c) 2012 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.
 
 #include "ash/wm/app_list_controller.h"
 
 #include "ash/ash_switches.h"
 #include "ash/root_window_controller.h"
 #include "ash/screen_util.h"
 #include "ash/shelf/shelf.h"
@@ skipping 155 matching lines @@
   // lives in the controller and app list view would access it on destruction.
   if (view_) {
     view_->GetAppsPaginationModel()->RemoveObserver(this);
     if (view_->GetWidget())
       view_->GetWidget()->CloseNow();
   }
 
   Shell::GetInstance()->RemoveShellObserver(this);
 }
 
-void AppListController::SetVisible(bool visible, aura::Window* window) {
-  if (visible == is_visible_)
+void AppListController::Show(aura::Window* window) {
+  if (is_visible_)
     return;
 
-  is_visible_ = visible;
+  is_visible_ = true;
 
   // App list needs to know the new shelf layout in order to calculate its
   // UI layout when AppListView visibility changes.
   Shell::GetPrimaryRootWindowController()->GetShelfLayoutManager()->
       UpdateAutoHideState();
 
-  if (view_) {
-    // Our widget is currently active. When the animation completes we'll hide
-    // the widget, changing activation. If a menu is shown before the animation
-    // completes then the activation change triggers the menu to close. By
-    // deactivating now we ensure there is no activation change when the
-    // animation completes and any menus stay open.
-    if (!visible)
-      view_->GetWidget()->Deactivate();
-    ScheduleAnimation();
-  } else if (is_visible_) {
+  if (!view_) {
     // AppListModel and AppListViewDelegate are owned by AppListView. They
     // will be released with AppListView on close.
     app_list::AppListView* view = new app_list::AppListView(
         Shell::GetInstance()->delegate()->CreateAppListViewDelegate());
     aura::Window* root_window = window->GetRootWindow();
     aura::Window* container = GetRootWindowController(root_window)->
         GetContainer(kShellWindowId_AppListContainer);
     views::View* applist_button =
         Shelf::ForWindow(container)->GetAppListButtonView();
     is_centered_ = view->ShouldCenterWindow();
@@ skipping 25 matching lines @@
               Shelf::ForWindow(container)->GetAppListButtonView()->GetWidget()),
           GetBubbleArrow(container),
           true /* border_accepts_events */);
       view->SetArrowPaintType(views::BubbleBorder::PAINT_NONE);
     }
     SetView(view);
     // By setting us as DnD recipient, the app list knows that we can
     // handle items.
     SetDragAndDropHostOfCurrentAppList(
         Shelf::ForWindow(window)->GetDragAndDropHostForAppList());
+  } else {
+    ScheduleAnimation();

[Matt Giuca, 2014/08/19 08:26:29]

Is there a reason to invert the if statement? Why not just leave this where it
was (but delete the comment and the "if (!visible)" bit).

[calamity, 2014/08/20 00:44:53]

Done.

   }
+
   // Update applist button status when app list visibility is changed.
   Shelf::ForWindow(window)->GetAppListButtonView()->SchedulePaint();
 }
 
+void AppListController::Dismiss() {
+  if (!is_visible_)
+    return;
+
+  is_visible_ = false;
+
+  // App list needs to know the new shelf layout in order to calculate its
+  // UI layout when AppListView visibility changes.
+  Shell::GetPrimaryRootWindowController()
+      ->GetShelfLayoutManager()
+      ->UpdateAutoHideState();
+
+  if (!view_)

[Matt Giuca, 2014/08/19 08:26:29]

This slightly changes the logic in that you won't call SchedulePaint if !view_,
whereas previously you would have.

Is that important?

[calamity, 2014/08/20 00:44:53]

I don't think this will be a problem. !view_ should never happen except while
the widget is being destroyed. To be absolutely safe, we can add a
SchedulePaint() to ResetView().

[Matt Giuca, 2014/08/20 06:13:12]

Note: We discussed this and decided to replace this if statement with a
DCHECK(view_).

It *should* always be true; if it isn't true, returning from here would not be
correct.

+    return;
+
+  // Our widget is currently active. When the animation completes we'll hide
+  // the widget, changing activation. If a menu is shown before the animation
+  // completes then the activation change triggers the menu to close. By
+  // deactivating now we ensure there is no activation change when the
+  // animation completes and any menus stay open.
+  view_->GetWidget()->Deactivate();
+  ScheduleAnimation();
+
+  // Update applist button status when app list visibility is changed.
+  Shelf::ForWindow(view_->GetWidget()->GetNativeView())
+      ->GetAppListButtonView()
+      ->SchedulePaint();
+}
+
 bool AppListController::IsVisible() const {
   return view_ && view_->GetWidget()->IsVisible();
 }
 
 aura::Window* AppListController::GetWindow() {
   return is_visible_ && view_ ? view_->GetWidget()->GetNativeWindow() : NULL;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListController, private:
@@ skipping 90 matching lines @@
         return;
       aura::Window* keyboard_container = root_controller->GetContainer(
           kShellWindowId_VirtualKeyboardContainer);
       if (keyboard_container->Contains(target))
         return;
     }
   }
 
   aura::Window* window = view_->GetWidget()->GetNativeView()->parent();
   if (!window->Contains(target))
-    SetVisible(false, window);
+    Dismiss();
 }
 
 void AppListController::UpdateBounds() {
   if (!view_ || !is_visible_)
     return;
 
   view_->UpdateBounds();
 
   if (is_centered_)
     view_->SetAnchorPoint(GetCenterOfDisplayForView(
@@ skipping 17 matching lines @@
 // AppListController,  aura::FocusObserver implementation:
 
 void AppListController::OnWindowFocused(aura::Window* gained_focus,
                                         aura::Window* lost_focus) {
   if (view_ && is_visible_) {
     aura::Window* applist_window = view_->GetWidget()->GetNativeView();
     aura::Window* applist_container = applist_window->parent();
 
     if (applist_container->Contains(lost_focus) &&
         (!gained_focus || !applist_container->Contains(gained_focus))) {
-      SetVisible(false, applist_window);
+      Dismiss();
     }
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListController,  aura::WindowObserver implementation:
 void AppListController::OnWindowBoundsChanged(aura::Window* root,
                                               const gfx::Rect& old_bounds,
                                               const gfx::Rect& new_bounds) {
   UpdateBounds();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListController, ui::ImplicitAnimationObserver implementation:
 
 void AppListController::OnImplicitAnimationsCompleted() {
   if (is_visible_ )
     view_->GetWidget()->Activate();
   else
     view_->GetWidget()->Close();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListController, views::WidgetObserver implementation:
 
 void AppListController::OnWidgetDestroying(views::Widget* widget) {
   DCHECK(view_->GetWidget() == widget);
   if (is_visible_)
-    SetVisible(false, widget->GetNativeView());
+    Dismiss();
   ResetView();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListController, keyboard::KeyboardControllerObserver implementation:
 
 void AppListController::OnKeyboardBoundsChanging(const gfx::Rect& new_bounds) {
   UpdateBounds();
 }
 
@@ skipping 58 matching lines @@
   } else if (should_snap_back_) {
     should_snap_back_ = false;
     ui::ScopedLayerAnimationSettings animation(widget_animator);
     animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(
         app_list::kOverscrollPageTransitionDurationMs));
     widget->SetBounds(view_bounds_);
   }
 }
 
 }  // namespace ash