[GRC] ResourceCoordinatorInterface support to remove children

[GRC] ResourceCoordinatorInterface support to remove children

This CL extends the ResourceCoordinatorInterface to remove children.
the ResourceCoordinatorInterface has been updated, as well as the
CoordinationUnit, it's mojo interface definition, and unittests. A
few header issues were added based on a linter pass.

R=oysteine@chromium.org,nasko@chromium.org
BUG=724306
Review-Url: https://codereview.chromium.org/2929883002
Cr-Commit-Position: refs/heads/master@{#478829}
Committed: https://chromium.googlesource.com/chromium/src/+/a7f5acc2721d0735725d522748eaa90bf64a22b6

[matthalp, 2017-06-08 17:33:48]

The CQ bit was checked by matthalp@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 17:34:09]

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, 2017-06-08 18:43:46]

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

[commit-bot: I haz the power, 2017-06-08 18:43:46]

Dry run: This issue passed the CQ dry run.

[nasko, 2017-06-09 22:19:14]

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File services/resource_coordinator/coordination_unit/coordination_unit_impl.cc
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/coordination_unit/coordination_unit_impl.cc:162:
if (child_iter != g_cu_map().end()) {
nit: Invert the check and return early. Leads to less code indentation.

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File
services/resource_coordinator/coordination_unit/coordination_unit_impl_unittest.cc
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/coordination_unit/coordination_unit_impl_unittest.cc:147:
CoordinationUnitID child_cu_id(CoordinationUnitType::kWebContents,
Can a WebContents really be a child of WebContents? Wouldn't it be more correct
test to use CoordinationUnitType::kFrame?

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File services/resource_coordinator/public/cpp/resource_coordinator_interface.cc
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/public/cpp/resource_coordinator_interface.cc:8:
#include <utility>
Are these includes really needed?

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File services/resource_coordinator/public/interfaces/coordination_unit.mojom
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/public/interfaces/coordination_unit.mojom:60:
RemoveChild(CoordinationUnitID child_id);
Does the above comment apply to RemoveChild? Reading it, likely not. If it
doesn't, please leave an empty line to clarify that the comment is only for
AddChild. Ideally add a comment that is applicable to RemoveChild.

[matthalp, 2017-06-09 22:31:59]

CL has been updated based on reviewer feedback.

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File services/resource_coordinator/coordination_unit/coordination_unit_impl.cc
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/coordination_unit/coordination_unit_impl.cc:162:
if (child_iter != g_cu_map().end()) {
On 2017/06/09 at 22:19:13, nasko (slow) wrote:
> nit: Invert the check and return early. Leads to less code indentation.

Good suggestion -- Done.

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File
services/resource_coordinator/coordination_unit/coordination_unit_impl_unittest.cc
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/coordination_unit/coordination_unit_impl_unittest.cc:147:
CoordinationUnitID child_cu_id(CoordinationUnitType::kWebContents,
On 2017/06/09 at 22:19:13, nasko (slow) wrote:
> Can a WebContents really be a child of WebContents? Wouldn't it be more
correct test to use CoordinationUnitType::kFrame?

Agreed -- Done.

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File services/resource_coordinator/public/cpp/resource_coordinator_interface.cc
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/public/cpp/resource_coordinator_interface.cc:8:
#include <utility>
On 2017/06/09 at 22:19:13, nasko (slow) wrote:
> Are these includes really needed?

git cl lint says that "#include <utility>" is needed for the std::move (now on
line 68), but not <string>. <string> has been removed.

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
File services/resource_coordinator/public/interfaces/coordination_unit.mojom
(right):

https://codereview.chromium.org/2929883002/diff/1/services/resource_coordinat...
services/resource_coordinator/public/interfaces/coordination_unit.mojom:60:
RemoveChild(CoordinationUnitID child_id);
On 2017/06/09 at 22:19:13, nasko (slow) wrote:
> Does the above comment apply to RemoveChild? Reading it, likely not. If it
doesn't, please leave an empty line to clarify that the comment is only for
AddChild. Ideally add a comment that is applicable to RemoveChild.

It does not. Comment added.

[matthalp, 2017-06-09 22:32:03]

The CQ bit was checked by matthalp@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 22:32:22]

Dry run: CQ is trying da patch.

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

[nasko, 2017-06-09 22:40:57]

LGTM

[commit-bot: I haz the power, 2017-06-09 23:49:41]

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

[commit-bot: I haz the power, 2017-06-09 23:49:42]

Dry run: This issue passed the CQ dry run.

[matthalp, 2017-06-10 01:41:47]

The CQ bit was checked by matthalp@google.com

[commit-bot: I haz the power, 2017-06-10 01:42:03]

CQ is trying da patch.

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

[oystein (OOO til 10th of July), 2017-06-12 22:44:36]

lgtm

[matthalp, 2017-06-12 22:47:08]

The CQ bit was checked by matthalp@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 22:47:34]

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, 2017-06-13 00:00:22]

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

[commit-bot: I haz the power, 2017-06-13 00:00:23]

Dry run: This issue passed the CQ dry run.

[matthalp, 2017-06-13 00:01:54]

The CQ bit was checked by matthalp@google.com

[commit-bot: I haz the power, 2017-06-13 00:02:31]

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, 2017-06-13 00:07:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497312114253730,
"parent_rev": "64e6b930441e38e856ae424cbc11e4c18b637810", "commit_rev":
"a7f5acc2721d0735725d522748eaa90bf64a22b6"}

[commit-bot: I haz the power, 2017-06-13 00:07:29]

Description was changed from

==========
[GRC] ResourceCoordinatorInterface support to remove children

This CL extends the ResourceCoordinatorInterface to remove children.
the ResourceCoordinatorInterface has been updated, as well as the
CoordinationUnit, it's mojo interface definition, and unittests. A
few header issues were added based on a linter pass.

R=oysteine@chromium.org,nasko@chromium.org
BUG=724306
==========

to

==========
[GRC] ResourceCoordinatorInterface support to remove children

This CL extends the ResourceCoordinatorInterface to remove children.
the ResourceCoordinatorInterface has been updated, as well as the
CoordinationUnit, it's mojo interface definition, and unittests. A
few header issues were added based on a linter pass.

R=oysteine@chromium.org,nasko@chromium.org
BUG=724306

Review-Url: https://codereview.chromium.org/2929883002
Cr-Commit-Position: refs/heads/master@{#478829}
Committed:
https://chromium.googlesource.com/chromium/src/+/a7f5acc2721d0735725d522748ea...
==========

[commit-bot: I haz the power, 2017-06-13 00:07:31]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a7f5acc2721d0735725d522748ea...