Always consider SystemUI for panning boundaries in split-screen mode

remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java

 // Copyright 2015 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.
 
 package org.chromium.chromoting;
 
 import android.graphics.Matrix;
 import android.graphics.PointF;
 import android.graphics.Rect;
 import android.graphics.RectF;
@@ skipping 94 matching lines @@
         updateCursorPosition(
                 mDesiredCenter.x + deltaX, mDesiredCenter.y + deltaY, getViewportBounds());
     }
 
     /**
      * Handles System UI size and visibility changes.
      *
      * @param parameter The set of values defining the current System UI state.
      */
     public void onSystemUiVisibilityChanged(SystemUiVisibilityChangedEventParameter parameter) {
-        if (parameter.softInputMethodVisible) {
-            mSystemUiScreenRect.set(
-                    parameter.left, parameter.top, parameter.right, parameter.bottom);
+        mSystemUiScreenRect.set(parameter.left, parameter.top, parameter.right, parameter.bottom);
+        stopOffsetReductionAnimation();
 
-            stopOffsetReductionAnimation();
+        PointF targetOffset;
+        if (mSystemUiScreenRect.isEmpty()) {
+            targetOffset = new PointF(0.0f, 0.0f);
         } else {
-            mSystemUiScreenRect.setEmpty();
-            startOffsetReductionAnimation();
+            // If the System UI size has changed such viewport offset is affected, then start an
+            // animation to adjust the amount of offset used.  This functionality ensures that we
+            // don't leave content-less areas on the screen when the System UI resizes.
+            RectF systemUiOverlap = getSystemUiOverlap();
+            RectF newBounds = new RectF(-systemUiOverlap.left, -systemUiOverlap.top,
+                    systemUiOverlap.right, systemUiOverlap.bottom);
+            targetOffset = new PointF(mViewportOffset.x, mViewportOffset.y);
+            constrainPointToBounds(targetOffset, newBounds);
+        }
+
+        if (!targetOffset.equals(mViewportOffset.x, mViewportOffset.y)) {

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

Is this a float equality comparison?
Maybe better to move this test inside startOffsetReductionAnimation()?

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

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.

+            startOffsetReductionAnimation(targetOffset);
         }
 
         if (mRenderData.initialized()) {
             // The viewport center may have changed so update the position to reflect the new value.
             repositionImage();
         }
     }
 
     public void adjustViewportForSystemUi(boolean adjustViewportForSystemUi) {
         mAdjustViewportSizeForSystemUi = adjustViewportForSystemUi;
@@ skipping 259 matching lines @@
         // viewport offset as a maximum boundary.  If we add the offset first, the result ends up
         // being unbounded.  Thus we use a temporary object for the boundary calculation.
         PointF requestedOffset =
                 new PointF(mViewportOffset.x + offsets[0], mViewportOffset.y + offsets[1]);
         constrainPointToBounds(requestedOffset, getViewportOffsetBounds());
         mViewportOffset.set(requestedOffset);
     }
 
     /**
      * Starts an animation to smoothly reduce the viewport offset.  Does nothing if an animation is
      * already running or the offset is already 0.

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

"already 0" still correct?

[joedow, 2016/11/08 23:04:16]

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

      */
-    private void startOffsetReductionAnimation() {
+    private void startOffsetReductionAnimation(final PointF targetOffset) {
         if (mFrameRenderedCallback != null || mViewportOffset.length() < EPSILON) {

[Lambros, 2016/11/08 21:55:02]

Is this still the right test if targetOffset is non-zero?

[joedow, 2016/11/08 23:04:16]

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.

             return;
         }
 
         mFrameRenderedCallback = new Event.ParameterCallback<Boolean, Void>() {
             private static final float DURATION_MS = 250.0f;
 
             private final Interpolator mInterpolator = new DecelerateInterpolator();
 
             private long mStartTime = 0;
-            private float mOriginalX = 0.0f;
-            private float mOriginalY = 0.0f;
+            private final float mOriginalX = mViewportOffset.x - targetOffset.x;
+            private final float mOriginalY = mViewportOffset.y - targetOffset.y;
 
             @Override
             public Boolean run(Void p) {
                 if (mFrameRenderedCallback == null) {
                     return false;
                 }
 
                 if (mStartTime == 0) {
                     mStartTime = SystemClock.elapsedRealtime();
-                    mOriginalX = mViewportOffset.x;
-                    mOriginalY = mViewportOffset.y;
                 }
 
                 float progress = (SystemClock.elapsedRealtime() - mStartTime) / DURATION_MS;
                 if (progress < 1.0f) {
                     float reductionFactor = 1.0f - mInterpolator.getInterpolation(progress);
-                    mViewportOffset.set(mOriginalX * reductionFactor, mOriginalY * reductionFactor);
+                    mViewportOffset.set(mOriginalX * reductionFactor + targetOffset.x,
+                            mOriginalY * reductionFactor + targetOffset.y);
                 } else {
-                    mViewportOffset.set(0.0f, 0.0f);
+                    mViewportOffset.set(targetOffset.x, targetOffset.y);
                     mFrameRenderedCallback = null;
                 }
 
                 repositionImage();
 
                 return mFrameRenderedCallback != null;
             }
         };
 
         mRenderStub.onCanvasRendered().addSelfRemovable(mFrameRenderedCallback);
     }
 
     /**
      * Stops an existing offset reduction animation.
      */
     private void stopOffsetReductionAnimation() {
         // Setting this value this null will prevent it from continuing to execute.
         mFrameRenderedCallback = null;
     }
 }