Update gesture logic.

Update gesture logic.

- record the original state when the gesture starts
- changes the height condition for the closest state based on the
  original state
- limit the fling handling only when the closest state ends up with
  the original state. This will save the most of the case, however
  it may cause unexpected results if the user makes complicated
  gestures (like moves up from minimized, up to the full-screen,
  then fling-down). I think this is a rare case and we don't
  have to care about.

BUG=415211
R=pkotwicz@chromium.org
TEST=athena_unittests && manually

Committed: https://crrev.com/deb67ab915f05ff05495e55f0bb3e615202712a2
Cr-Commit-Position: refs/heads/master@{#295616}

[pkotwicz, 2014-09-18 21:11:50]

LGTM with comment nits

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager.cc (right):

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:16: const int kMinimizedHomeBufferSize
= 50;
This might be slightly clearer:

// The maximum height, in pixels, of a home card with final state
VISIBLE_MINIMIZED.
const int kFinalStateMaxHeight = 50;

// The maximum height, in pixels, of an initially centered home card with final
state VISIBLE_MINIMIZED.
const int kMinimizedFinalStateMaxHeightInitiallyCentered = 90;

// The minimum height, as a percentage of the screen height, of a home card with
final state VISIBLE_CENTERED.
const int kCenteredFinalStateMinScreenRatio = 0.5f;

// The minimum height, as a percentage of the screen height, of an initially
minimized home card with final state VISIBLE_CENTERED.
const int kCenteredFinalStateMinScreenRatio = 0.3f;

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:63: // When the scroll updates don't
move enough to switch to another states
How about: "When the user does not drag far enough to switch the final state,
but a fling happens at the end of the gesture, the state should change based on
the direction of the fling.
Checking |final_state| == |original_state| may cause unexpected results for
gestures where the user flings in the opposite direction that they moved the
home card (e.g. drag home card up from minimized state and then fling down)
TODO(mukai): Consider this case once reported.

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:90: HomeCard::State
HomeCardGestureManager::GetClosestState() const {
GetFinalState() is probably a better name

[pkotwicz, 2014-09-18 21:24:26]

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager_unittest.cc (right):

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager_unittest.cc:194: // No changes for slight
changes.
changes -> move

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager_unittest.cc:261: // state, but the state
should not change to centered but remain in bottom.
I am not sure whether this test case is very useful. The test case tests a case
where the desired behavior is ambiguous.

Testing the behavior of a weak fling after the home card is slightly larger than
kHomeCardHeight is more useful in my opinion.

[Jun Mukai, 2014-09-18 21:57:29]

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager.cc (right):

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:16: const int kMinimizedHomeBufferSize
= 50;
On 2014/09/18 21:11:50, pkotwicz wrote:
> This might be slightly clearer:
> 
> // The maximum height, in pixels, of a home card with final state
> VISIBLE_MINIMIZED.
> const int kFinalStateMaxHeight = 50;
> 
> // The maximum height, in pixels, of an initially centered home card with
final
> state VISIBLE_MINIMIZED.
> const int kMinimizedFinalStateMaxHeightInitiallyCentered = 90;
> 
> // The minimum height, as a percentage of the screen height, of a home card
with
> final state VISIBLE_CENTERED.
> const int kCenteredFinalStateMinScreenRatio = 0.5f;
> 
> // The minimum height, as a percentage of the screen height, of an initially
> minimized home card with final state VISIBLE_CENTERED.
> const int kCenteredFinalStateMinScreenRatio = 0.3f;

Done.

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:63: // When the scroll updates don't
move enough to switch to another states
On 2014/09/18 21:11:50, pkotwicz wrote:
> How about: "When the user does not drag far enough to switch the final state,
> but a fling happens at the end of the gesture, the state should change based
on
> the direction of the fling.
> Checking |final_state| == |original_state| may cause unexpected results for
> gestures where the user flings in the opposite direction that they moved the
> home card (e.g. drag home card up from minimized state and then fling down)
> TODO(mukai): Consider this case once reported.

Done.

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:90: HomeCard::State
HomeCardGestureManager::GetClosestState() const {
On 2014/09/18 21:11:50, pkotwicz wrote:
> GetFinalState() is probably a better name

Done.

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager_unittest.cc (right):

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager_unittest.cc:194: // No changes for slight
changes.
On 2014/09/18 21:24:26, pkotwicz wrote:
> changes -> move

Done.

https://codereview.chromium.org/581933004/diff/20001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager_unittest.cc:261: // state, but the state
should not change to centered but remain in bottom.
On 2014/09/18 21:24:26, pkotwicz wrote:
> I am not sure whether this test case is very useful. The test case tests a
case
> where the desired behavior is ambiguous.
> 
> Testing the behavior of a weak fling after the home card is slightly larger
than
> kHomeCardHeight is more useful in my opinion.

Hmm, a weak fling doesn't cause any effects ("WeakFling" captures the case that
a weak fling is treated as a touch-release).

Updated the comment. Is it clearer?

[pkotwicz, 2014-09-18 22:15:32]

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager.cc (right):

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:93: if (last_estimated_height_ <
kHomeCardMinimizedHeight + height)
Nit: Can this simply be:
if (last_estimated_height_ < max_height)
  return HomeCard::VISIBLE_MINIMIZED

(I have renamed |height| to |max_height|)

I suspect that the values of kMinimizedFinalStateMaxHeight and
kMinimizedFinalStateMaxHeightInitiallyCentered will need to be adjusted

Sorry that I was unclear in my previous comment

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager_unittest.cc (right):

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager_unittest.cc:264: 
I now understand what the test tests. Thank you for updating the comment

Nit: Tests the situation where

[Jun Mukai, 2014-09-18 22:33:33]

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager.cc (right):

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager.cc:93: if (last_estimated_height_ <
kHomeCardMinimizedHeight + height)
On 2014/09/18 22:15:32, pkotwicz wrote:
> Nit: Can this simply be:
> if (last_estimated_height_ < max_height)
>   return HomeCard::VISIBLE_MINIMIZED
> 
> (I have renamed |height| to |max_height|)
> 
> I suspect that the values of kMinimizedFinalStateMaxHeight and
> kMinimizedFinalStateMaxHeightInitiallyCentered will need to be adjusted
> 
> Sorry that I was unclear in my previous comment

Done.

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
File athena/home/home_card_gesture_manager_unittest.cc (right):

https://codereview.chromium.org/581933004/diff/40001/athena/home/home_card_ge...
athena/home/home_card_gesture_manager_unittest.cc:264: 
On 2014/09/18 22:15:32, pkotwicz wrote:
> I now understand what the test tests. Thank you for updating the comment
> 
> Nit: Tests the situation where

Done.

[Jun Mukai, 2014-09-18 22:33:36]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-09-18 22:34:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/581933004/50001

[commit-bot: I haz the power, 2014-09-19 00:10:40]

Committed patchset #4 (id:50001) as 47f4c25298e0f58c976d914cbe309878dad27dfd

[commit-bot: I haz the power, 2014-09-19 00:11:24]

Patchset 4 (id:??) landed as
https://crrev.com/deb67ab915f05ff05495e55f0bb3e615202712a2
Cr-Commit-Position: refs/heads/master@{#295616}