[WebRTC] Add ScreenCapturerDifferWrapper to share Differ across ScreenCapturers

webrtc/modules/desktop_capture/screen_capturer_mock_objects.h

Index: webrtc/modules/desktop_capture/screen_capturer_mock_objects.h
diff --git a/webrtc/modules/desktop_capture/screen_capturer_mock_objects.h b/webrtc/modules/desktop_capture/screen_capturer_mock_objects.h
index 1f17e8c8be3e57b1caf53c5fe38d169ff5602708..24147fbf944ee747d636799f19c9c4a47cf1bf29 100644
--- a/webrtc/modules/desktop_capture/screen_capturer_mock_objects.h
+++ b/webrtc/modules/desktop_capture/screen_capturer_mock_objects.h
@@ -11,23 +11,83 @@
 #ifndef WEBRTC_MODULES_DESKTOP_CAPTURE_SCREEN_CAPTURER_MOCK_OBJECTS_H_
 #define WEBRTC_MODULES_DESKTOP_CAPTURE_SCREEN_CAPTURER_MOCK_OBJECTS_H_
 
+#include "webrtc/modules/desktop_capture/screen_capturer.h"
+
+#include <memory>
+
 #include "testing/gmock/include/gmock/gmock.h"
 #include "webrtc/base/constructormagic.h"
-#include "webrtc/modules/desktop_capture/screen_capturer.h"
+#include "webrtc/modules/desktop_capture/desktop_geometry.h"
+#include "webrtc/modules/desktop_capture/desktop_region.h"
+#include "webrtc/modules/desktop_capture/shared_memory.h"
 
 namespace webrtc {
 
 class MockScreenCapturer : public ScreenCapturer {

[Sergey Ulanov, 2016/08/03 23:19:12]

This class is not used anywhere right now. I suggest removing it completely and
adding FakeScreenCapturer that doesn't use on GMOCK.
There is also FakeDesktopCapturer in remoting/protocol/fake_desktop_capturer.h
which may be useful for you.

[Hzj_jie, 2016/08/04 02:28:07]

Yes, I have found fake_desktop_capturer in remoting/protocol. I have no
background information, but is there any reason we do not want to use GMock? I
think it's convenient EXPECT_CALL is helpful in this specific test case.

[Sergey Ulanov, 2016/08/12 05:25:50]

Canonical way to use gmock is to have a virtual interface stub without any
behavior and let the test how each call is handled. Here you are adding default
behavior for the interface via ON_CALL() in the .cc file. This is not what most
people would expect from Mock* class. (there was a somewhat related discussion
in chromium-dev:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/-KH_IP0rIWQ/dis...
).
So you can either
 1. Keep this class as is and move all behavior to the tests.
 2. Add a FakeScreenCapturer without GMock and use it instead.

I'd recommend 2 as it usually leads to simpler and more readable tests

[Hzj_jie, 2016/08/15 00:38:06]

Then I would rather choose the second one, as the implementation would be reused
in other test cases.

  public:
-  MockScreenCapturer() {}
-  virtual ~MockScreenCapturer() {}
+  MockScreenCapturer();
+  // TODO(zijiehe): Use override instead of virtual for overrided functions,
+  // once http://crbug.com/428099 has been addressed.
+  virtual ~MockScreenCapturer();
+
+  MOCK_METHOD1(Start, void(Callback*));
+  MOCK_METHOD1(Capture, void(const DesktopRegion&));
+  MOCK_METHOD1(GetScreenList, bool(ScreenList*));
+  MOCK_METHOD1(SelectScreen, bool(ScreenId));
+
+  // MOCK_METHOD* cannot accept std::unique_ptr as parameter.
+  virtual void SetSharedMemoryFactory(
+      std::unique_ptr<SharedMemoryFactory> shared_memory_factory);
+
+  // The size of the frame which will be returned in next Capture() callback.
+  DesktopSize* size() {
+    return &size_;
+  }
+
+  // The dirty region of the frame which will be returned in next Capture()
+  // callback. MockScreenCapturer will return a white frame, with black in the
+  // dirty_region_. Each Capture() function call with return_frame_ as true will
+  // consume dirty_region_.
+  DesktopRegion* dirty_region() {

[Sergey Ulanov, 2016/08/12 05:25:50]

call it updated_region or next_updated_region for consistency with DesktopFrame?

+    return &dirty_region_;
+  }
 
-  MOCK_METHOD1(Start, void(Callback* callback));
-  MOCK_METHOD1(Capture, void(const DesktopRegion& region));
-  MOCK_METHOD1(GetScreenList, bool(ScreenList* screens));
-  MOCK_METHOD1(SelectScreen, bool(ScreenId id));
+  // The result which will be returned in next Capture() callback.
+  Result* result() {
+    return &result_;
+  }
+
+  // Decides whether MockScreenCapturer returns a frame in next Capture()
+  // callback. If return_frame_ is true, MockScreenCapturer will create a frame
+  // according to both size_ and dirty_region_.
+  bool* return_frame() {

[Sergey Ulanov, 2016/08/03 23:19:12]

why do you need return bool pointer?

[Hzj_jie, 2016/08/04 02:28:07]

1. According to the coding style, we cannot return a reference.
2. A set / get function is not so good for dirty_region(), i.e. several
functions of DesktopRegion can impact it's state, I do not want to write them
all here. So I have to return DesktopRegion* dirty_region().
3. So to keep all the functions consistent, I decided to use pointers for all
these fields. Which is also same as protobuf.

[Sergey Ulanov, 2016/08/12 05:25:50]

What's wrong with [set_]dirty_region()?
You could have mutable_updated_region() as in DesktopFrame. Would that work
here?
I think this approach is rather confusing and inconsistent with most other
classes that have get/set methods.

+    return &return_frame_;
+  }
+
+  // Decides whether MockScreenCapturer returns a frame with dirty_region.
+  // MockScreenCapturer will keep DesktopFrame::dirty_region() empty if this
+  // field is false.
+  bool* provide_dirty_region_hints() {
+    return &provide_dirty_region_hints_;
+  }
+
+  // Decides whether MockScreenCapturer randomly enlarges dirty_region in the
+  // DesktopFrame. Setting this field to true to simulate an inaccurate dirty
+  // region from OS APIs.
+  bool* enlarge_dirty_region() {
+    return &enlarge_dirty_region_;
+  }
 
  private:
+  Callback* callback_;
+  std::unique_ptr<SharedMemoryFactory> shared_memory_factory_;
+  DesktopSize size_;
+  DesktopRegion dirty_region_;
+  Result result_;
+  bool return_frame_;
+  bool provide_dirty_region_hints_;
+  bool enlarge_dirty_region_;
+
   RTC_DISALLOW_COPY_AND_ASSIGN(MockScreenCapturer);
 };