Deflaky ScreenCapturerTest

ScreenCapturer tests may fail on trybot, so this change is to fix the issue.

Changes include,
1. Sometimes, a capturer may capture part of the change, i.e. usually the draw
actions are not atomic. So the updated_region may be inaccurate. So I have added
a MayDrawIncompleteShapes() function in ScreenDrawer. If it returns false, the
updated_region check will be ignored.
2. Several test cases may run concurrently, which makes one ScreenDrawer won't
really work. Its window may be covered by another ScreenDrawer. So I have added
a system wide lock to ensure only one ScreenDrawer is working at a certain time.
3. On unity (Linux), the top several pixels of a window may be covered by a
shadow effect if the window is not focused. So I have added a BringToFront()
function, and call it in WaitForPendingDraws().
4. On Windows, the drawn shapes are 'temporary drawing', which will be erased
once the window is covered by another one. So I repeat DrawRectangle() function
call in the test case.

TODO(zijiehe): The DISABLED_ prefixes will be added back after the code review.
And I will move these test cases into modules_test in a coming change.

BUG=647067

Review-Url: https://codereview.webrtc.org/2337073007
Cr-Commit-Position: refs/heads/master@{#14674}

webrtc/modules/BUILD.gn

@@ -551,6 +551,7 @@ if (rtc_include_tests) {
         "desktop_capture/screen_capturer_mac_unittest.cc",
         "desktop_capture/screen_capturer_mock_objects.h",
         "desktop_capture/screen_capturer_unittest.cc",
+        "desktop_capture/screen_drawer.cc",
         "desktop_capture/screen_drawer.h",
         "desktop_capture/screen_drawer_linux.cc",
         "desktop_capture/screen_drawer_mac.cc",

webrtc/modules/desktop_capture/screen_capturer_unittest.cc

@@ -51,12 +51,15 @@ ACTION_P(SaveUniquePtrArg, dest) {
 // Returns true if color in |rect| of |frame| is |color|.
 bool ArePixelsColoredBy(const DesktopFrame& frame,
                         DesktopRect rect,
-                        RgbaColor color) {
-  // updated_region() should cover the painted area.
-  DesktopRegion updated_region(frame.updated_region());
-  updated_region.IntersectWith(rect);
-  if (!updated_region.Equals(DesktopRegion(rect))) {
-    return false;
+                        RgbaColor color,
+                        bool may_partially_draw) {
+  if (!may_partially_draw) {
+    // updated_region() should cover the painted area.
+    DesktopRegion updated_region(frame.updated_region());
+    updated_region.IntersectWith(rect);
+    if (!updated_region.Equals(DesktopRegion(rect))) {
+      return false;
+    }
   }
 
   // Color in the |rect| should be |color|.
@@ -107,18 +110,28 @@ class ScreenCapturerTest : public testing::Test {
       capturer->Start(&callback_);
     }
 
-    // Draw a set of |kRectSize| by |kRectSize| rectangles at (|i|, |i|). One of
-    // (controlled by |c|) its primary colors is |i|, and the other two are
-    // 0xff. So we won't draw a white rectangle.
+    // Draw a set of |kRectSize| by |kRectSize| rectangles at (|i|, |i|), or
+    // |i| by |i| rectangles at (|kRectSize|, |kRectSize|). One of (controlled
+    // by |c|) its primary colors is |i|, and the other two are 0x7f. So we
+    // won't draw a black or white rectangle.
     for (int c = 0; c < 3; c++) {
+      // A fixed size rectangle.
       for (int i = 0; i < kTestArea - kRectSize; i += 16) {
         DesktopRect rect = DesktopRect::MakeXYWH(i, i, kRectSize, kRectSize);
         rect.Translate(drawer->DrawableRegion().top_left());
         RgbaColor color((c == 0 ? (i & 0xff) : 0x7f),
                         (c == 1 ? (i & 0xff) : 0x7f),
                         (c == 2 ? (i & 0xff) : 0x7f));
-        drawer->Clear();
-        drawer->DrawRectangle(rect, color);
+        TestCaptureOneFrame(capturers, drawer.get(), rect, color);
+      }
+
+      // A variable-size rectangle.
+      for (int i = 0; i < kTestArea - kRectSize; i += 16) {
+        DesktopRect rect = DesktopRect::MakeXYWH(kRectSize, kRectSize, i, i);
+        rect.Translate(drawer->DrawableRegion().top_left());
+        RgbaColor color((c == 0 ? (i & 0xff) : 0x7f),
+                        (c == 1 ? (i & 0xff) : 0x7f),
+                        (c == 2 ? (i & 0xff) : 0x7f));
         TestCaptureOneFrame(capturers, drawer.get(), rect, color);
       }
     }
@@ -166,9 +179,11 @@ class ScreenCapturerTest : public testing::Test {
                            ScreenDrawer* drawer,
                            DesktopRect rect,
                            RgbaColor color) {
-    size_t succeeded_capturers = 0;
     const int wait_capture_round = 600;
+    drawer->Clear();
+    size_t succeeded_capturers = 0;
     for (int i = 0; i < wait_capture_round; i++) {
+      drawer->DrawRectangle(rect, color);
       drawer->WaitForPendingDraws();
       for (size_t j = 0; j < capturers.size(); j++) {
         if (capturers[j] == nullptr) {
@@ -184,7 +199,8 @@ class ScreenCapturerTest : public testing::Test {
           return;
         }
 
-        if (ArePixelsColoredBy(*frame, rect, color)) {
+        if (ArePixelsColoredBy(
+            *frame, rect, color, drawer->MayDrawIncompleteShapes())) {
           capturers[j] = nullptr;
           succeeded_capturers++;
         }

webrtc/modules/desktop_capture/screen_drawer.cc

@@ -0,0 +1,30 @@
+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/modules/desktop_capture/screen_drawer.h"
+
+namespace webrtc {
+
+namespace {
+std::unique_ptr<ScreenDrawerLock> g_screen_drawer_lock;
+}  // namespace
+
+ScreenDrawerLock::ScreenDrawerLock() = default;
+ScreenDrawerLock::~ScreenDrawerLock() = default;
+
+ScreenDrawer::ScreenDrawer() {
+  g_screen_drawer_lock = ScreenDrawerLock::Create();
+}
+
+ScreenDrawer::~ScreenDrawer() {
+  g_screen_drawer_lock.reset();
+}
+
+}  // namespace webrtc

webrtc/modules/desktop_capture/screen_drawer.h

@@ -20,16 +20,31 @@
 
 namespace webrtc {
 
+// A cross-process lock to ensure only one ScreenDrawer can be used at a certain
+// time.
+class ScreenDrawerLock {
+ public:
+  virtual ~ScreenDrawerLock();
+
+  static std::unique_ptr<ScreenDrawerLock> Create();
+
+ protected:
+  ScreenDrawerLock();
+};
+
 // A set of basic platform dependent functions to draw various shapes on the
 // screen.
 class ScreenDrawer {
  public:
   // Creates a ScreenDrawer for the current platform, returns nullptr if no
   // ScreenDrawer implementation available.
+  // If the implementation cannot guarantee two ScreenDrawer instances won't
+  // impact each other, this function may block current thread until another
+  // ScreenDrawer has been destroyed.
   static std::unique_ptr<ScreenDrawer> Create();
 
-  ScreenDrawer() {}
-  virtual ~ScreenDrawer() {}
+  ScreenDrawer();
+  virtual ~ScreenDrawer();
 
   // Returns the region inside which DrawRectangle() function are expected to
   // work, in capturer coordinates (assuming ScreenCapturer::SelectScreen has
@@ -49,6 +64,12 @@ class ScreenDrawer {
   // ScreenCapturer should be able to capture the changes after this function
   // finish.
   virtual void WaitForPendingDraws() = 0;
+
+  // Returns true if incomplete shapes previous actions required may be drawn on
+  // the screen after a WaitForPendingDraws() call. i.e. Though the complete
+  // shapes will eventually be drawn on the screen, due to some OS limitations,
+  // these shapes may be partially appeared sometimes.
+  virtual bool MayDrawIncompleteShapes() = 0;
 };
 
 }  // namespace webrtc

webrtc/modules/desktop_capture/screen_drawer_linux.cc

@@ -8,6 +8,11 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <semaphore.h>
+#include <string.h>
+
 #include <memory>
 
 #include "webrtc/base/checks.h"
@@ -19,6 +24,30 @@ namespace webrtc {
 
 namespace {
 
+static constexpr char kSemaphoreName[] =
+    "/global-screen-drawer-linux-54fe5552-8047-11e6-a725-3f429a5b4fb4";
+
+class ScreenDrawerLockLinux : public ScreenDrawerLock {
+ public:
+  ScreenDrawerLockLinux();
+  ~ScreenDrawerLockLinux();
+
+ private:
+  sem_t* semaphore_;
+};
+
+ScreenDrawerLockLinux::ScreenDrawerLockLinux() {
+  semaphore_ =
+      sem_open(kSemaphoreName, O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO, 1);
+  sem_wait(semaphore_);
+}
+
+ScreenDrawerLockLinux::~ScreenDrawerLockLinux() {
+  sem_post(semaphore_);
+  sem_close(semaphore_);
+  // sem_unlink(kSemaphoreName);
+}
+
 // A ScreenDrawer implementation for X11.
 class ScreenDrawerLinux : public ScreenDrawer {
  public:
@@ -30,8 +59,13 @@ class ScreenDrawerLinux : public ScreenDrawer {
   void DrawRectangle(DesktopRect rect, RgbaColor color) override;
   void Clear() override;
   void WaitForPendingDraws() override;
+  bool MayDrawIncompleteShapes() override;
 
  private:
+  // Bring the window to the front, this can help to avoid the impact from other
+  // windows or shadow effect.
+  void BringToFront();
+
   rtc::scoped_refptr<SharedXDisplay> display_;
   int screen_num_;
   DesktopRect rect_;
@@ -80,6 +114,7 @@ ScreenDrawerLinux::ScreenDrawerLinux() {
                                 root_attributes.height);
   context_ = DefaultGC(display_->display(), screen_num_);
   colormap_ = DefaultColormap(display_->display(), screen_num_);
+  BringToFront();
   // Wait for window animations.
   SleepMs(200);
 }
@@ -121,8 +156,39 @@ void ScreenDrawerLinux::WaitForPendingDraws() {
   SleepMs(50);
 }
 
+bool ScreenDrawerLinux::MayDrawIncompleteShapes() {
+  return true;
+}
+
+void ScreenDrawerLinux::BringToFront() {
+  Atom state_above = XInternAtom(display_->display(), "_NET_WM_STATE_ABOVE", 1);
+  Atom window_state = XInternAtom(display_->display(), "_NET_WM_STATE", 1);
+  if (state_above == None || window_state == None) {
+    // Fallback to use XRaiseWindow, it's not reliable if two windows are both
+    // raise itself to the top.
+    XRaiseWindow(display_->display(), window_);
+    return;
+  }
+
+  XEvent event;
+  memset(&event, 0, sizeof(event));
+  event.type = ClientMessage;
+  event.xclient.window = window_;
+  event.xclient.message_type = window_state;
+  event.xclient.format = 32;
+  event.xclient.data.l[0] = 1;  // _NET_WM_STATE_ADD
+  event.xclient.data.l[1] = state_above;
+  XSendEvent(display_->display(), RootWindow(display_->display(), screen_num_),
+             False, SubstructureRedirectMask | SubstructureNotifyMask, &event);
+}
+
 }  // namespace
 
+// static
+std::unique_ptr<ScreenDrawerLock> ScreenDrawerLock::Create() {
+  return std::unique_ptr<ScreenDrawerLock>(new ScreenDrawerLockLinux());
+}
+
 // static
 std::unique_ptr<ScreenDrawer> ScreenDrawer::Create() {
   if (SharedXDisplay::CreateDefault().get()) {

webrtc/modules/desktop_capture/screen_drawer_mac.cc

@@ -14,6 +14,11 @@
 
 namespace webrtc {
 
+// static
+std::unique_ptr<ScreenDrawerLock> ScreenDrawerLock::Create() {
+  return nullptr;
+}
+
 // static
 std::unique_ptr<ScreenDrawer> ScreenDrawer::Create() {
   return nullptr;

webrtc/modules/desktop_capture/screen_drawer_win.cc

@@ -19,6 +19,36 @@ namespace webrtc {
 
 namespace {
 
+static constexpr TCHAR kMutexName[] =
+    TEXT("Local\\ScreenDrawerWin-da834f82-8044-11e6-ac81-73dcdd1c1869");
+
+class ScreenDrawerLockWin : public ScreenDrawerLock {
+ public:
+  ScreenDrawerLockWin();
+  ~ScreenDrawerLockWin();
+
+ private:
+  HANDLE mutex_;
+};
+
+ScreenDrawerLockWin::ScreenDrawerLockWin() {
+  while (true) {
+    mutex_ = CreateMutex(NULL, FALSE, kMutexName);
+    if (GetLastError() != ERROR_ALREADY_EXISTS && mutex_ != NULL) {
+      break;
+    } else {
+      if (mutex_) {
+        CloseHandle(mutex_);
+      }
+      SleepMs(1000);
+    }
+  }
+}
+
+ScreenDrawerLockWin::~ScreenDrawerLockWin() {
+  CloseHandle(mutex_);
+}
+
 DesktopRect GetScreenRect() {
   HDC hdc = GetDC(NULL);
   DesktopRect rect = DesktopRect::MakeWH(GetDeviceCaps(hdc, HORZRES),
@@ -51,8 +81,13 @@ class ScreenDrawerWin : public ScreenDrawer {
   void DrawRectangle(DesktopRect rect, RgbaColor color) override;
   void Clear() override;
   void WaitForPendingDraws() override;
+  bool MayDrawIncompleteShapes() override;
 
  private:
+  // Bring the window to the front, this can help to avoid the impact from other
+  // windows or shadow effects.
+  void BringToFront();
+
   // Draw a line with |color|.
   void DrawLine(DesktopVector start, DesktopVector end, RgbaColor color);
 
@@ -76,6 +111,7 @@ ScreenDrawerWin::ScreenDrawerWin()
   // Always use stock pen (DC_PEN) and brush (DC_BRUSH).
   SelectObject(hdc_, GetStockObject(DC_PEN));
   SelectObject(hdc_, GetStockObject(DC_BRUSH));
+  BringToFront();
 }
 
 ScreenDrawerWin::~ScreenDrawerWin() {
@@ -117,6 +153,10 @@ void ScreenDrawerWin::WaitForPendingDraws() {
   SleepMs(50);
 }
 
+bool ScreenDrawerWin::MayDrawIncompleteShapes() {
+  return true;
+}
+
 void ScreenDrawerWin::DrawLine(DesktopVector start,
                                DesktopVector end,
                                RgbaColor color) {
@@ -133,8 +173,28 @@ void ScreenDrawerWin::DrawDot(DesktopVector vect, RgbaColor color) {
   SetPixel(hdc_, vect.x(), vect.y(), ColorToRef(color));
 }
 
+void ScreenDrawerWin::BringToFront() {
+  if (SUCCEEDED(SetWindowPos(
+      window_, HWND_TOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE))) {
+    return;
+  }
+
+  long ex_style = GetWindowLong(window_, GWL_EXSTYLE);
+  ex_style |= WS_EX_TOPMOST;
+  if (SetWindowLong(window_, GWL_EXSTYLE, ex_style) != 0) {
+    return;
+  }
+
+  BringWindowToTop(window_);
+}
+
 }  // namespace
 
+// static
+std::unique_ptr<ScreenDrawerLock> ScreenDrawerLock::Create() {
+  return std::unique_ptr<ScreenDrawerLock>(new ScreenDrawerLockWin());
+}
+
 // static
 std::unique_ptr<ScreenDrawer> ScreenDrawer::Create() {
   return std::unique_ptr<ScreenDrawer>(new ScreenDrawerWin());