[CRD iOS] Preserve viewport state when toggling the soft keyboard

remoting/client/ui/desktop_viewport.h

 // Copyright 2017 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.
 
 #ifndef REMOTING_CLIENT_UI_DESKTOP_VIEWPORT_H_
 #define REMOTING_CLIENT_UI_DESKTOP_VIEWPORT_H_
 
 #include "base/callback.h"
 #include "remoting/client/ui/view_matrix.h"
 
@@ skipping 17 matching lines @@
 // You may either manipulate the desktop on the surface coordinate or manipulate
 // the viewport on the desktop coordinate, depending on your choice of the
 // reference frame.
 class DesktopViewport {
  public:
   using TransformationCallback = base::Callback<void(const ViewMatrix&)>;
 
   DesktopViewport();
   ~DesktopViewport();
 
-  // Sets the |desktop_size_| and initializes the viewport when necessary.
+  // Sets the |desktop_size_| and (re)initializes the viewport.
   void SetDesktopSize(int desktop_width, int desktop_height);
 
-  // Sets the |surface_size_| and initializes the viewport when necessary.
+  // Sets the |surface_size_| and (re)initializes the viewport if both

[nicholss, 2017/05/26 20:12:49]

I think this is too much to assume.

[Yuwei, 2017/05/26 21:38:21]

I agree that we bring too many conditional branching into one function.
Basically:

Neither dimension has changed => No-op
Both dimension changed => ResizeToFit
Only one dimension changed => Only update the constraints

I like to hide the idea of the constraint outside the class.

Maybe we can remove the ResizeToFit() call from the function and give it to the
caller to control, and make SetSurfaceSize always update the constraints.
Potentially this will trigger two viewport changes but they will be synchronized
into the same frame so I think it's fine.

I think the bigger question is what is the criteria for triggering
ResizeToFit(). Both rotating screen and toggling the keyboard (maybe later
opening the side menu) will trigger the same codepath of viewDidLayoutSubviews.

For use cases we have:
* Rotation => Reset
* Keyboard => No reset
* Settings menu => No reset

Checking whether two dimensions match covers all use cases above. I'm not sure
whether that's an overfitting for this problem thought..

+  // dimensions are changed.
   void SetSurfaceSize(int surface_width, int surface_height);
 
   // Translates the desktop on the surface's reference frame by <dx, dy>.
   void MoveDesktop(float dx, float dy);
 
   // Scales the desktop on the surface's reference frame at pivot point (px, py)
   // by |scale|.
   void ScaleDesktop(float px, float py, float scale);
 
   // Moves the viewport center by <x, y> on the desktop's coordinate.
@@ skipping 64 matching lines @@
 
   TransformationCallback on_transformation_changed_;
 
   // DesktopViewport is neither copyable nor movable.
   DesktopViewport(const DesktopViewport&) = delete;
   DesktopViewport& operator=(const DesktopViewport&) = delete;
 };
 
 }  // namespace remoting
 #endif  // REMOTING_CLIENT_UI_DESKTOP_VIEWPORT_H_