Reland of [WebRTC] A real ScreenCapturer test

Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of https://codereview.webrtc.org/2268093002/ )
>
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
>
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a new
> > ScreenDrawer interface is added to the code base to draw various shapes on the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps 50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
>
> TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,zijiehe@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
>
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asapersson@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644130
Committed: https://crrev.com/0f49daccbe3fc98bd791b2092fbea3a5907c7685
Cr-Commit-Position: refs/heads/master@{#14113}

[Hzj_jie, 2016-09-05 21:25:25]

The CQ bit was checked by zijiehe@chromium.org

[Hzj_jie, 2016-09-05 21:25:25]

Created Reland of [WebRTC] A real ScreenCapturer test

[commit-bot: I haz the power, 2016-09-05 21:25:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-05 21:25:38]

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

[commit-bot: I haz the power, 2016-09-05 21:25:38]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-webrtc-committers".
Note that this has nothing to do with OWNERS files.

[Hzj_jie, 2016-09-05 21:26:10]

The CQ bit was unchecked by zijiehe@chromium.org

[Hzj_jie, 2016-09-05 21:26:56]

Description was changed from

==========
Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of
https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of
https://codereview.webrtc.org/2268093002/ )
> 
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
> 
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a
new
> > ScreenDrawer interface is added to the code base to draw various shapes on
the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides
test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps
50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color
and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer
and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create
colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
> 
>
TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,ziji...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
> 
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asap...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=314516
==========

to

==========
Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of
https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of
https://codereview.webrtc.org/2268093002/ )
> 
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
> 
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a
new
> > ScreenDrawer interface is added to the code base to draw various shapes on
the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides
test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps
50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color
and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer
and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create
colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
> 
>
TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,ziji...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
> 
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asap...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644130
==========

[Hzj_jie, 2016-09-05 21:27:08]

zijiehe@chromium.org changed reviewers:
- asapersson@webrtc.org, jamiewalch@chromium.org, kjellander@chromium.org,
sergeyu@chromium.org

[Hzj_jie, 2016-09-05 22:52:53]

Patchset #2 (id:200001) has been deleted

[Hzj_jie, 2016-09-06 01:36:28]

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

[kjellander_webrtc, 2016-09-06 05:31:41]

kjellander@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[kjellander_webrtc, 2016-09-06 05:31:41]

lgtm (the dr memory full error is a different error, which I'll suppress in a
new CL)

[Hzj_jie, 2016-09-06 19:07:44]

On 2016/09/06 05:31:41, kjellander_webrtc wrote:
> lgtm (the dr memory full error is a different error, which I'll suppress in a
> new CL)

Oh, yes, I sent before win_drmemory_full has finished modules_unittests.

Sergey, do you have comments for this change? I applied your comment in change
2311683002 to this CL.

[Hzj_jie, 2016-09-06 22:19:59]

On 2016/09/06 19:07:44, Hzj_jie wrote:
> On 2016/09/06 05:31:41, kjellander_webrtc wrote:
> > lgtm (the dr memory full error is a different error, which I'll suppress in
a
> > new CL)
> 
> Oh, yes, I sent before win_drmemory_full has finished modules_unittests.
> 
> Sergey, do you have comments for this change? I applied your comment in change
> 2311683002 to this CL.

I am rerunning the win_drmemory_full tests to ensure it's not flaky. I will
submit this change once win_drmemory_full/modules_unittests passed, if you have
no other comments.

[Sergey Ulanov, 2016-09-06 23:34:19]

When relanding a reverted change the normal practice is to reopen the original
CR issue and reland from there instead of uploading the change again. That way
we have full history of the change in a single place.

[Hzj_jie, 2016-09-06 23:37:26]

On 2016/09/06 23:34:19, Sergey Ulanov wrote:
> When relanding a reverted change the normal practice is to reopen the original
> CR issue and reland from there instead of uploading the change again. That way
> we have full history of the change in a single place.

I see. I will update the original change.

[kjellander_chromium, 2016-09-07 05:22:17]

On 2016/09/06 23:37:26, Hzj_jie wrote:
> On 2016/09/06 23:34:19, Sergey Ulanov wrote:
> > When relanding a reverted change the normal practice is to reopen the
original
> > CR issue and reland from there instead of uploading the change again. That
way
> > we have full history of the change in a single place.
> 
> I see. I will update the original change.

Actually, there's no consensus regarding this AFAIK and it's been discussed
several times in the past:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/HspPND4XN08/vdkWP...
I personally don't like re-using a submitted CL, especially not if additional
patch sets are being added to it.

[Hzj_jie, 2016-09-07 18:51:49]

On 2016/09/07 05:22:17, kjellander_chromium wrote:
> On 2016/09/06 23:37:26, Hzj_jie wrote:
> > On 2016/09/06 23:34:19, Sergey Ulanov wrote:
> > > When relanding a reverted change the normal practice is to reopen the
> original
> > > CR issue and reland from there instead of uploading the change again. That
> way
> > > we have full history of the change in a single place.
> > 
> > I see. I will update the original change.
> 
> Actually, there's no consensus regarding this AFAIK and it's been discussed
> several times in the past:
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/HspPND4XN08/vdkWP...
> I personally don't like re-using a submitted CL, especially not if additional
> patch sets are being added to it.

Since linux_baremetal trybot has blocked change 2268093002 to be submitted
again, I will submit this one to unblock my following work.

[Hzj_jie, 2016-09-07 18:52:07]

The CQ bit was checked by zijiehe@chromium.org

[commit-bot: I haz the power, 2016-09-07 18:52:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-07 18:52:27]

Description was changed from

==========
Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of
https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of
https://codereview.webrtc.org/2268093002/ )
> 
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
> 
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a
new
> > ScreenDrawer interface is added to the code base to draw various shapes on
the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides
test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps
50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color
and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer
and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create
colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
> 
>
TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,ziji...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
> 
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asap...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644130
==========

to

==========
Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of
https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of
https://codereview.webrtc.org/2268093002/ )
> 
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
> 
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a
new
> > ScreenDrawer interface is added to the code base to draw various shapes on
the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides
test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps
50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color
and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer
and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create
colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
> 
>
TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,ziji...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
> 
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asap...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644130
==========

[commit-bot: I haz the power, 2016-09-07 18:52:28]

Committed patchset #2 (id:200002)

[commit-bot: I haz the power, 2016-09-07 18:52:32]

Description was changed from

==========
Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of
https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of
https://codereview.webrtc.org/2268093002/ )
> 
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
> 
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a
new
> > ScreenDrawer interface is added to the code base to draw various shapes on
the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides
test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps
50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color
and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer
and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create
colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
> 
>
TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,ziji...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
> 
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asap...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644130
==========

to

==========
Reland of [WebRTC] A real ScreenCapturer test (patchset #1 id:1 of
https://codereview.webrtc.org/2310953002/ )

Reason for revert:
Resubmit capturer tests

Original issue's description:
> Revert of [WebRTC] A real ScreenCapturer test (patchset #8 id:240001 of
https://codereview.webrtc.org/2268093002/ )
>
> Reason for revert:
> ScreenCapturerTest.CaptureUpdatedRegion fails on Win DrMemory Full.
>
> Original issue's description:
> > [WebRTC] A real ScreenCapturer test
> >
> > We do not have a real ScreenCapturer test before. And after CL 2210443002, a
new
> > ScreenDrawer interface is added to the code base to draw various shapes on
the
> > screen. This change is to use ScreenDrawer to test ScreenCapturer. Besides
test
> > cases, some other changes are included,
> >
> > 1. A WaitForPendingPaintings() function in ScreenDrawer, to wait for a
> > ScreenDrawer to finish all the pending draws. This function now only sleeps
50
> > milliseconds on X11 and 100 milliseconds on Windows.
> >
> > 2. A Color structure to help handle a big-endian or little-endian safe color
and
> > provide functions to compare with DesktopFrame::data(). Both ScreenDrawer
and
> > DesktopFrameGenerator (in change 2202443002) can use this class to create
colors
> > and compare with or paint to a DesktopFrame.
> >
> > 3. ScreenDrawer now uses Color structure instead of uint32_t.
> >
> > BUG=314516
> >
> > TBR=kjellander@chromium.org
> >
> > Committed: https://crrev.com/9d1c54ace0dc9f68da0152aa1ded2a8dba0a43ae
> > Cr-Commit-Position: refs/heads/master@{#14058}
>
>
TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,ziji...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=314516
>
> Committed: https://crrev.com/4c44202dc348613695a4b529bbd7c9bdab6195ec
> Cr-Commit-Position: refs/heads/master@{#14071}

TBR=sergeyu@chromium.org,jamiewalch@chromium.org,kjellander@chromium.org,asap...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644130

Committed: https://crrev.com/0f49daccbe3fc98bd791b2092fbea3a5907c7685
Cr-Commit-Position: refs/heads/master@{#14113}
==========

[commit-bot: I haz the power, 2016-09-07 18:52:34]

Patchset 2 (id:??) landed as
https://crrev.com/0f49daccbe3fc98bd791b2092fbea3a5907c7685
Cr-Commit-Position: refs/heads/master@{#14113}

[stefan-webrtc, 2016-09-14 15:37:08]

stefan@webrtc.org changed reviewers:
+ stefan@webrtc.org

[stefan-webrtc, 2016-09-14 15:37:09]

https://codereview.webrtc.org/2313653003/diff/200002/webrtc/modules/desktop_c...
File webrtc/modules/desktop_capture/screen_capturer_unittest.cc (right):

https://codereview.webrtc.org/2313653003/diff/200002/webrtc/modules/desktop_c...
webrtc/modules/desktop_capture/screen_capturer_unittest.cc:260: }
I'm not sure we want to add tests to modules_unittests which render things on
the screen. If nothing else, it's annoying when you run tests locally, but it
also seems to run for a longer time than what I think is acceptible for a
unittest (10 seconds on my machine). Can we at least move it to modules_tests?

[kjellander_webrtc, 2016-09-15 11:43:17]

On 2016/09/14 15:37:09, stefan-webrtc (holmer) wrote:
>
https://codereview.webrtc.org/2313653003/diff/200002/webrtc/modules/desktop_c...
> File webrtc/modules/desktop_capture/screen_capturer_unittest.cc (right):
> 
>
https://codereview.webrtc.org/2313653003/diff/200002/webrtc/modules/desktop_c...
> webrtc/modules/desktop_capture/screen_capturer_unittest.cc:260: }
> I'm not sure we want to add tests to modules_unittests which render things on
> the screen. If nothing else, it's annoying when you run tests locally, but it
> also seems to run for a longer time than what I think is acceptible for a
> unittest (10 seconds on my machine). Can we at least move it to modules_tests?

I agree with Stefan, the unittests target should generally only be for the
lower-level, really quick tests. Launching a windows and rendering things on the
screen shouldn't be in there IMHO. Stefan's suggestion of moving into
modules_tests sounds like a good first step. 

Please loop me in as reviewer on future test related CLs for this component.