ESC accelerator and consistent overview mode for Athena homecard

athena/home/home_card_impl.cc

Index: athena/home/home_card_impl.cc
diff --git a/athena/home/home_card_impl.cc b/athena/home/home_card_impl.cc
index 2b11db46220dc0e0f33b46f7b0f6d66810ef2376..8036a4e6c3fcbc90beb7d7f72e8cfae0d096f608 100644
--- a/athena/home/home_card_impl.cc
+++ b/athena/home/home_card_impl.cc
@@ -226,6 +226,7 @@ HomeCardImpl::HomeCardImpl(AppModelBuilder* model_builder)
     : model_builder_(model_builder),
       state_(HIDDEN),
       original_state_(VISIBLE_MINIMIZED),
+      overview_state_(VISIBLE_BOTTOM),
       home_card_widget_(NULL),
       home_card_view_(NULL),
       layout_manager_(NULL),
@@ -302,6 +303,21 @@ void HomeCardImpl::InstallAccelerators() {
       accelerator_data, arraysize(accelerator_data), this);
 }
 
+// Use when switching internally between "shown" and "not shown" so that
+// we enter and exit overview mode appropriately
+void HomeCardImpl::SetStateAndOverview(HomeCard::State state) {

[Jun Mukai, 2014/09/25 21:53:41]

This method name is unclear.  SetStateAndToggleOverview?  But it doesn't toggle
overview state at some time.

Based on my comments below, I believe this function is simply two line of:
SetState(state);
WindowManager::Get()->ToggleOverview();

[Greg Levin, 2014/10/02 21:56:09]

Done (mostly... see below)

+  if (state_ == state)
+    return;
+
+  bool show = (state == VISIBLE_BOTTOM || state == VISIBLE_CENTERED);
+  if (show == WindowManager::GetInstance()->IsOverviewModeActive()) {
+      SetState(state); // overview mode already matches state
+  } else {
+    overview_state_ = state;
+    WindowManager::GetInstance()->ToggleOverview();
+  }
+}
+
 void HomeCardImpl::SetState(HomeCard::State state) {
   if (state_ == state)
     return;
@@ -368,9 +384,9 @@ bool HomeCardImpl::OnAcceleratorFired(int command_id,
   DCHECK_EQ(COMMAND_SHOW_HOME_CARD, command_id);
 
   if (state_ == VISIBLE_CENTERED && original_state_ != VISIBLE_BOTTOM)
-    SetState(VISIBLE_MINIMIZED);
+    SetStateAndOverview(VISIBLE_MINIMIZED);
   else if (state_ == VISIBLE_MINIMIZED)
-    SetState(VISIBLE_CENTERED);
+    SetStateAndOverview(VISIBLE_CENTERED);
   return true;
 }
 
@@ -415,11 +431,12 @@ void HomeCardImpl::OnGestureProgressed(
 
 void HomeCardImpl::OnOverviewModeEnter() {
   if (state_ == HIDDEN || state_ == VISIBLE_MINIMIZED)
-    SetState(VISIBLE_BOTTOM);
+    SetState(overview_state_);

[Jun Mukai, 2014/09/25 21:53:41]

I am not sure why overview_state_ is necessary.  It's used here, right?

I once introduced similar field, and some reviewer (oshima?) pointed out that
it's okay to run SetState() -> ToggleOverview() in this order.  The if-clause of
the previous line was introduced for this purpose -- it prevents setting state
back to BOTTOM from CENTERED.

I believe you can go with the same way.

[Greg Levin, 2014/10/02 21:56:09]

ToggleOverview() was a little imprecise for use here, so I created
WindowManager::ActivateOverview(), and am using SetState() -> ActivateOverview()
instead.

 }
 
 void HomeCardImpl::OnOverviewModeExit() {
   SetState(VISIBLE_MINIMIZED);
+  overview_state_ = VISIBLE_BOTTOM;
 }
 
 void HomeCardImpl::OnSplitViewModeEnter() {