Restore original host resolution on receipt of an empty ClientResolution message.

This allows clients to restore the original host desktop size when the relevant
option is unchecked, rather than only on disconnect.

Change-Id: Ia344223bb1475c3b60fbbdd0510f20be8694203d
Reviewed-on: https://chromium-review.googlesource.com/1058675
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560800}

remoting/host/client_session.cc

@@ -98,8 +98,7 @@ ClientSession::~ClientSession() {
 void ClientSession::NotifyClientResolution(
     const protocol::ClientResolution& resolution) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(resolution.dips_width() > 0 && resolution.dips_height() > 0);
-
+  DCHECK(resolution.dips_width() >= 0 && resolution.dips_height() >= 0);
   VLOG(1) << "Received ClientResolution (dips_width="
           << resolution.dips_width() << ", dips_height="
           << resolution.dips_height() << ")";

remoting/host/desktop_session_proxy.cc

@@ -370,9 +370,6 @@ void DesktopSessionProxy::SetScreenResolution(
     const ScreenResolution& resolution) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 
-  if (resolution.IsEmpty())
-    return;
-
   screen_resolution_ = resolution;
 
   // Connect to the desktop session if it is not done yet.

remoting/host/resizing_host_observer.cc

@@ -126,8 +126,8 @@ ResizingHostObserver::ResizingHostObserver(
       weak_factory_(this) {}
 
 ResizingHostObserver::~ResizingHostObserver() {
-  if (restore_ && !original_resolution_.IsEmpty())
-    desktop_resizer_->RestoreResolution(original_resolution_);
+  if (restore_)
+    RestoreScreenResolution();
 }
 
 void ResizingHostObserver::SetScreenResolution(
@@ -136,8 +136,10 @@ void ResizingHostObserver::SetScreenResolution(
   // to SetScreenResolution to simplify the implementation of unit-tests.
   base::TimeTicks now = now_function_.Run();
 
-  if (resolution.IsEmpty())
+  if (resolution.IsEmpty()) {
+    RestoreScreenResolution();
     return;
+  }
 
   // Resizing the desktop too often is probably not a good idea, so apply a
   // simple rate-limiting scheme.
@@ -185,4 +187,11 @@ void ResizingHostObserver::SetNowFunctionForTesting(
   now_function_ = now_function;
 }
 
+void ResizingHostObserver::RestoreScreenResolution() {
+  if (!original_resolution_.IsEmpty()) {
+    desktop_resizer_->RestoreResolution(original_resolution_);
+    original_resolution_ = ScreenResolution();
+  }
+}
+
 }  // namespace remoting

remoting/host/resizing_host_observer.h

@@ -45,6 +45,8 @@ class ResizingHostObserver : public ScreenControls {
       const base::Callback<base::TimeTicks(void)>& now_function);
 
  private:
+  void RestoreScreenResolution();
+
   std::unique_ptr<DesktopResizer> desktop_resizer_;
   ScreenResolution original_resolution_;
   bool restore_;

remoting/host/resizing_host_observer_unittest.cc

@@ -197,6 +197,20 @@ TEST_F(ResizingHostObserverTest, RestoreFlag) {
   EXPECT_EQ(MakeResolution(640, 480), current_resolution_);
 }
 
+// Check that the size is restored if an empty ClientResolution is received.
+TEST_F(ResizingHostObserverTest, RestoreOnEmptyClientResolution) {
+  InitDesktopResizer(MakeResolution(640, 480), true,
+                     std::vector<ScreenResolution>(), true);
+  resizing_host_observer_->SetScreenResolution(MakeResolution(200, 100));
+  EXPECT_EQ(1, call_counts_.set_resolution);
+  EXPECT_EQ(0, call_counts_.restore_resolution);
+  EXPECT_EQ(MakeResolution(200, 100), current_resolution_);
+  resizing_host_observer_->SetScreenResolution(MakeResolution(0, 0));
+  EXPECT_EQ(1, call_counts_.set_resolution);
+  EXPECT_EQ(1, call_counts_.restore_resolution);
+  EXPECT_EQ(MakeResolution(640, 480), current_resolution_);
+}
+
 // Check that if the implementation supports exact size matching, it is used.
 TEST_F(ResizingHostObserverTest, SelectExactSize) {
   InitDesktopResizer(MakeResolution(640, 480), true,

remoting/host/screen_controls.h

@@ -15,7 +15,9 @@ class ScreenControls {
  public:
   virtual ~ScreenControls() {}
 
-  // Attempts to set new screen resolution in the session.
+  // If |resolution| is not empty, attempts to set new screen resolution in the
+  // session. If |resolution| is empty, attempts to restore the original screen
+  // resolution.
   virtual void SetScreenResolution(const ScreenResolution& resolution) = 0;
 };
 

remoting/proto/control.proto

@@ -10,8 +10,10 @@ option optimize_for = LITE_RUNTIME;
 
 package remoting.protocol;
 
+// Set the host resolution to match the client. If none of the fields are
+// present, restore the host resolution instead.
 message ClientResolution {
-  // Width and height of the client in Density-Independent Pixels
+  // Width and height of the client in Density-Independent Pixels.
   optional int32 dips_width = 1;
   optional int32 dips_height = 2;
 

remoting/protocol/host_control_dispatcher.cc

@@ -72,13 +72,13 @@ void HostControlDispatcher::OnIncomingMessage(
   if (!message)
     return;
 
-  // TODO(sergeyu): Move message valudation from the message handlers here.
+  // TODO(sergeyu): Move message validation from the message handlers here.
   if (message->has_clipboard_event()) {
     clipboard_stub_->InjectClipboardEvent(message->clipboard_event());
   } else if (message->has_client_resolution()) {
     const ClientResolution& resolution = message->client_resolution();
-    if (!resolution.has_dips_width() || !resolution.has_dips_height() ||
-        resolution.dips_width() <= 0 || resolution.dips_height() <= 0) {
+    if ((resolution.has_dips_width() && resolution.dips_width() <= 0) ||
+        (resolution.has_dips_height() && resolution.dips_height() <= 0)) {
       LOG(ERROR) << "Received invalid ClientResolution message.";
       return;
     }