Add new constructors for all DesktopFrame inheritances

This change adds constructors for all DesktopFrame inheritances to pass in
DesktopRect instead of DesktopSize.
Because the newly added constructors and DesktopFrame::top_left() function are
not actively used, this change should have no logic impact.

Bug: webrtc:7950
Change-Id: If78187865c991211dfc28d3723403ce6e6fe0290
Reviewed-on: https://chromium-review.googlesource.com/590508
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19204}

webrtc/modules/desktop_capture/cropped_desktop_frame.cc

@@ -17,6 +17,17 @@
 
 namespace webrtc {
 
+namespace {
+
+DesktopRect TranslateRect(const DesktopRect& rect,
+                          const DesktopVector& top_left) {
+  DesktopRect result = rect;
+  result.Translate(top_left);
+  return result;
+}
+
+}  // namespace
+
 // A DesktopFrame that is a sub-rect of another DesktopFrame.
 class CroppedDesktopFrame : public DesktopFrame {
  public:
@@ -48,7 +59,7 @@ std::unique_ptr<DesktopFrame> CreateCroppedDesktopFrame(
 
 CroppedDesktopFrame::CroppedDesktopFrame(std::unique_ptr<DesktopFrame> frame,
                                          const DesktopRect& rect)
-    : DesktopFrame(rect.size(),
+    : DesktopFrame(TranslateRect(rect, frame->top_left()),
                    frame->stride(),
                    frame->GetFrameDataAtPos(rect.top_left()),
                    frame->shared_memory()) {

webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc

@@ -79,7 +79,7 @@ DesktopFrameWithCursor::DesktopFrameWithCursor(
     std::unique_ptr<DesktopFrame> frame,
     const MouseCursor& cursor,
     const DesktopVector& position)
-    : DesktopFrame(frame->size(),
+    : DesktopFrame(frame->rect(),
                    frame->stride(),
                    frame->data(),
                    frame->shared_memory()) {

webrtc/modules/desktop_capture/desktop_frame.cc

@@ -16,6 +16,7 @@
 
 #include "webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/ptr_util.h"
 
 namespace webrtc {
 
@@ -37,7 +38,11 @@ DesktopFrame::DesktopFrame(DesktopRect rect,
       rect_(rect),
       stride_(stride),
       capture_time_ms_(0),
-      capturer_id_(DesktopCapturerId::kUnknown) {}
+      capturer_id_(DesktopCapturerId::kUnknown) {
+  RTC_DCHECK(stride_ >= 0);
+  RTC_DCHECK(rect.width() >= 0);
+  RTC_DCHECK(rect.height() >= 0);
+}
 
 DesktopFrame::~DesktopFrame() = default;
 
@@ -68,10 +73,12 @@ uint8_t* DesktopFrame::GetFrameDataAtPos(const DesktopVector& pos) const {
 }
 
 BasicDesktopFrame::BasicDesktopFrame(DesktopSize size)
-    : DesktopFrame(size, kBytesPerPixel * size.width(),
+    : BasicDesktopFrame(DesktopRect::MakeSize(size)) {}
-                   new uint8_t[kBytesPerPixel * size.width() * size.height()],
+
-                   NULL) {
+BasicDesktopFrame::BasicDesktopFrame(DesktopRect rect)
-}
+    : DesktopFrame(rect, kBytesPerPixel * rect.width(),
+                   new uint8_t[kBytesPerPixel * rect.width() * rect.height()],
+                   nullptr) {}
 
 BasicDesktopFrame::~BasicDesktopFrame() {
   delete[] data_;
@@ -95,29 +102,45 @@ DesktopFrame* BasicDesktopFrame::CopyOf(const DesktopFrame& frame) {
 std::unique_ptr<DesktopFrame> SharedMemoryDesktopFrame::Create(
     DesktopSize size,
     SharedMemoryFactory* shared_memory_factory) {
-  size_t buffer_size = size.height() * size.width() * kBytesPerPixel;
+  return Create(DesktopRect::MakeSize(size), shared_memory_factory);
+}
+
+// static
+std::unique_ptr<DesktopFrame> SharedMemoryDesktopFrame::Create(
+    DesktopRect rect,
+    SharedMemoryFactory* shared_memory_factory) {
+  RTC_DCHECK(shared_memory_factory);
+
+  size_t buffer_size = rect.height() * rect.width() * kBytesPerPixel;
   std::unique_ptr<SharedMemory> shared_memory =
       shared_memory_factory->CreateSharedMemory(buffer_size);
   if (!shared_memory)
     return nullptr;
 
-  return Create(size, std::move(shared_memory));
+  return rtc::MakeUnique<SharedMemoryDesktopFrame>(
-}
+      rect, rect.width() * kBytesPerPixel, std::move(shared_memory));
-
-// static
-std::unique_ptr<DesktopFrame> SharedMemoryDesktopFrame::Create(
-    DesktopSize size,
-    std::unique_ptr<SharedMemory> shared_memory) {
-  RTC_DCHECK(shared_memory);
-  int stride = size.width() * kBytesPerPixel;
-  return std::unique_ptr<DesktopFrame>(new SharedMemoryDesktopFrame(
-      size, stride, shared_memory.release()));
 }
 
 SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(DesktopSize size,
                                                    int stride,
                                                    SharedMemory* shared_memory)
-    : DesktopFrame(size,
+    : SharedMemoryDesktopFrame(DesktopRect::MakeSize(size),
+                               stride,
+                               shared_memory) {}
+
+SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(
+    DesktopRect rect,
+    int stride,
+    std::unique_ptr<SharedMemory> shared_memory)
+    : SharedMemoryDesktopFrame(rect,
+                               stride,
+                               shared_memory.release()) {}
+
+SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(
+    DesktopRect rect,
+    int stride,
+    SharedMemory* shared_memory)
+    : DesktopFrame(rect,
                    stride,
                    reinterpret_cast<uint8_t*>(shared_memory->data()),
                    shared_memory) {}

webrtc/modules/desktop_capture/desktop_frame.h

@@ -86,6 +86,9 @@ class DesktopFrame {
 
  protected:
   // Deprecated, use the constructor below.
+  // TODO(zijiehe): Remove deprecated constructors. DesktopFrame should describe
+  // its own position in the system. See
+  // https://bugs.chromium.org/p/webrtc/issues/detail?id=7950
   DesktopFrame(DesktopSize size,
                int stride,
                uint8_t* data,
@@ -118,7 +121,12 @@ class DesktopFrame {
 // A DesktopFrame that stores data in the heap.
 class BasicDesktopFrame : public DesktopFrame {
  public:
+  // Deprecated, use the next constructor.
   explicit BasicDesktopFrame(DesktopSize size);
+
+  // Preferred.
+  explicit BasicDesktopFrame(DesktopRect rect);
+
   ~BasicDesktopFrame() override;
 
   // Creates a BasicDesktopFrame that contains copy of |frame|.
@@ -131,23 +139,44 @@ class BasicDesktopFrame : public DesktopFrame {
 // A DesktopFrame that stores data in shared memory.
 class SharedMemoryDesktopFrame : public DesktopFrame {
  public:
+  // May return nullptr if |shared_memory_factory| failed to create a
+  // SharedMemory instance.
+  // |shared_memory_factory| should not be nullptr.
+  // Deprecated, use the next Create() function.
   static std::unique_ptr<DesktopFrame> Create(
       DesktopSize size,
       SharedMemoryFactory* shared_memory_factory);
 
+  // Preferred.
   static std::unique_ptr<DesktopFrame> Create(
-      DesktopSize size,
+      DesktopRect rect,
-      std::unique_ptr<SharedMemory> shared_memory);
+      SharedMemoryFactory* shared_memory_factory);
 
   // Takes ownership of |shared_memory|.
-  // TODO(zijiehe): Hide constructors after fake_desktop_capturer.cc has been
+  // Deprecated, use the next constructor.
-  // migrated, Create() is preferred.
   SharedMemoryDesktopFrame(DesktopSize size,
                            int stride,
                            SharedMemory* shared_memory);
+
+  // Preferred.
+  SharedMemoryDesktopFrame(DesktopRect rect,
+                           int stride,
+                           std::unique_ptr<SharedMemory> shared_memory);
+
   ~SharedMemoryDesktopFrame() override;
 
  private:
+  // Avoid unexpected order of parameter evaluation.
+  // Executing both std::unique_ptr<T>::operator->() and
+  // std::unique_ptr<T>::release() in the member initializer list is not safe.
+  // Depends on the order of parameter evaluation,
+  // std::unique_ptr<T>::operator->() may trigger assertion failure if it has
+  // been evaluated after std::unique_ptr<T>::release(). By using this
+  // constructor, std::unique_ptr<T>::operator->() won't be involved anymore.
+  SharedMemoryDesktopFrame(DesktopRect rect,
+                           int stride,
+                           SharedMemory* shared_memory);
+
   RTC_DISALLOW_COPY_AND_ASSIGN(SharedMemoryDesktopFrame);
 };
 

webrtc/modules/desktop_capture/shared_desktop_frame.cc

@@ -47,7 +47,7 @@ bool SharedDesktopFrame::IsShared() {
 }
 
 SharedDesktopFrame::SharedDesktopFrame(rtc::scoped_refptr<Core> core)
-    : DesktopFrame((*core)->size(),
+    : DesktopFrame((*core)->rect(),
                    (*core)->stride(),
                    (*core)->data(),
                    (*core)->shared_memory()),