[WebRTC] Add TwoCapturers test and TwoMagnifierCapturers test

webrtc/modules/desktop_capture/screen_capturer_unittest.cc

 /*
  *  Copyright (c) 2013 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.
  */
 
@@ skipping 102 matching lines @@
         drawer->DrawableRegion().height() < kTestArea) {
       LOG(LS_WARNING) << "ScreenDrawer::DrawableRegion() is too small for the "
                          "CaptureUpdatedRegion tests.";
       return;
     }
 
     for (ScreenCapturer* capturer : capturers) {
       capturer->Start(&callback_);
     }
 
     for (int c = 0; c < 3; c++) {

[Sergey Ulanov, 2016/09/09 17:23:15]

Where does 3 come from? Maybe add a comment here with a high-level description
of what happens here.

[Hzj_jie, 2016/09/12 23:13:40]

Done.

       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);
 
-        const int wait_first_capture_round = 20;
-        for (int j = 0; j < wait_first_capture_round; j++) {
+        std::vector<ScreenCapturer*> test_capturers(capturers);
+        size_t succeeded_capturers = 0;
+        const int wait_capture_round = 20;
+        for (int j = 0; j < wait_capture_round; j++) {

[Sergey Ulanov, 2016/09/09 17:23:15]

Maybe move this code to a separate function to make it easier to parse? There
are 4 nested for loops here, which makes it hard to understand what's going on.

[Hzj_jie, 2016/09/12 23:13:41]

Sure.

           drawer->WaitForPendingDraws();
-          std::unique_ptr<DesktopFrame> frame =
-              CaptureFrame(*capturers.begin(), &callback_);
-          if (!frame) {
-            return;
+          for (size_t k = 0; k < test_capturers.size(); k++) {
+            if (test_capturers[k] == nullptr) {
+              // TODO(zijiehe): ScreenCapturerX11 and ScreenCapturerWinGdi
+              // cannot capture a correct frame again if screen does not update.

[Sergey Ulanov, 2016/09/09 17:23:14]

I don't understand this comment. All capturers should be capture a new frame
even if nothing has changed. They will just keep updated_region() empty.
Also TODO() normally should say what needs to be done. 

[Hzj_jie, 2016/09/12 23:13:40]

Emmm, updated.

+              continue;
+            }
+            std::unique_ptr<DesktopFrame> frame =
+                CaptureFrame(test_capturers[k], &callback_);
+            if (!frame) {
+              return;

[Sergey Ulanov, 2016/09/09 17:23:15]

Is it appropriate to just return if one of the capturer fails? It means that the
test will pass, and this doesn't look right to me.

[Hzj_jie, 2016/09/12 23:13:41]

Not really. CaptureFrame triggers an assertion failure. But I should add comment
to explain, I remember Jamie asked same question before.

+            }
+
+            if (ArePixelsColoredBy(*frame, rect, color)) {
+              test_capturers[k] = nullptr;
+              succeeded_capturers++;
+            }
           }
 
-          if (ArePixelsColoredBy(*frame, rect, color)) {
-            // The first capturer successfully captured the frame we expected.
-            // So the others should also be able to capture it.
+          if (succeeded_capturers == test_capturers.size()) {
             break;
-          } else {
-            ASSERT_LT(j, wait_first_capture_round);
-          }
-        }
-
-        for (ScreenCapturer* capturer : capturers) {
-          if (capturer == *capturers.begin()) {
-            // TODO(zijiehe): ScreenCapturerX11 and ScreenCapturerWinGdi cannot
-            // capture a correct frame again if screen does not update.
-            continue;
-          }
-          std::unique_ptr<DesktopFrame> frame =
-              CaptureFrame(capturer, &callback_);
-          if (!frame) {
-            return;
           }
 
-          ASSERT_TRUE(ArePixelsColoredBy(*frame, rect, color));
+          ASSERT_LT(j, wait_capture_round);
         }
       }
     }
   }
 
   void TestCaptureUpdatedRegion() {
     TestCaptureUpdatedRegion({capturer_.get()});
   }
 
 #if defined(WEBRTC_WIN)
   bool SetDirectxCapturerMode() {
     if (!ScreenCapturerWinDirectx::IsSupported()) {
       LOG(LS_WARNING) << "Directx capturer is not supported";
       return false;
     }
 
     DesktopCaptureOptions options(DesktopCaptureOptions::CreateDefault());
     options.set_allow_directx_capturer(true);
     capturer_.reset(ScreenCapturer::Create(options));
     return true;
   }
+
+  void SetMagnifierMode() {
+    DesktopCaptureOptions options(DesktopCaptureOptions::CreateDefault());
+    options.set_allow_use_magnification_api(true);
+    capturer_.reset(ScreenCapturer::Create(options));

[Sergey Ulanov, 2016/09/09 17:23:15]

It doesn't look right that a method called Set*() create a screen capturer.
Rename it to CreateMagnifierCapturer()?

[Hzj_jie, 2016/09/12 23:13:40]

Done.

+  }
 #endif  // defined(WEBRTC_WIN)
 
   std::unique_ptr<ScreenCapturer> capturer_;
   MockScreenCapturerCallback callback_;
 };
 
 class FakeSharedMemory : public SharedMemory {
  public:
   FakeSharedMemory(char* buffer, size_t size)
     : SharedMemory(buffer, size, 0, kTestSharedMemoryId),
@@ skipping 57 matching lines @@
   ASSERT_TRUE(!it.IsAtEnd());
   EXPECT_TRUE(it.rect().equals(DesktopRect::MakeSize(frame->size())));
   it.Advance();
   EXPECT_TRUE(it.IsAtEnd());
 }
 
 TEST_F(ScreenCapturerTest, CaptureUpdatedRegion) {
   TestCaptureUpdatedRegion();
 }
 
+TEST_F(ScreenCapturerTest, TwoCapturers) {
+  std::unique_ptr<ScreenCapturer> capturer2(capturer_.release());
+  SetUp();
+  TestCaptureUpdatedRegion({capturer_.get(), capturer2.get()});
+}
+
 #if defined(WEBRTC_WIN)
 
 TEST_F(ScreenCapturerTest, UseSharedBuffers) {
   std::unique_ptr<DesktopFrame> frame;
   EXPECT_CALL(callback_,
               OnCaptureResultPtr(DesktopCapturer::Result::SUCCESS, _))
       .WillOnce(SaveUniquePtrArg(&frame));
 
   capturer_->Start(&callback_);
   capturer_->SetSharedMemoryFactory(
       std::unique_ptr<SharedMemoryFactory>(new FakeSharedMemoryFactory()));
   capturer_->Capture(DesktopRegion());
 
   ASSERT_TRUE(frame);
   ASSERT_TRUE(frame->shared_memory());
   EXPECT_EQ(frame->shared_memory()->id(), kTestSharedMemoryId);
 }
 
 TEST_F(ScreenCapturerTest, UseMagnifier) {
-  DesktopCaptureOptions options(DesktopCaptureOptions::CreateDefault());
-  options.set_allow_use_magnification_api(true);
-  capturer_.reset(ScreenCapturer::Create(options));
+  SetMagnifierMode();
 
   std::unique_ptr<DesktopFrame> frame;
   EXPECT_CALL(callback_,
               OnCaptureResultPtr(DesktopCapturer::Result::SUCCESS, _))
       .WillOnce(SaveUniquePtrArg(&frame));
 
   capturer_->Start(&callback_);
   capturer_->Capture(DesktopRegion());
   ASSERT_TRUE(frame);
 }
@@ skipping 43 matching lines @@
 TEST_F(ScreenCapturerTest, TwoDirectxCapturers) {
   if (!SetDirectxCapturerMode()) {
     return;
   }
 
   std::unique_ptr<ScreenCapturer> capturer2(capturer_.release());
   RTC_CHECK(SetDirectxCapturerMode());
   TestCaptureUpdatedRegion({capturer_.get(), capturer2.get()});
 }
 
+TEST_F(ScreenCapturerTest, TwoMagnifierCapturers) {
+  SetMagnifierMode();
+  std::unique_ptr<ScreenCapturer> capturer2(capturer_.release());

[Sergey Ulanov, 2016/09/09 17:23:15]

capturer2 = std::move(capturer_);

[Hzj_jie, 2016/09/12 23:13:41]

Done.

+  SetMagnifierMode();
+  TestCaptureUpdatedRegion({capturer_.get(), capturer2.get()});
+}
+
 #endif  // defined(WEBRTC_WIN)
 
 }  // namespace webrtc