Always consider SystemUI for panning boundaries in split-screen mode

Always consider SystemUI for panning boundaries in split-screen mode

This change allows the user to pan the desktop out from under System UI
when in windowed/split-screen mode.  In fullscreen mode, we only allow
this behavior when the Soft Keyboard is present.  For N+, we need to
allow the user to pan out from under System UI as it is always present
in this mode (unlike fullscreen where the UI will autohide).

This CL moves the logic for determining when to allow 'over-panning' to
the Desktop activity (from the DesktopCanvas class).  The Desktop class
is a better place for this logic as it is aware of more state
information and should own the end user experience (whereas the
DesktopCanvas class should only handle boundary and viewport
calculations).

I've also updated the offset reduction animation code to allow it to
animate to a specific offset (instead of a hard-coded 0, 0).  This
functionality is used when the size of the System UI is reduced to a
non-zero value.  An example of this is when the toolbar auto-hides but
the status bar is still displayed.  In that case we want the desktop
image to be animated upwards (reducing the amount of viewport offset)
but we don't want to animate to the top of the screen, just to the
bottom of the status bar.

BUG=662458
Committed: https://crrev.com/20e8fb18e771067d5cbbb1e22d7fc6c19c4b4d3a
Cr-Commit-Position: refs/heads/master@{#430755}

[joedow, 2016-11-08 20:23:11]

The CQ bit was checked by joedow@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 20:24:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 20:38:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-08 20:38:40]

Dry run: This issue passed the CQ dry run.

[joedow, 2016-11-08 20:44:43]

joedow@chromium.org changed reviewers:
+ lambroslambrou@chromium.org, yuweih@chromium.org

[joedow, 2016-11-08 20:44:44]

joedow@chromium.org changed required reviewers:
+ lambroslambrou@chromium.org

[joedow, 2016-11-08 20:44:44]

PTAL!

[Lambros, 2016-11-08 21:55:03]

LGTM when comments are addressed.

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java
(right):

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java:132: if
(!targetOffset.equals(mViewportOffset.x, mViewportOffset.y)) {
Is this a float equality comparison?
Maybe better to move this test inside startOffsetReductionAnimation()?

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java:413: *
already running or the offset is already 0.
"already 0" still correct?

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java:416: if
(mFrameRenderedCallback != null || mViewportOffset.length() < EPSILON) {
Is this still the right test if targetOffset is non-zero?

[joedow, 2016-11-08 23:03:41]

The CQ bit was checked by joedow@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 23:04:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[joedow, 2016-11-08 23:04:17]

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java
(right):

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java:132: if
(!targetOffset.equals(mViewportOffset.x, mViewportOffset.y)) {
On 2016/11/08 21:55:03, Lambros wrote:
> Is this a float equality comparison?
> Maybe better to move this test inside startOffsetReductionAnimation()?

I wanted to use the Android class functions for the comparison but looking into
them I don't think they are good enough.  I've added a helper function to make
the comparison more reliable.

I've moved the check into the function as suggested.

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java:413: *
already running or the offset is already 0.
On 2016/11/08 21:55:03, Lambros wrote:
> "already 0" still correct?

Yeah this is correct.  There is no work to do if the offset is already 0.

https://codereview.chromium.org/2485143004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java:416: if
(mFrameRenderedCallback != null || mViewportOffset.length() < EPSILON) {
On 2016/11/08 21:55:02, Lambros wrote:
> Is this still the right test if targetOffset is non-zero?

Yeah I think so.  The point of this method is to reduce the viewport offset to a
minimum of zero so this check is correct.  A non-zero target and a zero offset
would be a bug.  If we decided to animation the addition of offset padding, then
it wouldn't be correct.

[commit-bot: I haz the power, 2016-11-08 23:13:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-08 23:13:45]

Dry run: This issue passed the CQ dry run.

[joedow, 2016-11-08 23:18:44]

The CQ bit was checked by joedow@chromium.org

[joedow, 2016-11-08 23:18:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
lambroslambrou@chromium.org
Link to the patchset: https://codereview.chromium.org/2485143004/#ps20001
(title: "Addressing CR Fedback")

[commit-bot: I haz the power, 2016-11-08 23:19:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 23:28:48]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-08 23:38:25]

Description was changed from

==========
Always consider SystemUI for panning boundaries in split-screen mode

This change allows the user to pan the desktop out from under System UI
when in windowed/split-screen mode.  In fullscreen mode, we only allow
this behavior when the Soft Keyboard is present.  For N+, we need to
allow the user to pan out from under System UI as it is always present
in this mode (unlike fullscreen where the UI will autohide).

This CL moves the logic for determining when to allow 'over-panning' to
the Desktop activity (from the DesktopCanvas class).  The Desktop class
is a better place for this logic as it is aware of more state
information and should own the end user experience (whereas the
DesktopCanvas class should only handle boundary and viewport
calculations).

I've also updated the offset reduction animation code to allow it to
animate to a specific offset (instead of a hard-coded 0, 0).  This
functionality is used when the size of the System UI is reduced to a
non-zero value.  An example of this is when the toolbar auto-hides but
the status bar is still displayed.  In that case we want the desktop
image to be animated upwards (reducing the amount of viewport offset)
but we don't want to animate to the top of the screen, just to the
bottom of the status bar.

BUG=662458
==========

to

==========
Always consider SystemUI for panning boundaries in split-screen mode

This change allows the user to pan the desktop out from under System UI
when in windowed/split-screen mode.  In fullscreen mode, we only allow
this behavior when the Soft Keyboard is present.  For N+, we need to
allow the user to pan out from under System UI as it is always present
in this mode (unlike fullscreen where the UI will autohide).

This CL moves the logic for determining when to allow 'over-panning' to
the Desktop activity (from the DesktopCanvas class).  The Desktop class
is a better place for this logic as it is aware of more state
information and should own the end user experience (whereas the
DesktopCanvas class should only handle boundary and viewport
calculations).

I've also updated the offset reduction animation code to allow it to
animate to a specific offset (instead of a hard-coded 0, 0).  This
functionality is used when the size of the System UI is reduced to a
non-zero value.  An example of this is when the toolbar auto-hides but
the status bar is still displayed.  In that case we want the desktop
image to be animated upwards (reducing the amount of viewport offset)
but we don't want to animate to the top of the screen, just to the
bottom of the status bar.

BUG=662458

Committed: https://crrev.com/20e8fb18e771067d5cbbb1e22d7fc6c19c4b4d3a
Cr-Commit-Position: refs/heads/master@{#430755}
==========

[commit-bot: I haz the power, 2016-11-08 23:38:26]

Patchset 2 (id:??) landed as
https://crrev.com/20e8fb18e771067d5cbbb1e22d7fc6c19c4b4d3a
Cr-Commit-Position: refs/heads/master@{#430755}