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_) {
+  } else {
     // 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 26 matching lines @@
           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());
   }
+

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

nit: extra blank line.

   // 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();
+
+  // 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.
+  DCHECK(view_);

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

nit: I think this DCHECK should be immediately after the !is_visible if
statement. Add a comment // If the app list is currently visible, there should
be an existing view.

The rationale for both of these changes is that it makes it obvious why the
DCHECK should be valid.

+  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