Fix screen flickering for Windows platform…

Screen was flickering when the picker for desktop medias showed up in Windows platform. Keeping track of window size for each window so that BitBlt() instead of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857
Committed: https://crrev.com/77f3e0da5a2ad50dab6c1ebad490dad8a88a779a
Cr-Commit-Position: refs/heads/master@{#11721}

[Sergey Ulanov, 2016-02-18 18:49:59]

sergeyu@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Sergey Ulanov, 2016-02-18 18:50:00]

https://codereview.chromium.org/1705183002/diff/1/webrtc/modules/desktop_capt...
File webrtc/modules/desktop_capture/window_capturer_win.cc (right):

https://codereview.chromium.org/1705183002/diff/1/webrtc/modules/desktop_capt...
webrtc/modules/desktop_capture/window_capturer_win.cc:106: std::map<HWND,
DesktopSize> window_size_map_;
Add a comment to explain why we need to cache the size.

https://codereview.chromium.org/1705183002/diff/1/webrtc/modules/desktop_capt...
webrtc/modules/desktop_capture/window_capturer_win.cc:124:
windows->swap(result);
Currently windows never get removed from window_size_map_. Cleanup it here?

[GeorgeZ, 2016-02-19 19:26:59]

Description was changed from

==========
Screen was flickering when the picker for desktop medias showed up in Windows
platform. Keeping track of window size for each window so that BitBlt() instead
of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857
==========

to

==========
Screen was flickering when the picker for desktop medias showed up in Windows
platform. Keeping track of window size for each window so that BitBlt() instead
of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857
==========

[GeorgeZ, 2016-02-19 23:34:48]

Sergey,

I updated code based on your comments.

Thanks

https://codereview.chromium.org/1705183002/diff/1/webrtc/modules/desktop_capt...
File webrtc/modules/desktop_capture/window_capturer_win.cc (right):

https://codereview.chromium.org/1705183002/diff/1/webrtc/modules/desktop_capt...
webrtc/modules/desktop_capture/window_capturer_win.cc:106: std::map<HWND,
DesktopSize> window_size_map_;
On 2016/02/18 18:50:00, Sergey Ulanov wrote:
> Add a comment to explain why we need to cache the size.

Done.

https://codereview.chromium.org/1705183002/diff/1/webrtc/modules/desktop_capt...
webrtc/modules/desktop_capture/window_capturer_win.cc:124:
windows->swap(result);
On 2016/02/18 18:50:00, Sergey Ulanov wrote:
> Currently windows never get removed from window_size_map_. Cleanup it here?

Done.

[Sergey Ulanov, 2016-02-20 18:47:49]

https://codereview.chromium.org/1705183002/diff/20001/webrtc/modules/desktop_...
File webrtc/modules/desktop_capture/window_capturer_win.cc (right):

https://codereview.chromium.org/1705183002/diff/20001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:107: // JavaScript
fucntion ChooseDesktopMedia() leads SelectWindow() and Capture()
nit: no need mention JavaScript here. Just say that we want to avoid flickering
for the case when SelectWindow() calls are interleaved with Capture().

https://codereview.chromium.org/1705183002/diff/20001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:133: std::set<HWND> ids;
I think it would be better to just build a new map here instead of building a
set and then filtering entries from the existing map. 
I.e.:
std::map<HWND, DesktopSize> new_map;
for (const auto& hwnd : *windows) {
  new_map[hwnd] = window_size_map_[hwnd];
}
window_size_map_.swap(new_map);

This makes the code simpler, and it's likely to be faster.

[GeorgeZ, 2016-02-22 17:08:46]

Sergey,

I updated code based on your comments.

Thanks,

https://codereview.chromium.org/1705183002/diff/20001/webrtc/modules/desktop_...
File webrtc/modules/desktop_capture/window_capturer_win.cc (right):

https://codereview.chromium.org/1705183002/diff/20001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:107: // JavaScript
fucntion ChooseDesktopMedia() leads SelectWindow() and Capture()
On 2016/02/20 18:47:49, Sergey Ulanov wrote:
> nit: no need mention JavaScript here. Just say that we want to avoid
flickering
> for the case when SelectWindow() calls are interleaved with Capture(). 

Done.

https://codereview.chromium.org/1705183002/diff/20001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:133: std::set<HWND> ids;
On 2016/02/20 18:47:49, Sergey Ulanov wrote:
> I think it would be better to just build a new map here instead of building a
> set and then filtering entries from the existing map. 
> I.e.:
> std::map<HWND, DesktopSize> new_map;
> for (const auto& hwnd : *windows) {
>   new_map[hwnd] = window_size_map_[hwnd];
> }
> window_size_map_.swap(new_map);
> 
> This makes the code simpler, and it's likely to be faster.

Good idea.

[Sergey Ulanov, 2016-02-22 17:49:54]

LGTM. Thanks for fixing it.

https://codereview.chromium.org/1705183002/diff/40001/webrtc/modules/desktop_...
File webrtc/modules/desktop_capture/window_capturer_win.cc (right):

https://codereview.chromium.org/1705183002/diff/40001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:14: #include <set>
don't need this.

https://codereview.chromium.org/1705183002/diff/40001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:146: previous_size_ =
window_size_map_[window];
Note that this also has side-effect of inserting an empty size entry into the
map. Not a problem in this case.

[GeorgeZ, 2016-02-22 18:40:46]

The CQ bit was checked by gyzhou@chromium.org

[commit-bot: I haz the power, 2016-02-22 18:40:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1705183002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1705183002/40001

[commit-bot: I haz the power, 2016-02-22 18:41:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-22 18:41:44]

Try jobs failed on following builders:
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/5313)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/7640)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/7558)

[GeorgeZ, 2016-02-22 18:59:13]

Sergey,

Updated based your comments.

Thanks

https://codereview.chromium.org/1705183002/diff/40001/webrtc/modules/desktop_...
File webrtc/modules/desktop_capture/window_capturer_win.cc (right):

https://codereview.chromium.org/1705183002/diff/40001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:14: #include <set>
On 2016/02/22 17:49:54, Sergey Ulanov wrote:
> don't need this.

Done.

https://codereview.chromium.org/1705183002/diff/40001/webrtc/modules/desktop_...
webrtc/modules/desktop_capture/window_capturer_win.cc:146: previous_size_ =
window_size_map_[window];
On 2016/02/22 17:49:54, Sergey Ulanov wrote:
> Note that this also has side-effect of inserting an empty size entry into the
> map. Not a problem in this case.

Acknowledged.

[GeorgeZ, 2016-02-23 16:56:17]

The CQ bit was checked by gyzhou@chromium.org

[GeorgeZ, 2016-02-23 16:56:17]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/1705183002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-02-23 16:56:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1705183002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1705183002/60001

[commit-bot: I haz the power, 2016-02-23 16:57:49]

Description was changed from

==========
Screen was flickering when the picker for desktop medias showed up in Windows
platform. Keeping track of window size for each window so that BitBlt() instead
of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857
==========

to

==========
Screen was flickering when the picker for desktop medias showed up in Windows
platform. Keeping track of window size for each window so that BitBlt() instead
of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857
==========

[commit-bot: I haz the power, 2016-02-23 16:57:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-23 16:57:58]

Description was changed from

==========
Screen was flickering when the picker for desktop medias showed up in Windows
platform. Keeping track of window size for each window so that BitBlt() instead
of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857
==========

to

==========
Screen was flickering when the picker for desktop medias showed up in Windows
platform. Keeping track of window size for each window so that BitBlt() instead
of PrintWindow() will be called for windows with unchanged sizes.

BUG=472857

Committed: https://crrev.com/77f3e0da5a2ad50dab6c1ebad490dad8a88a779a
Cr-Commit-Position: refs/heads/master@{#11721}
==========

[commit-bot: I haz the power, 2016-02-23 16:57:59]

Patchset 4 (id:??) landed as
https://crrev.com/77f3e0da5a2ad50dab6c1ebad490dad8a88a779a
Cr-Commit-Position: refs/heads/master@{#11721}