Add DisplayCompositorTest.

Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
Committed: https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461
Cr-Commit-Position: refs/heads/master@{#435840}

[kylechar, 2016-11-18 19:37:18]

Description was changed from

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
==========

to

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[kylechar, 2016-11-18 19:38:25]

kylechar@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2016-11-18 20:18:24]

lgtm + comment.

https://codereview.chromium.org/2520513002/diff/1/services/ui/surfaces/displa...
File services/ui/surfaces/display_compositor.h (right):

https://codereview.chromium.org/2520513002/diff/1/services/ui/surfaces/displa...
services/ui/surfaces/display_compositor.h:72: cc::SurfaceReferenceManager*
reference_manager_;
Can we avoid having two references to the same thing? SurfaceManager is a
SurfaceReferenceManager?

[kylechar, 2016-11-24 20:40:53]

Patchset #4 (id:60001) has been deleted

[kylechar, 2016-11-24 20:43:15]

https://codereview.chromium.org/2520513002/diff/1/services/ui/surfaces/displa...
File services/ui/surfaces/display_compositor.h (right):

https://codereview.chromium.org/2520513002/diff/1/services/ui/surfaces/displa...
services/ui/surfaces/display_compositor.h:72: cc::SurfaceReferenceManager*
reference_manager_;
On 2016/11/18 20:18:24, Fady Samuel wrote:
> Can we avoid having two references to the same thing? SurfaceManager is a
> SurfaceReferenceManager?

Not easily, at least from what I can think of. We need the whole SurfaceManager
public API available from DisplayCompositor. Making all of SurfaceManager into
an interface doesn't sound ideal just to write tests a bit easier.

Alternatively I could write code to generate
SurfaceFactories/Surfaces/SurfaceIds for tests so the real SurfaceManager is
used. Again, that isn't perfect and results in more complicated tests.

[kylechar, 2016-11-28 15:30:25]

kylechar@chromium.org changed reviewers:
+ enne@chromium.org, sky@chromium.org

[kylechar, 2016-11-28 15:30:25]

+enne for cc/surfaces/*

+sky for services/ui/ws/BUILD.gn. Not sure how you feel about adding the test to
mus_ws_unittests. I can create something like mus_surfaces_unittests for it but
I think DisplayCompositor is going to move soonish anyways.

[sky, 2016-11-28 16:30:07]

On 2016/11/28 15:30:25, kylechar wrote:
> +enne for cc/surfaces/*
> 
> +sky for services/ui/ws/BUILD.gn. Not sure how you feel about adding the test
to
> mus_ws_unittests. I can create something like mus_surfaces_unittests for it
but
> I think DisplayCompositor is going to move soonish anyways.

Where will it move to? I don't think having a surface test in mus_ws_unittests
is the right place. But if this is temporary for a couple of days I'm ok with
it.

[Fady Samuel, 2016-11-28 16:33:44]

On 2016/11/28 16:30:07, sky wrote:
> On 2016/11/28 15:30:25, kylechar wrote:
> > +enne for cc/surfaces/*
> > 
> > +sky for services/ui/ws/BUILD.gn. Not sure how you feel about adding the
test
> to
> > mus_ws_unittests. I can create something like mus_surfaces_unittests for it
> but
> > I think DisplayCompositor is going to move soonish anyways.
> 
> Where will it move to? I don't think having a surface test in mus_ws_unittests
> is the right place. But if this is temporary for a couple of days I'm ok with
> it.

I plan to move DisplayCompositor to a central location to be used by all
platforms: likely cc/surfaces or components/display_compositor.

[sky, 2016-11-28 16:40:01]

On 2016/11/28 16:33:44, Fady Samuel wrote:
> On 2016/11/28 16:30:07, sky wrote:
> > On 2016/11/28 15:30:25, kylechar wrote:
> > > +enne for cc/surfaces/*
> > > 
> > > +sky for services/ui/ws/BUILD.gn. Not sure how you feel about adding the
> test
> > to
> > > mus_ws_unittests. I can create something like mus_surfaces_unittests for
it
> > but
> > > I think DisplayCompositor is going to move soonish anyways.
> > 
> > Where will it move to? I don't think having a surface test in
mus_ws_unittests
> > is the right place. But if this is temporary for a couple of days I'm ok
with
> > it.
> 
> I plan to move DisplayCompositor to a central location to be used by all
> platforms: likely cc/surfaces or components/display_compositor.

Ok, LGTM

[kylechar, 2016-11-30 15:40:31]

Think you'll have a chance to look today enne?

[kylechar, 2016-11-30 22:48:03]

kylechar@chromium.org changed reviewers:
+ vmpstr@chromium.org
- enne@chromium.org

[kylechar, 2016-11-30 22:48:03]

+vmpstr for cc/surfaces/*

[vmpstr, 2016-12-01 21:15:23]

cc/ lgtm

https://codereview.chromium.org/2520513002/diff/80001/services/ui/surfaces/di...
File services/ui/surfaces/display_compositor_unittest.cc (right):

https://codereview.chromium.org/2520513002/diff/80001/services/ui/surfaces/di...
services/ui/surfaces/display_compositor_unittest.cc:25: using cc::SurfaceId;
You have a mix of SurfaceId and cc::SurfaceId in this file, can you make it
consistent please?

https://codereview.chromium.org/2520513002/diff/80001/services/ui/ws/BUILD.gn
File services/ui/ws/BUILD.gn (right):

https://codereview.chromium.org/2520513002/diff/80001/services/ui/ws/BUILD.gn...
services/ui/ws/BUILD.gn:199: # TODO(fsamuel): Move this test with
DisplayCompositor.
Can you file a bug for this if one doesn't exist, and reference it here, please?

[kylechar, 2016-12-01 21:53:29]

Thanks!

https://codereview.chromium.org/2520513002/diff/80001/services/ui/surfaces/di...
File services/ui/surfaces/display_compositor_unittest.cc (right):

https://codereview.chromium.org/2520513002/diff/80001/services/ui/surfaces/di...
services/ui/surfaces/display_compositor_unittest.cc:25: using cc::SurfaceId;
On 2016/12/01 21:15:23, vmpstr wrote:
> You have a mix of SurfaceId and cc::SurfaceId in this file, can you make it
> consistent please?

Done.

https://codereview.chromium.org/2520513002/diff/80001/services/ui/ws/BUILD.gn
File services/ui/ws/BUILD.gn (right):

https://codereview.chromium.org/2520513002/diff/80001/services/ui/ws/BUILD.gn...
services/ui/ws/BUILD.gn:199: # TODO(fsamuel): Move this test with
DisplayCompositor.
On 2016/12/01 21:15:23, vmpstr wrote:
> Can you file a bug for this if one doesn't exist, and reference it here,
please?

Done.

[kylechar, 2016-12-01 21:54:00]

The CQ bit was checked by kylechar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-01 21:54:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-01 21:56:29]

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

[commit-bot: I haz the power, 2016-12-01 21:56:30]

Dry run: Try jobs failed on following builders:
  linux_precise_blink_rel on master.tryserver.blink (JOB_FAILED, no build URL)

[kylechar, 2016-12-02 02:12:03]

The CQ bit was checked by kylechar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-02 02:12:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-02 02:14:38]

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

[commit-bot: I haz the power, 2016-12-02 02:14:38]

Dry run: Try jobs failed on following builders:
  linux_precise_blink_rel on master.tryserver.blink (JOB_FAILED, no build URL)

[kylechar, 2016-12-02 02:46:16]

Description was changed from

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
==========

[kylechar, 2016-12-02 02:46:30]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-12-02 02:46:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, sky@chromium.org, vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2520513002/#ps120001
(title: "Add non-exported base.")

[commit-bot: I haz the power, 2016-12-02 02:47:18]

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

[commit-bot: I haz the power, 2016-12-02 03:13:29]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1480646790589160,
"parent_rev": "a8f5e79174382762bab45d6b55848b2e9cdd60f5", "commit_rev":
"948e3af01f2f6503c8d24bf743efadbe9f682b15"}

[commit-bot: I haz the power, 2016-12-02 03:14:01]

Description was changed from

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
==========

to

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
==========

[commit-bot: I haz the power, 2016-12-02 03:14:02]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-12-02 03:16:01]

Description was changed from

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227
==========

to

==========
Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG=659227

Committed: https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461
Cr-Commit-Position: refs/heads/master@{#435840}
==========

[commit-bot: I haz the power, 2016-12-02 03:16:02]

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