Add Blimp feedback data about visible BlimpContents.

Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
Committed: https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c
Cr-Commit-Position: refs/heads/master@{#421144}

[nyquist, 2016-09-23 23:00:42]

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

[commit-bot: I haz the power, 2016-09-23 23:01:08]

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-09-24 00:03:11]

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

[commit-bot: I haz the power, 2016-09-24 00:03:12]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[nyquist, 2016-09-24 02:23:55]

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

[commit-bot: I haz the power, 2016-09-24 02:24:10]

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-09-24 03:46:41]

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

[commit-bot: I haz the power, 2016-09-24 03:46:42]

Dry run: This issue passed the CQ dry run.

[nyquist, 2016-09-24 22:01:17]

Description was changed from

==========
Add blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

BUG=637988
==========

to

==========
Add blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
==========

[nyquist, 2016-09-24 22:01:33]

Description was changed from

==========
Add blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
==========

to

==========
Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
==========

[nyquist, 2016-09-24 22:01:50]

nyquist@chromium.org changed reviewers:
+ dtrainor@chromium.org

[nyquist, 2016-09-24 22:01:51]

dtrainor: PTAL

[nyquist, 2016-09-24 22:01:59]

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

[commit-bot: I haz the power, 2016-09-24 22:02:05]

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-09-24 22:55:28]

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

[commit-bot: I haz the power, 2016-09-24 22:55:29]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-09-27 03:49:26]

lgtm % nits

https://codereview.chromium.org/2371503002/diff/20001/blimp/client/core/conte...
File blimp/client/core/contents/blimp_contents_manager.cc (right):

https://codereview.chromium.org/2371503002/diff/20001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_manager.cc:104:
all_blimp_contents.push_back(
Check if the contents is null or not.  We do a posttask on removing it from the
map after the destroy call.  So the observer might have a null contents.

[nyquist, 2016-09-27 06:21:10]

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

[commit-bot: I haz the power, 2016-09-27 06:21:28]

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

[nyquist, 2016-09-27 06:21:29]

All done. Will send to CQ when the dependency patchset lands.

https://codereview.chromium.org/2371503002/diff/20001/blimp/client/core/conte...
File blimp/client/core/contents/blimp_contents_manager.cc (right):

https://codereview.chromium.org/2371503002/diff/20001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_manager.cc:104:
all_blimp_contents.push_back(
On 2016/09/27 03:49:26, David Trainor wrote:
> Check if the contents is null or not.  We do a posttask on removing it from
the
> map after the destroy call.  So the observer might have a null contents.

Wow! Good catch, thanks! Fixed now. I also of course added a test for this
specific case and verified it fails without the fix, and that the result was as
expected (1 bad entry in return result).

[commit-bot: I haz the power, 2016-09-27 07:16:33]

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

[commit-bot: I haz the power, 2016-09-27 07:16:35]

Dry run: This issue passed the CQ dry run.

[nyquist, 2016-09-27 07:54:04]

The CQ bit was checked by nyquist@chromium.org

[nyquist, 2016-09-27 07:54:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2371503002/#ps80001
(title: "Rebased for good measure")

[commit-bot: I haz the power, 2016-09-27 07:54:24]

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-09-27 07:59:30]

Description was changed from

==========
Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
==========

to

==========
Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
==========

[commit-bot: I haz the power, 2016-09-27 07:59:32]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-27 08:01:42]

Description was changed from

==========
Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988
==========

to

==========
Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG=637988

Committed: https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c
Cr-Commit-Position: refs/heads/master@{#421144}
==========

[commit-bot: I haz the power, 2016-09-27 08:01:43]

Patchset 5 (id:??) landed as
https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c
Cr-Commit-Position: refs/heads/master@{#421144}