Crash in DirectX capturer

webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc

 /*
  *  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.
  */
 
@@ skipping 279 matching lines @@
       contexts_[i] = nullptr;
       return;
     }
   }
 
   RTC_NOTREACHED();
 }
 
 void DxgiOutputDuplicator::SpreadContextChange(const Context* const source) {
   for (Context* dest : contexts_) {
-    if (dest != source) {
+    if (dest && dest != source) {

[Jamie, 2016/11/11 00:00:41]

Under what circumstances does it makes sense to allow nulls in contexts_? Could
we instead make sure that doesn't happen?

[Hzj_jie, 2016/11/11 00:10:51]

It's a performance trick, and totally expected. You may refer to line 290.

More background,
The Context is designed as a bridge between singleton (DXGI APIs) and multiton
(ScreenCapturerWinDirectx). Each ScreenCapturerWinDirectx has its own state,
since it's double-buffered, and we do not want to copy entire frame each time
for performance concern. But DxgiDuplicatorController is singleton because of
the restriction of DXGI APIs.
So once a ScreenCapturerWinDirectx is created, it needs to register its Context
object in DxgiDuplicatorController to get updated region information during each
capturer call. And once it's destroyed, its context needs to be removed. So in
line 290, there is a small trick to avoid removing element from a vector (O(1)
vs O(n)). So we need to have this check here.

[Jamie, 2016/11/11 00:15:21]

Thanks for the context. I think my question changes in that case to whether or
not that performance optimization is worthwhile. How large is the vector
expected to grow, and how often is Unregister called? If it's not on the
critical path, I would prefer we just clean up the vector properly, otherwise
we're leaving ourselves open to bugs like this in the future.

       dest->updated_region.AddRegion(source->updated_region);
     }
   }
 }
 
 DesktopRect DxgiOutputDuplicator::SourceRect(DesktopRect rect) {
   // |texture_|->AsDesktopFrame() starts from (0, 0).
   rect.Translate(-desktop_rect_.left(), -desktop_rect_.top());
   return rect;
 }
 
 DesktopRect DxgiOutputDuplicator::TargetRect(DesktopRect rect,
                                              DesktopVector offset) {
   rect = SourceRect(rect);
   rect.Translate(offset);
   return rect;
 }
 
 }  // namespace webrtc