Revert "[Chromoting] Add DisplaySize class to better track DIPS vs pixels."

This reverts commit 96c9c934.

Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 694864 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzk2YzljOTM0YWYxYjY5MDdmNDJkYmY1ZmRlN2YwZjlhYzk4MmEwYzQM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/82989

Sample Failed Step: remoting_unittests

Original change's description:
> [Chromoting] Add DisplaySize class to better track DIPS vs pixels.
> 
> This cl also removes some support for the ICE protocol to simplify
> the display handling code.
> 
> Change-Id: I64523c0fc766cd80acdaae832c8f0fa08bb59757
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790274
> Auto-Submit: Gary Kacmarcik <garykac@chromium.org>
> Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#694864}

TBR=jamiewalch@chromium.org,garykac@chromium.org

Change-Id: Icdc2a54da344f2dc082c0d12477f190c56257394
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792985
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695023}

remoting/client/chromoting_client.cc

@@ -183,9 +183,6 @@ void ChromotingClient::SetVideoLayout(const protocol::VideoLayout& layout) {
   const protocol::VideoTrackLayout& track_layout = layout.video_track(0);
   int x_dpi = track_layout.has_x_dpi() ? track_layout.x_dpi() : kDefaultDpi;
   int y_dpi = track_layout.has_y_dpi() ? track_layout.y_dpi() : kDefaultDpi;
-  if (x_dpi != y_dpi) {
-    LOG(ERROR) << "Mismatched x,y dpi. x=" << x_dpi << " y=" << y_dpi;
-  }
 
   webrtc::DesktopSize size_dips(track_layout.width(), track_layout.height());
   webrtc::DesktopSize size_pixels(size_dips.width() * x_dpi / kDefaultDpi,
@@ -193,9 +190,11 @@ void ChromotingClient::SetVideoLayout(const protocol::VideoLayout& layout) {
   user_interface_->SetDesktopSize(size_pixels,
                                   webrtc::DesktopVector(x_dpi, y_dpi));
 
-  DisplaySize size(size_dips.width(), size_dips.height(), x_dpi);
-  mouse_input_scaler_.set_input_size(size);
-  mouse_input_scaler_.set_output_size(size);
+  mouse_input_scaler_.set_input_size(webrtc::DesktopSize(size_pixels));
+  mouse_input_scaler_.set_output_size(
+      connection_->config().protocol() == protocol::SessionConfig::Protocol::ICE
+          ? webrtc::DesktopSize(size_pixels)
+          : webrtc::DesktopSize(size_dips));
 }
 
 void ChromotingClient::InjectClipboardEvent(

remoting/host/client_session.cc

@@ -502,11 +502,25 @@ std::unique_ptr<protocol::ClipboardStub> ClientSession::CreateClipboardProxy() {
       base::ThreadTaskRunnerHandle::Get());
 }
 
-void ClientSession::SetMouseClampingFilter(const DisplaySize& size) {
+void ClientSession::SetMouseClampingFilter(const webrtc::DesktopSize& size,
+                                           const webrtc::DesktopVector& dpi) {
   UpdateMouseClampingFilterOffset();
-  // TODO(garykac) Combine these into a single call.
   mouse_clamping_filter_.set_output_size(size);
-  mouse_clamping_filter_.set_input_size(size);
+
+  switch (connection_->session()->config().protocol()) {
+    case protocol::SessionConfig::Protocol::ICE:
+      mouse_clamping_filter_.set_input_size(webrtc::DesktopSize(size));
+      break;
+
+    case protocol::SessionConfig::Protocol::WEBRTC: {
+      // When using WebRTC protocol the client sends mouse coordinates in DIPs,
+      // while InputInjector expects them in physical pixels.
+      // TODO(sergeyu): Fix InputInjector implementations to use DIPs as well.
+      webrtc::DesktopSize size_dips =
+          DesktopDisplayInfo::CalcSizeDips(size, dpi.x(), dpi.y());
+      mouse_clamping_filter_.set_input_size(webrtc::DesktopSize(size_dips));
+    }
+  }
 }
 
 void ClientSession::UpdateMouseClampingFilterOffset() {
@@ -518,41 +532,42 @@ void ClientSession::UpdateMouseClampingFilterOffset() {
 }
 
 void ClientSession::OnVideoSizeChanged(protocol::VideoStream* video_stream,
-                                       const webrtc::DesktopSize& size_px,
+                                       const webrtc::DesktopSize& size,
                                        const webrtc::DesktopVector& dpi) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(connection_->session()->config().protocol() ==
-         protocol::SessionConfig::Protocol::WEBRTC);
   LOG(INFO) << "ClientSession::OnVideoSizeChanged";
-  DisplaySize size =
-      DisplaySize::FromPixels(size_px.width(), size_px.height(), dpi.x());
-  LOG(INFO) << "   DisplaySize (dips): " << size;
-  LOG(INFO) << "   DisplaySize (px): " << size.WidthAsPixels() << ","
-            << size.HeightAsPixels();
-  SetMouseClampingFilter(size);
+  SetMouseClampingFilter(size, dpi);
 
   // Record default DPI in case a display reports 0 for DPI.
   default_x_dpi_ = dpi.x();
   default_y_dpi_ = dpi.y();
-  if (dpi.x() != dpi.y()) {
-    LOG(WARNING) << "Mismatch x,y dpi: x=" << dpi.x() << " y=" << dpi.y();
-  }
 
   if (connection_->session()->config().protocol() !=
       protocol::SessionConfig::Protocol::WEBRTC) {
     return;
   }
 
+  LOG(INFO) << "   WebRTC input size (pixels): " << size.width() << "x"
+            << size.height() << " @ " << dpi.x() << "," << dpi.y();
+  webrtc::DesktopSize size_dips =
+      DesktopDisplayInfo::CalcSizeDips(size, dpi.x(), dpi.y());
+  LOG(INFO) << "   DPI: " << dpi.x() << "," << dpi.y();
+  LOG(INFO) << "   WebRTC input size (DIPS): " << size_dips.width() << "x"
+            << size_dips.height();
+
   // Generate and send VideoLayout message.
   protocol::VideoLayout layout;
   protocol::VideoTrackLayout* video_track = layout.add_video_track();
   video_track->set_position_x(0);
   video_track->set_position_y(0);
-  video_track->set_width(size.WidthAsDips());
-  video_track->set_height(size.HeightAsDips());
+  video_track->set_width(size_dips.width());
+  video_track->set_height(size_dips.height());
   video_track->set_x_dpi(dpi.x());
   video_track->set_y_dpi(dpi.y());
 
+  LOG(INFO) << "  VideoLayout Desktop (DIPS) = 0,0 " << size_dips.width() << "x"
+            << size_dips.height() << " [" << dpi.x() << "," << dpi.y() << "]";
+
   // VideoLayout can be sent only after the control channel is connected.
   // TODO(sergeyu): Change client_stub() implementation to allow queuing
   // while connection is being established.

remoting/host/client_session.h

@@ -29,7 +29,6 @@
 #include "remoting/protocol/clipboard_stub.h"
 #include "remoting/protocol/connection_to_client.h"
 #include "remoting/protocol/data_channel_manager.h"
-#include "remoting/protocol/display_size.h"
 #include "remoting/protocol/host_stub.h"
 #include "remoting/protocol/input_event_tracker.h"
 #include "remoting/protocol/input_filter.h"
@@ -170,7 +169,8 @@ class ClientSession : public protocol::HostStub,
   // Creates a proxy for sending clipboard events to the client.
   std::unique_ptr<protocol::ClipboardStub> CreateClipboardProxy();
 
-  void SetMouseClampingFilter(const DisplaySize& size);
+  void SetMouseClampingFilter(const webrtc::DesktopSize& size,
+                              const webrtc::DesktopVector& dpi);
 
   // protocol::VideoStream::Observer implementation.
   void OnVideoSizeChanged(protocol::VideoStream* stream,

remoting/protocol/BUILD.gn

@@ -52,8 +52,6 @@ static_library("protocol") {
     "data_channel_manager.cc",
     "data_channel_manager.h",
     "datagram_channel_factory.h",
-    "display_size.cc",
-    "display_size.h",
     "errors.cc",
     "errors.h",
     "file_transfer_helpers.cc",

remoting/protocol/display_size.cc

-// Copyright 2019 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.
-
-#include "remoting/protocol/display_size.h"
-
-#include "build/build_config.h"
-#include "remoting/base/constants.h"
-
-namespace remoting {
-
-bool DisplaySize::operator==(const DisplaySize& other) {
-  return other.width_dips_ == width_dips_ &&
-         other.height_dips_ == height_dips_ && other.dpi_ == dpi_;
-}
-
-bool DisplaySize::operator!=(const DisplaySize& other) {
-  return !(*this == other);
-}
-
-bool DisplaySize::IsEmpty() const {
-  return width_dips_ == 0 || height_dips_ == 0;
-}
-
-int32_t DisplaySize::WidthAsPixels() const {
-  return width_dips_ * GetScalingFactor();
-}
-
-int32_t DisplaySize::WidthAsDips() const {
-  return width_dips_;
-}
-
-int32_t DisplaySize::HeightAsPixels() const {
-  return height_dips_ * GetScalingFactor();
-}
-
-int32_t DisplaySize::HeightAsDips() const {
-  return height_dips_;
-}
-
-uint32_t DisplaySize::GetDpi() const {
-  return dpi_;
-}
-
-float DisplaySize::GetScalingFactor() const {
-  return (float)dpi_ / (float)kDefaultDpi;
-}
-
-}  // namespace remoting
-
-std::ostream& operator<<(std::ostream& out, const remoting::DisplaySize& size) {
-  out << size.WidthAsDips() << "x" << size.HeightAsDips() << " @"
-      << size.GetDpi();
-  return out;
-}

remoting/protocol/display_size.h

-// Copyright 2019 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_PROTOCOL_DISPLAY_SIZE_H_
-#define REMOTING_PROTOCOL_DISPLAY_SIZE_H_
-
-#include <stddef.h>
-#include <ostream>
-
-#include "base/logging.h"
-#include "remoting/base/constants.h"
-
-namespace remoting {
-
-class DisplaySize {
- public:
-  constexpr DisplaySize() : width_dips_(0), height_dips_(0), dpi_(0) {}
-
-  constexpr DisplaySize(int32_t width_dips, int32_t height_dips, uint32_t dpi)
-      : width_dips_(width_dips), height_dips_(height_dips), dpi_(dpi) {}
-
-  ~DisplaySize() = default;
-
-  // Static method to build a DisplaySize from pixels rather than DIPs.
-  static constexpr DisplaySize FromPixels(int32_t width_px,
-                                          int32_t height_px,
-                                          uint32_t dpi) {
-    const int32_t width_dips = width_px * ((float)kDefaultDpi / dpi);
-    const int32_t height_dips = height_px * ((float)kDefaultDpi / dpi);
-    return DisplaySize(width_dips, height_dips, dpi);
-  }
-
-  bool operator==(const DisplaySize& other);
-  bool operator!=(const DisplaySize& other);
-
-  bool IsEmpty() const;
-
-  int32_t WidthAsDips() const;
-  int32_t HeightAsDips() const;
-
-  int32_t WidthAsPixels() const;
-  int32_t HeightAsPixels() const;
-
-  uint32_t GetDpi() const;
-  float GetScalingFactor() const;
-
- private:
-  int32_t width_dips_, height_dips_;
-  uint32_t dpi_;  // Number of "dots" (physical pixels) per logical inch.
-};
-
-}  // namespace remoting
-
-std::ostream& operator<<(std::ostream& out, const remoting::DisplaySize& size);
-
-#endif  // REMOTING_PROTOCOL_DISPLAY_SIZE_H_

remoting/protocol/mouse_input_filter.cc

@@ -22,7 +22,7 @@ MouseInputFilter::MouseInputFilter(InputStub* input_stub)
 MouseInputFilter::~MouseInputFilter() = default;
 
 void MouseInputFilter::InjectMouseEvent(const MouseEvent& event) {
-  if (input_size_.IsEmpty() || output_size_.IsEmpty())
+  if (input_size_.is_empty() || output_size_.is_empty())
     return;
 
   // We scale based on the maximum input & output coordinates, rather than the
@@ -33,14 +33,14 @@ void MouseInputFilter::InjectMouseEvent(const MouseEvent& event) {
   // but will be non-zero when we are showing a single display.
   MouseEvent out_event(event);
   if (out_event.has_x()) {
-    int x = out_event.x() * output_size_.WidthAsPixels();
-    x = (x + input_size_.WidthAsDips() / 2) / input_size_.WidthAsDips();
+    int x = out_event.x() * output_size_.width();
+    x = (x + input_size_.width() / 2) / input_size_.width();
     out_event.set_x(output_offset_.x() +
                     base::ClampToRange(x, 0, output_size_.WidthAsPixels()));
   }
   if (out_event.has_y()) {
-    int y = out_event.y() * output_size_.HeightAsPixels();
-    y = (y + input_size_.HeightAsDips() / 2) / input_size_.HeightAsDips();
+    int y = out_event.y() * output_size_.height();
+    y = (y + input_size_.height() / 2) / input_size_.height();
     out_event.set_y(output_offset_.y() +
                     base::ClampToRange(y, 0, output_size_.HeightAsPixels()));
   }
@@ -48,18 +48,16 @@ void MouseInputFilter::InjectMouseEvent(const MouseEvent& event) {
   InputFilter::InjectMouseEvent(out_event);
 }
 
-void MouseInputFilter::set_input_size(const DisplaySize& size) {
-  input_size_ = DisplaySize(size.WidthAsDips() - 1, size.HeightAsDips() - 1,
-                            size.GetDpi());
-  LOG(INFO) << "Setting MouseInputFilter input_size to (dips) " << input_size_;
+void MouseInputFilter::set_input_size(const webrtc::DesktopSize& size) {
+  input_size_ = webrtc::DesktopSize(size.width() - 1, size.height() - 1);
+  LOG(INFO) << "Setting MouseInputFilter input_size to " << input_size_.width()
+            << "x" << input_size_.height();
 }
 
-void MouseInputFilter::set_output_size(const DisplaySize& size) {
-  output_size_ = DisplaySize::FromPixels(
-      size.WidthAsPixels() - 1, size.HeightAsPixels() - 1, size.GetDpi());
-  LOG(INFO) << "Setting MouseInputFilter output_size to (px) "
-            << output_size_.WidthAsPixels() << "x"
-            << output_size_.HeightAsPixels();
+void MouseInputFilter::set_output_size(const webrtc::DesktopSize& size) {
+  output_size_ = webrtc::DesktopSize(size.width() - 1, size.height() - 1);
+  LOG(INFO) << "Setting MouseInputFilter output_size to "
+            << output_size_.width() << "x" << output_size_.height();
 }
 
 void MouseInputFilter::set_output_offset(const webrtc::DesktopVector& v) {

remoting/protocol/mouse_input_filter.h

@@ -7,7 +7,6 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "remoting/protocol/display_size.h"
 #include "remoting/protocol/input_filter.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
 
@@ -24,18 +23,19 @@ class MouseInputFilter : public InputFilter {
   ~MouseInputFilter() override;
 
   // Specify the input dimensions for mouse events.
-  void set_input_size(const DisplaySize& size_dips);
+  // This is specified in DIPs for WebRTC and pixels for ICE protocol.
+  void set_input_size(const webrtc::DesktopSize& r);
 
   // Specify the output dimensions (always in physical pixels).
-  void set_output_size(const DisplaySize& size_px);
+  void set_output_size(const webrtc::DesktopSize& r);
   void set_output_offset(const webrtc::DesktopVector& v);
 
   // InputStub overrides.
   void InjectMouseEvent(const protocol::MouseEvent& event) override;
 
  private:
-  DisplaySize input_size_;
-  DisplaySize output_size_;
+  webrtc::DesktopSize input_size_;
+  webrtc::DesktopSize output_size_;
   webrtc::DesktopVector output_offset_;
 
   DISALLOW_COPY_AND_ASSIGN(MouseInputFilter);

remoting/protocol/mouse_input_filter_unittest.cc

@@ -32,27 +32,25 @@ struct Point {
   int y;
 };
 
-const int kDefaultDpi = 96;
-
 class MouseInputFilterTest : public testing::Test {
  public:
   MouseInputFilterTest() : mouse_filter_(&mock_stub_) {}
 
   // Set the size of the client viewing rectangle.
   void SetClientSize(int width, int height) {
-    mouse_filter_.set_input_size(DisplaySize(width, height, kDefaultDpi));
+    mouse_filter_.set_input_size(webrtc::DesktopSize(width, height));
   }
 
   // Set the size of the host desktop. For multimon, this is the bounding box
   // that encloses all displays.
   void SetHostDesktop(int width, int height) {
-    mouse_filter_.set_output_size(DisplaySize(width, height, kDefaultDpi));
+    mouse_filter_.set_output_size(webrtc::DesktopSize(width, height));
   }
 
   // Set the size and offset of a single display in a multimon setup.
   void SetHostMultimonSingleDisplay(int x, int y, int width, int height) {
     mouse_filter_.set_output_offset(webrtc::DesktopVector(x, y));
-    mouse_filter_.set_output_size(DisplaySize(width, height, kDefaultDpi));
+    mouse_filter_.set_output_size(webrtc::DesktopSize(width, height));
   }
 
   void InjectMouse(const Point& point, bool swap = false) {