Use std::unique_ptr<> to pass frame ownership in DesktopCapturer impls.

webrtc/modules/desktop_capture/win/screen_capturer_win_gdi.cc

 /*
  *  Copyright (c) 2014 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 21 matching lines @@
 namespace {
 
 // Constants from dwmapi.h.
 const UINT DWM_EC_DISABLECOMPOSITION = 0;
 const UINT DWM_EC_ENABLECOMPOSITION = 1;
 
 const wchar_t kDwmapiLibraryName[] = L"dwmapi.dll";
 
 }  // namespace
 
-ScreenCapturerWinGdi::ScreenCapturerWinGdi(const DesktopCaptureOptions& options)
-    : callback_(NULL),
-      current_screen_id_(kFullDesktopScreenId),
-      desktop_dc_(NULL),
-      memory_dc_(NULL),
-      dwmapi_library_(NULL),
-      composition_func_(NULL),
-      set_thread_execution_state_failed_(false) {
+ScreenCapturerWinGdi::ScreenCapturerWinGdi(
+    const DesktopCaptureOptions& options) {
   if (options.disable_effects()) {
     // Load dwmapi.dll dynamically since it is not available on XP.
     if (!dwmapi_library_)
       dwmapi_library_ = LoadLibrary(kDwmapiLibraryName);
 
     if (dwmapi_library_) {
       composition_func_ = reinterpret_cast<DwmEnableCompositionFunc>(
           GetProcAddress(dwmapi_library_, "DwmEnableComposition"));
     }
   }
 }
 
 ScreenCapturerWinGdi::~ScreenCapturerWinGdi() {
   if (desktop_dc_)
-    ReleaseDC(NULL, desktop_dc_);
+    ReleaseDC(nullptr, desktop_dc_);

[Wez, 2016/05/18 01:29:50]

As per header comment, technically this should remain NULL.

[Sergey Ulanov, 2016/05/31 12:02:49]

Done.

   if (memory_dc_)
     DeleteDC(memory_dc_);
 
   // Restore Aero.
   if (composition_func_)
     (*composition_func_)(DWM_EC_ENABLECOMPOSITION);
 
   if (dwmapi_library_)
     FreeLibrary(dwmapi_library_);
 }
@@ skipping 15 matching lines @@
       set_thread_execution_state_failed_ = true;
       LOG_F(LS_WARNING) << "Failed to make system & display power assertion: "
                         << GetLastError();
     }
   }
 
   // Make sure the GDI capture resources are up-to-date.
   PrepareCaptureResources();
 
   if (!CaptureImage()) {
-    callback_->OnCaptureCompleted(NULL);
+    callback_->OnCaptureCompleted(std::unique_ptr<DesktopFrame>());
     return;
   }
 
   const DesktopFrame* current_frame = queue_.current_frame();
   const DesktopFrame* last_frame = queue_.previous_frame();
   if (last_frame && last_frame->size().equals(current_frame->size())) {
     // Make sure the differencer is set up correctly for these previous and
     // current screens.
     if (!differ_.get() ||
         (differ_->width() != current_frame->size().width()) ||
@@ skipping 52 matching lines @@
   // will restore Aero automatically if the process exits. This has no effect
   // under Windows 8 or higher.  See crbug.com/124018.
   if (composition_func_)
     (*composition_func_)(DWM_EC_DISABLECOMPOSITION);
 }
 
 void ScreenCapturerWinGdi::PrepareCaptureResources() {
   // Switch to the desktop receiving user input if different from the current
   // one.
   std::unique_ptr<Desktop> input_desktop(Desktop::GetInputDesktop());
-  if (input_desktop.get() != NULL && !desktop_.IsSame(*input_desktop)) {
+  if (input_desktop && !desktop_.IsSame(*input_desktop)) {
     // Release GDI resources otherwise SetThreadDesktop will fail.
     if (desktop_dc_) {
-      ReleaseDC(NULL, desktop_dc_);
-      desktop_dc_ = NULL;
+      ReleaseDC(nullptr, desktop_dc_);
+      desktop_dc_ = nullptr;
     }
 
     if (memory_dc_) {
       DeleteDC(memory_dc_);
-      memory_dc_ = NULL;
+      memory_dc_ = nullptr;
     }
 
     // If SetThreadDesktop() fails, the thread is still assigned a desktop.
     // So we can continue capture screen bits, just from the wrong desktop.
     desktop_.SetThreadDesktop(input_desktop.release());
 
     // Re-assert our vote to disable Aero.
     // See crbug.com/124018 and crbug.com/129906.
-    if (composition_func_ != NULL) {
+    if (composition_func_) {
       (*composition_func_)(DWM_EC_DISABLECOMPOSITION);
     }
   }
 
   // If the display bounds have changed then recreate GDI resources.
   // TODO(wez): Also check for pixel format changes.
   DesktopRect screen_rect(DesktopRect::MakeXYWH(
       GetSystemMetrics(SM_XVIRTUALSCREEN),
       GetSystemMetrics(SM_YVIRTUALSCREEN),
       GetSystemMetrics(SM_CXVIRTUALSCREEN),
       GetSystemMetrics(SM_CYVIRTUALSCREEN)));
   if (!screen_rect.equals(desktop_dc_rect_)) {
     if (desktop_dc_) {
-      ReleaseDC(NULL, desktop_dc_);
-      desktop_dc_ = NULL;
+      ReleaseDC(nullptr, desktop_dc_);
+      desktop_dc_ = nullptr;
     }
     if (memory_dc_) {
       DeleteDC(memory_dc_);
-      memory_dc_ = NULL;
+      memory_dc_ = nullptr;
     }
     desktop_dc_rect_ = DesktopRect();
   }
 
-  if (desktop_dc_ == NULL) {
-    assert(memory_dc_ == NULL);
+  if (!desktop_dc_) {
+    assert(!memory_dc_);
 
     // Create GDI device contexts to capture from the desktop into memory.
-    desktop_dc_ = GetDC(NULL);
+    desktop_dc_ = GetDC(nullptr);
     if (!desktop_dc_)
       abort();

[Wez, 2016/05/18 01:29:50]

How is if (!foo) abort() different from assert(foo)?

[Sergey Ulanov, 2016/05/31 12:02:49]

assert() is enabled only in debug builds.

capturers need to be updated to use RTC_[D]CHECK() instead of assert()/abort(),
but I'd prefer doing it in a separate CL.

[Wez, 2016/06/01 21:29:05]

Acknowledged.

     memory_dc_ = CreateCompatibleDC(desktop_dc_);
     if (!memory_dc_)
       abort();
 
     desktop_dc_rect_ = screen_rect;
 
     // Make sure the frame buffers will be reallocated.
     queue_.Reset();
 
     helper_.ClearInvalidRegion();
   }
 }
 
 bool ScreenCapturerWinGdi::CaptureImage() {
   DesktopRect screen_rect =
       GetScreenRect(current_screen_id_, current_device_key_);
   if (screen_rect.is_empty())
     return false;
 
   DesktopSize size = screen_rect.size();
   // If the current buffer is from an older generation then allocate a new one.
   // Note that we can't reallocate other buffers at this point, since the caller
   // may still be reading from them.
   if (!queue_.current_frame() ||
       !queue_.current_frame()->size().equals(screen_rect.size())) {
-    assert(desktop_dc_ != NULL);
-    assert(memory_dc_ != NULL);
+    assert(desktop_dc_);
+    assert(memory_dc_);

[Wez, 2016/05/18 01:29:50]

Why assert here, given that we already check that they're non-null earlier on?

[Sergey Ulanov, 2016/05/31 12:02:49]

It's not checked in this function, so I think this asserts are useful.

[Wez, 2016/06/01 21:29:05]

Acknowledged.

 
-    std::unique_ptr<DesktopFrame> buffer(DesktopFrameWin::Create(
-        size, shared_memory_factory_.get(), desktop_dc_));
+    std::unique_ptr<DesktopFrame> buffer = DesktopFrameWin::Create(
+        size, shared_memory_factory_.get(), desktop_dc_);
     if (!buffer)
       return false;
-    queue_.ReplaceCurrentFrame(SharedDesktopFrame::Wrap(buffer.release()));
+    queue_.ReplaceCurrentFrame(SharedDesktopFrame::Wrap(std::move(buffer)));
   }
 
   // Select the target bitmap into the memory dc and copy the rect from desktop
   // to memory.
   DesktopFrameWin* current = static_cast<DesktopFrameWin*>(
       queue_.current_frame()->GetUnderlyingFrame());
   HGDIOBJ previous_object = SelectObject(memory_dc_, current->bitmap());
-  if (previous_object != NULL) {
-    BitBlt(memory_dc_,
-           0, 0, screen_rect.width(), screen_rect.height(),
-           desktop_dc_,
-           screen_rect.left(), screen_rect.top(),
+  if (previous_object) {
+    BitBlt(memory_dc_, 0, 0, screen_rect.width(), screen_rect.height(),
+           desktop_dc_, screen_rect.left(), screen_rect.top(),
            SRCCOPY | CAPTUREBLT);
 
     // Select back the previously selected object to that the device contect
     // could be destroyed independently of the bitmap if needed.
     SelectObject(memory_dc_, previous_object);
   }
   return true;
 }
 
 }  // namespace webrtc