Refactor SetInOverview

athena/wm/window_manager_impl.cc

 // Copyright 2014 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 "athena/wm/window_manager_impl.h"
 
 #include <algorithm>
 
 #include "athena/screen/public/screen_manager.h"
 #include "athena/util/container_priorities.h"
@@ skipping 175 matching lines @@
     // Relayout so that windows are maximzied.
     container_->layout_manager()->OnWindowResized();
   } else if (split_view_controller_->CanActivateSplitViewMode()) {
     FOR_EACH_OBSERVER(WindowManagerObserver,
                       observers_,
                       OnSplitViewModeEnter());
     split_view_controller_->ActivateSplitMode(NULL, NULL, NULL);
   }
 }
 
-void WindowManagerImpl::ToggleOverview() {
-  if (IsOverviewModeActive()) {
-    SetInOverview(false);
+void WindowManagerImpl::EnterOverview() {
+  if (IsOverviewModeActive())
+    return;
 
-    // Activate the window which was active prior to entering overview.
-    const aura::Window::Windows windows =
-        window_list_provider_->GetWindowList();
-    if (!windows.empty()) {
-      aura::Window* window = windows.back();
-      // Show the window in case the exit overview animation has finished and
-      // |window| was hidden.
-      window->Show();
+  bezel_controller_->set_left_right_delegate(NULL);
+  FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnOverviewModeEnter());
 
-      wm::ActivateWindow(window);
-    }
-  } else {
-    SetInOverview(true);
+  // Note: The window_list_provider_ resembles the exact window list of the
+  // container, so no re-stacking is required before showing the OverviewMode.
+  overview_ = WindowOverviewMode::Create(
+      container_.get(), window_list_provider_.get(),
+      split_view_controller_.get(), this);
+}
+
+// Will reactivate the previously active window

[pkotwicz, 2014/10/09 21:13:59]

Nit: This comment should be in the window_manager.h
How about: "Exits overview and activates the previously active activity."

[Greg Levin, 2014/10/10 21:13:45]

Done.

+void WindowManagerImpl::ExitOverview() {
+  if (!IsOverviewModeActive())
+    return;
+
+  ExitOverviewNoActivate();
+
+  // Activate the window which was active prior to entering overview.
+  const aura::Window::Windows windows = window_list_provider_->GetWindowList();
+  if (!windows.empty()) {
+    aura::Window* window_to_activate = windows.back();
+
+    // Show the window in case the exit overview animation has finished and
+    // |window| was hidden.
+    window_to_activate->Show();
+    wm::ActivateWindow(window_to_activate);
   }
 }
 
 bool WindowManagerImpl::IsOverviewModeActive() {
   return overview_;
 }
 
-void WindowManagerImpl::SetInOverview(bool active) {
-  bool in_overview = !!overview_;
-  if (active == in_overview)
+// Only handles overview mode, will not actiavte any windows

[pkotwicz, 2014/10/09 21:13:59]

Nit: The comment should be in the .h file
For the comment, how about: "Exits overview mode without changing activation.
The caller should ensure that a window is active after exiting overview mode."

[Greg Levin, 2014/10/10 21:13:45]

Done.

+void WindowManagerImpl::ExitOverviewNoActivate() {
+  if (!IsOverviewModeActive())
     return;
 
-  bezel_controller_->set_left_right_delegate(
-      active ? NULL : split_view_controller_.get());
-  if (active) {
-    FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnOverviewModeEnter());
-
-    // Note: The window_list_provider_ resembles the exact window list of the
-    // container, so no re-stacking is required before showing the OverviewMode.
-    overview_ = WindowOverviewMode::Create(
-        container_.get(), window_list_provider_.get(),
-        split_view_controller_.get(), this);
-  } else {
-    overview_.reset();
-    FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnOverviewModeExit());
-  }
+  bezel_controller_->set_left_right_delegate(split_view_controller_.get());
+  overview_.reset();
+  FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnOverviewModeExit());
 }
 
 void WindowManagerImpl::InstallAccelerators() {
   const AcceleratorData accelerator_data[] = {
       {TRIGGER_ON_PRESS, ui::VKEY_F6, ui::EF_NONE, CMD_TOGGLE_OVERVIEW,
        AF_NONE},
       {TRIGGER_ON_PRESS, ui::VKEY_F6, ui::EF_CONTROL_DOWN,
        CMD_TOGGLE_SPLIT_VIEW, AF_NONE},
   };
   AcceleratorManager::Get()->RegisterAccelerators(
@@ skipping 10 matching lines @@
 
 void WindowManagerImpl::ToggleSplitViewForTest() {
   ToggleSplitView();
 }
 
 WindowListProvider* WindowManagerImpl::GetWindowListProvider() {
   return window_list_provider_.get();
 }
 
 void WindowManagerImpl::OnSelectWindow(aura::Window* window) {
-  SetInOverview(false);
+  ExitOverviewNoActivate();
 
   // Show the window in case the exit overview animation has finished and
   // |window| was hidden.
   window->Show();
-

[pkotwicz, 2014/10/09 21:13:59]

Nit: Keep the white space.

In this case, it is ambiguous whether removing the whitespace is a positive
change. Given that you are not modifying the code around the whitespace it is
preferable not to modify the whitespace in this CL.

[Greg Levin, 2014/10/10 21:13:45]

Done.

   wm::ActivateWindow(window);
 
   if (split_view_controller_->IsSplitViewModeActive()) {
     split_view_controller_->DeactivateSplitMode();
     FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnSplitViewModeExit());
   }
   // If |window| does not have the size of the work-area, then make sure it is
   // resized.
   const gfx::Size work_area =
       gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().work_area().size();
@@ skipping 13 matching lines @@
                                                     base::Unretained(window),
                                                     desired_bounds,
                                                     gfx::Transform())));
     window->SetTransform(transform);
   }
 }
 
 void WindowManagerImpl::OnSelectSplitViewWindow(aura::Window* left,
                                                 aura::Window* right,
                                                 aura::Window* to_activate) {
-  SetInOverview(false);
+  ExitOverviewNoActivate();
   FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnSplitViewModeEnter());
   split_view_controller_->ActivateSplitMode(left, right, to_activate);
 }
 
 void WindowManagerImpl::OnWindowDestroying(aura::Window* window) {
   if (window == container_)
     container_.reset();
 }
 
 bool WindowManagerImpl::IsCommandEnabled(int command_id) const {
   return true;
 }
 
 bool WindowManagerImpl::OnAcceleratorFired(int command_id,
                                            const ui::Accelerator& accelerator) {
   switch (command_id) {
     case CMD_TOGGLE_OVERVIEW:
-      ToggleOverview();
+      if (IsOverviewModeActive())
+        ExitOverview();
+      else
+        EnterOverview();
       break;
     case CMD_TOGGLE_SPLIT_VIEW:
       ToggleSplitView();
       break;
   }
   return true;
 }
 
 aura::Window* WindowManagerImpl::GetWindowBehind(aura::Window* window) {
   const aura::Window::Windows& windows = window_list_provider_->GetWindowList();
@@ skipping 88 matching lines @@
   DCHECK(!instance);
 }
 
 // static
 WindowManager* WindowManager::Get() {
   DCHECK(instance);
   return instance;
 }
 
 }  // namespace athena