Fix ASAN test failures for Multimon/Input unit tests and re-enable.

BUG: 961064

Change-Id: I14020da7f64f1cc5dd14b4ca89340d554031a82d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130486
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755523}

remoting/host/client_session_unittest.cc

@@ -39,25 +39,6 @@
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "ui/events/event.h"
 
-// TODO(crbug.com/961064): Fix memory leaks in tests and re-enable on LSAN.
-#ifdef LEAK_SANITIZER
-#define MAYBE_LocalInputTest DISABLED_LocalInputTest
-#define MAYBE_ClampMouseEvents DISABLED_ClampMouseEvents
-#define MAYBE_DisableInputs DISABLED_DisableInputs
-#define MAYBE_MultiMonMouseMove_SameSize DISABLED_MultiMonMouseMove_SameSize
-#define MAYBE_RestoreEventState DISABLED_RestoreEventState
-#define MAYBE_MultiMonMouseMove DISABLED_MultiMonMouseMove
-#define MAYBE_DisconnectOnLocalInputTest DISABLED_DisconnectOnLocalInputTest
-#else
-#define MAYBE_LocalInputTest LocalInputTest
-#define MAYBE_ClampMouseEvents ClampMouseEvents
-#define MAYBE_DisableInputs DisableInputs
-#define MAYBE_MultiMonMouseMove_SameSize MultiMonMouseMove_SameSize
-#define MAYBE_RestoreEventState RestoreEventState
-#define MAYBE_MultiMonMouseMove MultiMonMouseMove
-#define MAYBE_DisconnectOnLocalInputTest DisconnectOnLocalInputTest
-#endif
-
 namespace remoting {
 
 using protocol::FakeSession;
@@ -312,22 +293,22 @@ void ClientSessionTest::NotifyVideoSizeAll() {
 
   int x_min, x_max, y_min, y_max;
   bool initialized = false;
-  for (DisplayGeometry disp : displays_.displays()) {
-    int disp_x_max = disp.x + disp.width;
-    int disp_y_max = disp.y + disp.height;
+  for (auto& disp : displays_.displays()) {
+    int disp_x_max = disp->x + disp->width;
+    int disp_y_max = disp->y + disp->height;
     if (!initialized) {
-      x_min = disp.x;
+      x_min = disp->x;
       x_max = disp_x_max;
-      y_min = disp.y;
+      y_min = disp->y;
       y_max = disp_y_max;
       initialized = true;
     } else {
-      if (disp.x < x_min)
-        x_min = disp.x;
+      if (disp->x < x_min)
+        x_min = disp->x;
       if (disp_x_max > x_max)
         x_max = disp_x_max;
-      if (disp.y < y_min)
-        y_min = disp.y;
+      if (disp->y < y_min)
+        y_min = disp->y;
       if (disp_y_max > y_max)
         y_max = disp_y_max;
     }
@@ -430,7 +411,7 @@ void ClientSessionTest::MultiMon_SelectAllDisplays() {
   NotifyVideoSizeAll();
 }
 
-TEST_F(ClientSessionTest, MAYBE_MultiMonMouseMove) {
+TEST_F(ClientSessionTest, MultiMonMouseMove) {
   CreateClientSession();
   ConnectClientSession();
   SetupMultiDisplay();
@@ -487,7 +468,7 @@ TEST_F(ClientSessionTest, MAYBE_MultiMonMouseMove) {
                                    kDisplay2Height + kDisplay2YOffset - 1));
 }
 
-TEST_F(ClientSessionTest, MAYBE_MultiMonMouseMove_SameSize) {
+TEST_F(ClientSessionTest, MultiMonMouseMove_SameSize) {
   CreateClientSession();
   ConnectClientSession();
   SetupMultiDisplay_SameSize();
@@ -543,7 +524,7 @@ TEST_F(ClientSessionTest, MAYBE_MultiMonMouseMove_SameSize) {
                                    kDisplay1Height + kDisplay2YOffset - 1));
 }
 
-TEST_F(ClientSessionTest, MAYBE_DisableInputs) {
+TEST_F(ClientSessionTest, DisableInputs) {
   CreateClientSession();
   ConnectClientSession();
   SetupSingleDisplay();
@@ -599,7 +580,7 @@ TEST_F(ClientSessionTest, MAYBE_DisableInputs) {
               EqualsClipboardEvent(kMimeTypeTextUtf8, "c"));
 }
 
-TEST_F(ClientSessionTest, MAYBE_LocalInputTest) {
+TEST_F(ClientSessionTest, LocalInputTest) {
   CreateClientSession();
   ConnectClientSession();
   SetupSingleDisplay();
@@ -639,7 +620,7 @@ TEST_F(ClientSessionTest, MAYBE_LocalInputTest) {
   // eventually (via dependency injection, not sleep!)
 }
 
-TEST_F(ClientSessionTest, MAYBE_DisconnectOnLocalInputTest) {
+TEST_F(ClientSessionTest, DisconnectOnLocalInputTest) {
   desktop_environment_options_.set_terminate_upon_input(true);
   CreateClientSession();
   ConnectClientSession();
@@ -650,7 +631,7 @@ TEST_F(ClientSessionTest, MAYBE_DisconnectOnLocalInputTest) {
   EXPECT_FALSE(connection_->is_connected());
 }
 
-TEST_F(ClientSessionTest, MAYBE_RestoreEventState) {
+TEST_F(ClientSessionTest, RestoreEventState) {
   CreateClientSession();
   ConnectClientSession();
   SetupSingleDisplay();
@@ -688,7 +669,7 @@ TEST_F(ClientSessionTest, MAYBE_RestoreEventState) {
   EXPECT_THAT(key_events[3], EqualsKeyEvent(2, false));
 }
 
-TEST_F(ClientSessionTest, MAYBE_ClampMouseEvents) {
+TEST_F(ClientSessionTest, ClampMouseEvents) {
   CreateClientSession();
   ConnectClientSession();
   SetupSingleDisplay();

remoting/host/desktop_capturer_proxy.cc

@@ -193,8 +193,9 @@ bool DesktopCapturerProxy::SelectSource(SourceId id_index) {
 
   SourceId id = -1;
   if (id_index >= 0 && id_index < desktop_display_info_->NumDisplays()) {
-    DisplayGeometry display = desktop_display_info_->displays()[id_index];
-    id = display.id;
+    const DisplayGeometry* display =
+        desktop_display_info_->displays()[id_index].get();
+    id = display->id;
   }
   capture_task_runner_->PostTask(
       FROM_HERE,
@@ -217,17 +218,17 @@ void DesktopCapturerProxy::OnFrameCaptured(
 
       auto layout = std::make_unique<protocol::VideoLayout>();
       LOG(INFO) << "DCP::OnFrameCaptured";
-      for (auto display : desktop_display_info_->displays()) {
+      for (auto& display : desktop_display_info_->displays()) {
         protocol::VideoTrackLayout* track = layout->add_video_track();
-        track->set_position_x(display.x);
-        track->set_position_y(display.y);
-        track->set_width(display.width);
-        track->set_height(display.height);
-        track->set_x_dpi(display.dpi);
-        track->set_y_dpi(display.dpi);
-        LOG(INFO) << "   Display: " << display.x << "," << display.y << " "
-                  << display.width << "x" << display.height << " @ "
-                  << display.dpi;
+        track->set_position_x(display->x);
+        track->set_position_y(display->y);
+        track->set_width(display->width);
+        track->set_height(display->height);
+        track->set_x_dpi(display->dpi);
+        track->set_y_dpi(display->dpi);
+        LOG(INFO) << "   Display: " << display->x << "," << display->y << " "
+                  << display->width << "x" << display->height << " @ "
+                  << display->dpi;
       }
       client_session_control_->OnDesktopDisplayChanged(std::move(layout));
     }

remoting/host/desktop_display_info.cc

@@ -20,16 +20,16 @@ DesktopDisplayInfo::~DesktopDisplayInfo() = default;
 bool DesktopDisplayInfo::operator==(const DesktopDisplayInfo& other) {
   if (other.displays_.size() == displays_.size()) {
     for (size_t display = 0; display < displays_.size(); display++) {
-      DisplayGeometry this_display = displays_[display];
-      DisplayGeometry other_display = other.displays_[display];
-      if (this_display.id != other_display.id ||
-          this_display.x != other_display.x ||
-          this_display.y != other_display.y ||
-          this_display.width != other_display.width ||
-          this_display.height != other_display.height ||
-          this_display.dpi != other_display.dpi ||
-          this_display.bpp != other_display.bpp ||
-          this_display.is_default != other_display.is_default) {
+      const DisplayGeometry* this_display = displays_[display].get();
+      const DisplayGeometry* other_display = other.displays_[display].get();
+      if (this_display->id != other_display->id ||
+          this_display->x != other_display->x ||
+          this_display->y != other_display->y ||
+          this_display->width != other_display->width ||
+          this_display->height != other_display->height ||
+          this_display->dpi != other_display->dpi ||
+          this_display->bpp != other_display->bpp ||
+          this_display->is_default != other_display->is_default) {
         return false;
       }
     }
@@ -69,7 +69,7 @@ int DesktopDisplayInfo::NumDisplays() {
 const DisplayGeometry* DesktopDisplayInfo::GetDisplayInfo(unsigned int id) {
   if (id < 0 || id >= displays_.size())
     return nullptr;
-  return &displays_[id];
+  return displays_[id].get();
 }
 
 // Calculate the offset from the origin of the desktop to the origin of the
@@ -118,18 +118,18 @@ webrtc::DesktopVector DesktopDisplayInfo::CalcDisplayOffset(
     return webrtc::DesktopVector();
   }
 
-  DisplayGeometry disp_info = displays_[disp_index];
-  webrtc::DesktopVector origin(disp_info.x, disp_info.y);
+  const DisplayGeometry* disp_info = displays_[disp_index].get();
+  webrtc::DesktopVector origin(disp_info->x, disp_info->y);
 
   // Find topleft-most display coordinate. This is the topleft of the desktop.
   int dx = 0;
   int dy = 0;
-  for (size_t id = 0; id < displays_.size(); id++) {
-    DisplayGeometry disp = displays_[id];
-    if (disp.x < dx)
-      dx = disp.x;
-    if (disp.y < dy)
-      dy = disp.y;
+  for (auto& display : displays_) {
+    const DisplayGeometry* disp = display.get();
+    if (disp->x < dx)
+      dx = disp->x;
+    if (disp->y < dy)
+      dy = disp->y;
   }
   webrtc::DesktopVector topleft(dx, dy);
 
@@ -149,12 +149,12 @@ webrtc::DesktopVector DesktopDisplayInfo::CalcDisplayOffset(
 #endif  // defined(OS_MACOSX)
 }
 
-void DesktopDisplayInfo::AddDisplay(DisplayGeometry* display) {
-  displays_.push_back(*display);
+void DesktopDisplayInfo::AddDisplay(std::unique_ptr<DisplayGeometry> display) {
+  displays_.push_back(std::move(display));
 }
 
 void DesktopDisplayInfo::AddDisplayFrom(protocol::VideoTrackLayout track) {
-  auto* display = new DisplayGeometry();
+  std::unique_ptr<DisplayGeometry> display(new DisplayGeometry());
   display->x = track.position_x();
   display->y = track.position_y();
   display->width = track.width();
@@ -162,7 +162,7 @@ void DesktopDisplayInfo::AddDisplayFrom(protocol::VideoTrackLayout track) {
   display->dpi = track.x_dpi();
   display->bpp = 24;
   display->is_default = false;
-  displays_.push_back(*display);
+  displays_.push_back(std::move(display));
 }
 
 #if !defined(OS_MACOSX)
@@ -172,8 +172,8 @@ void DesktopDisplayInfo::LoadCurrentDisplayInfo() {
 #if defined(OS_WIN)
   BOOL enum_result = TRUE;
   for (int device_index = 0;; ++device_index) {
-    DisplayGeometry info;
-    info.id = device_index;
+    std::unique_ptr<DisplayGeometry> info(new DisplayGeometry());
+    info->id = device_index;
 
     DISPLAY_DEVICE device = {};
     device.cb = sizeof(device);
@@ -187,9 +187,9 @@ void DesktopDisplayInfo::LoadCurrentDisplayInfo() {
     if (!(device.StateFlags & DISPLAY_DEVICE_ACTIVE))
       continue;
 
-    info.is_default = false;
+    info->is_default = false;
     if (device.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE)
-      info.is_default = true;
+      info->is_default = true;
 
     // Get additional info about device.
     DEVMODE devmode;
@@ -197,13 +197,13 @@ void DesktopDisplayInfo::LoadCurrentDisplayInfo() {
     EnumDisplaySettingsEx(device.DeviceName, ENUM_CURRENT_SETTINGS, &devmode,
                           0);
 
-    info.x = devmode.dmPosition.x;
-    info.y = devmode.dmPosition.y;
-    info.width = devmode.dmPelsWidth;
-    info.height = devmode.dmPelsHeight;
-    info.dpi = devmode.dmLogPixels;
-    info.bpp = devmode.dmBitsPerPel;
-    displays_.push_back(info);
+    info->x = devmode.dmPosition.x;
+    info->y = devmode.dmPosition.y;
+    info->width = devmode.dmPelsWidth;
+    info->height = devmode.dmPelsHeight;
+    info->dpi = devmode.dmLogPixels;
+    info->bpp = devmode.dmBitsPerPel;
+    displays_.push_back(std::move(info));
   }
 #endif  // OS_WIN
 }

remoting/host/desktop_display_info.h

@@ -41,7 +41,7 @@ class DesktopDisplayInfo {
   webrtc::DesktopVector CalcDisplayOffset(webrtc::ScreenId id);
 
   // Add a new display with the given info to the display list.
-  void AddDisplay(DisplayGeometry* display);
+  void AddDisplay(std::unique_ptr<DisplayGeometry> display);
 
   void AddDisplayFrom(protocol::VideoTrackLayout track);
 
@@ -51,10 +51,12 @@ class DesktopDisplayInfo {
   bool operator==(const DesktopDisplayInfo& other);
   bool operator!=(const DesktopDisplayInfo& other);
 
-  const std::vector<DisplayGeometry>& displays() const { return displays_; }
+  const std::vector<std::unique_ptr<DisplayGeometry>>& displays() const {
+    return displays_;
+  }
 
  private:
-  std::vector<DisplayGeometry> displays_;
+  std::vector<std::unique_ptr<DisplayGeometry>> displays_;
 
   DISALLOW_COPY_AND_ASSIGN(DesktopDisplayInfo);
 };

remoting/host/desktop_display_info_mac.mm

@@ -22,13 +22,13 @@ void DesktopDisplayInfo::LoadCurrentDisplayInfo() {
   int main_display_height = 0;
 
   for (NSUInteger i = 0; i < [screens count]; ++i) {
-    DisplayGeometry info;
+    std::unique_ptr<DisplayGeometry> info(new DisplayGeometry());
 
     NSScreen* screen = [screens objectAtIndex:i];
     NSDictionary* device = [screen deviceDescription];
     CGDirectDisplayID id = static_cast<CGDirectDisplayID>(
         [[device objectForKey:@"NSScreenNumber"] intValue]);
-    info.id = id;
+    info->id = id;
 
     float dsf = 1.0f;
     if ([screen respondsToSelector:@selector(backingScaleFactor)])
@@ -42,21 +42,21 @@ void DesktopDisplayInfo::LoadCurrentDisplayInfo() {
     if (i == 0) {
       DCHECK(x == 0);
       DCHECK(y == 0);
-      info.is_default = true;
+      info->is_default = true;
       main_display_height = height;
     } else {
-      info.is_default = false;
+      info->is_default = false;
     }
 
-    info.x = x;
+    info->x = x;
     // Convert origin from lower left to upper left (based on main display).
-    info.y = main_display_height - y - height;
-    info.width = bounds.size.width;
-    info.height = height;
-    info.dpi = (int)(kDefaultScreenDpi * dsf);
-    info.bpp = 24;
+    info->y = main_display_height - y - height;
+    info->width = bounds.size.width;
+    info->height = height;
+    info->dpi = (int)(kDefaultScreenDpi * dsf);
+    info->bpp = 24;
 
-    displays_.push_back(info);
+    displays_.push_back(std::move(info));
   }
 }
 

remoting/host/desktop_display_info_unittest.cc

@@ -8,28 +8,6 @@
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// TODO(crbug.com/961064): Fix memory leaks in tests and re-enable on LSAN.
-#ifdef LEAK_SANITIZER
-#define MAYBE_SingleDisplay DISABLED_SingleDisplay
-#define MAYBE_DualDisplayRight_ReverseOrder \
-  DISABLED_DualDisplayRight_ReverseOrder
-#define MAYBE_DualDisplayLeft_ReverseOrder DISABLED_DualDisplayLeft_ReverseOrder
-#define MAYBE_DualDisplayRight DISABLED_DualDisplayRight
-#define MAYBE_DualDisplayLeft DISABLED_DualDisplayLeft
-#define MAYBE_Multimon3 DISABLED_Multimon3
-#define MAYBE_TripleDisplayMiddle DISABLED_TripleDisplayMiddle
-#define MAYBE_Multimon7 DISABLED_Multimon7
-#else
-#define MAYBE_SingleDisplay SingleDisplay
-#define MAYBE_DualDisplayRight_ReverseOrder DualDisplayRight_ReverseOrder
-#define MAYBE_DualDisplayLeft_ReverseOrder DualDisplayLeft_ReverseOrder
-#define MAYBE_DualDisplayRight DualDisplayRight
-#define MAYBE_DualDisplayLeft DualDisplayLeft
-#define MAYBE_Multimon3 Multimon3
-#define MAYBE_TripleDisplayMiddle TripleDisplayMiddle
-#define MAYBE_Multimon7 Multimon7
-#endif
-
 namespace remoting {
 
 class DesktopDisplayInfoTest : public testing::Test {
@@ -43,7 +21,7 @@ class DesktopDisplayInfoTest : public testing::Test {
     display->dpi = 96;
     display->bpp = 24;
     display->is_default = false;
-    info_.AddDisplay(display);
+    info_.AddDisplay(std::unique_ptr<DisplayGeometry>(display));
   }
 
   void VerifyDisplayOffset(const base::Location& from_here,
@@ -64,7 +42,7 @@ class DesktopDisplayInfoTest : public testing::Test {
 // | 300x200 |
 // +---------+
 // o = desktop origin
-TEST_F(DesktopDisplayInfoTest, MAYBE_SingleDisplay) {
+TEST_F(DesktopDisplayInfoTest, SingleDisplay) {
   AddDisplay(0, 0, 300, 200);
 
   VerifyDisplayOffset(FROM_HERE, 0, 0, 0);
@@ -75,7 +53,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_SingleDisplay) {
 // | 300x200 | 500x400    |
 // +---------+            |
 //           +------------+
-TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayRight) {
+TEST_F(DesktopDisplayInfoTest, DualDisplayRight) {
   AddDisplay(0, 0, 300, 200);
   AddDisplay(300, 0, 500, 400);
 
@@ -88,7 +66,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayRight) {
 // | 300x200 | 500x400    |
 // +---------+            |
 //           +------------+
-TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayRight_ReverseOrder) {
+TEST_F(DesktopDisplayInfoTest, DualDisplayRight_ReverseOrder) {
   AddDisplay(300, 0, 500, 400);
   AddDisplay(0, 0, 300, 200);
 
@@ -101,7 +79,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayRight_ReverseOrder) {
 // | 300x200 | 500x400    |
 // +---------+            |
 //           +------------+
-TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayLeft) {
+TEST_F(DesktopDisplayInfoTest, DualDisplayLeft) {
   AddDisplay(0, 0, 500, 400);
   AddDisplay(-300, 0, 300, 200);
 
@@ -119,7 +97,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayLeft) {
 // | 300x200 | 500x400    |
 // +---------+            |
 //           +------------+
-TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayLeft_ReverseOrder) {
+TEST_F(DesktopDisplayInfoTest, DualDisplayLeft_ReverseOrder) {
   AddDisplay(-300, 0, 300, 200);
   AddDisplay(0, 0, 500, 400);
 
@@ -137,7 +115,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_DualDisplayLeft_ReverseOrder) {
 // | 300x200 | 500x400    | 2       |
 // +---------+            | 400x350 |
 //           +------------+---------+
-TEST_F(DesktopDisplayInfoTest, MAYBE_TripleDisplayMiddle) {
+TEST_F(DesktopDisplayInfoTest, TripleDisplayMiddle) {
   AddDisplay(-300, 0, 300, 200);
   AddDisplay(0, 0, 500, 400);  // Default display.
   AddDisplay(500, 50, 400, 350);
@@ -162,7 +140,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_TripleDisplayMiddle) {
 //                    +-----------+    - 950
 //  |         |       |   |       |
 // -300       0      300 500     900
-TEST_F(DesktopDisplayInfoTest, MAYBE_Multimon3) {
+TEST_F(DesktopDisplayInfoTest, Multimon3) {
   AddDisplay(0, 0, 500, 400);  // Default display.
   AddDisplay(300, 400, 600, 450);
   AddDisplay(-300, 350, 300, 200);
@@ -195,7 +173,7 @@ TEST_F(DesktopDisplayInfoTest, MAYBE_Multimon3) {
 //  -       -       0     6 7 8   1  1  1     1
 //  7       4             0 0 0   2  3  5     9
 //  0       0                     0  5  0     0
-TEST_F(DesktopDisplayInfoTest, MAYBE_Multimon7) {
+TEST_F(DesktopDisplayInfoTest, Multimon7) {
   AddDisplay(80, -10, 70, 60);
   AddDisplay(60, -50, 60, 40);
   AddDisplay(-70, 40, 30, 60);