Read container_instance_id.

Read container_instance_id.

In case of race, ArcSession may stop by an event for unrelated
container instance.
To avoid such a situation, this CL reads the container instance id,
which is newly provided by session_manager, and let ArcSession check it.

BUG=665316
TEST=Ran on device. Ran trybots.

Review-Url: https://codereview.chromium.org/2887363003
Cr-Commit-Position: refs/heads/master@{#473527}
Committed: https://chromium.googlesource.com/chromium/src/+/02a11f55e9c6ec533b9c811f6f7cc2c255ef4694

[hidehiko, 2017-05-19 09:48:02]

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

[commit-bot: I haz the power, 2017-05-19 09:48:37]

Dry run: CQ is trying da patch.

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

[hidehiko, 2017-05-19 10:09:38]

hidehiko@chromium.org changed reviewers:
+ hashimoto@chromium.org, lhchavez@chromium.org

[hidehiko, 2017-05-19 10:09:39]

PTAL.

[commit-bot: I haz the power, 2017-05-19 10:57:34]

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

[commit-bot: I haz the power, 2017-05-19 10:57:35]

Dry run: This issue passed the CQ dry run.

[hashimoto, 2017-05-19 11:06:59]

chromeos/dbus lgtm

https://codereview.chromium.org/2887363003/diff/20001/chromeos/dbus/session_m...
File chromeos/dbus/session_manager_client.h (right):

https://codereview.chromium.org/2887363003/diff/20001/chromeos/dbus/session_m...
chromeos/dbus/session_manager_client.h:301:
base::Callback<void(StartArcInstanceResult, const std::string&)>;
Could you add names of the arguments, like GetArcStartTimeCallback above?

[hidehiko, 2017-05-19 12:35:50]

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

[commit-bot: I haz the power, 2017-05-19 12:36:07]

Dry run: CQ is trying da patch.

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

[hidehiko, 2017-05-19 12:36:49]

Thank you for review. Luis, PTAL.

https://codereview.chromium.org/2887363003/diff/20001/chromeos/dbus/session_m...
File chromeos/dbus/session_manager_client.h (right):

https://codereview.chromium.org/2887363003/diff/20001/chromeos/dbus/session_m...
chromeos/dbus/session_manager_client.h:301:
base::Callback<void(StartArcInstanceResult, const std::string&)>;
On 2017/05/19 11:06:58, hashimoto wrote:
> Could you add names of the arguments, like GetArcStartTimeCallback above?

Sure. Done.

[commit-bot: I haz the power, 2017-05-19 13:36:58]

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

[commit-bot: I haz the power, 2017-05-19 13:36:59]

Dry run: This issue passed the CQ dry run.

[Luis Héctor Chávez, 2017-05-19 15:23:03]

just nits, otherwise lgtm.

https://codereview.chromium.org/2887363003/diff/60001/chromeos/dbus/session_m...
File chromeos/dbus/session_manager_client.h (right):

https://codereview.chromium.org/2887363003/diff/60001/chromeos/dbus/session_m...
chromeos/dbus/session_manager_client.h:76: // See details for
StartArcInstance().
maybe it's better to refer to the details of StartArcInstanceCallback, since
it's the one that has the details.

https://codereview.chromium.org/2887363003/diff/60001/chromeos/dbus/session_m...
chromeos/dbus/session_manager_client.h:297: // In case of success,
container_instance_id will be passed as its second
nit: |container_instance_id|, for consistency.

https://codereview.chromium.org/2887363003/diff/60001/components/arc/arc_sess...
File components/arc/arc_session.cc (right):

https://codereview.chromium.org/2887363003/diff/60001/components/arc/arc_sess...
components/arc/arc_session.cc:259: // Should be available only after
OnInstanceStarted.
nit: OnInstanceStarted()

https://codereview.chromium.org/2887363003/diff/60001/components/arc/arc_sess...
components/arc/arc_session.cc:583: }
nit: do you want to reset the container id here? Just in case we get a duplicate
notification.

[hidehiko, 2017-05-22 06:57:19]

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

[commit-bot: I haz the power, 2017-05-22 06:57:33]

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-05-22 07:33:13]

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

[commit-bot: I haz the power, 2017-05-22 07:33:14]

Dry run: This issue passed the CQ dry run.

[hidehiko, 2017-05-22 08:38:06]

Thank you for review! Submitting.

https://codereview.chromium.org/2887363003/diff/60001/chromeos/dbus/session_m...
File chromeos/dbus/session_manager_client.h (right):

https://codereview.chromium.org/2887363003/diff/60001/chromeos/dbus/session_m...
chromeos/dbus/session_manager_client.h:76: // See details for
StartArcInstance().
On 2017/05/19 15:23:03, Luis Héctor Chávez wrote:
> maybe it's better to refer to the details of StartArcInstanceCallback, since
> it's the one that has the details.

Done.

https://codereview.chromium.org/2887363003/diff/60001/chromeos/dbus/session_m...
chromeos/dbus/session_manager_client.h:297: // In case of success,
container_instance_id will be passed as its second
On 2017/05/19 15:23:03, Luis Héctor Chávez wrote:
> nit: |container_instance_id|, for consistency.

Done.

https://codereview.chromium.org/2887363003/diff/60001/components/arc/arc_sess...
File components/arc/arc_session.cc (right):

https://codereview.chromium.org/2887363003/diff/60001/components/arc/arc_sess...
components/arc/arc_session.cc:259: // Should be available only after
OnInstanceStarted.
On 2017/05/19 15:23:03, Luis Héctor Chávez wrote:
> nit: OnInstanceStarted()

Done.

https://codereview.chromium.org/2887363003/diff/60001/components/arc/arc_sess...
components/arc/arc_session.cc:583: }
On 2017/05/19 15:23:03, Luis Héctor Chávez wrote:
> nit: do you want to reset the container id here? Just in case we get a
duplicate
> notification.

Done.

[hidehiko, 2017-05-22 08:38:34]

The CQ bit was checked by hidehiko@chromium.org

[hidehiko, 2017-05-22 08:38:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
hashimoto@chromium.org, lhchavez@chromium.org
Link to the patchset: https://codereview.chromium.org/2887363003/#ps100001
(title: "Address comments.")

[commit-bot: I haz the power, 2017-05-22 08:38:49]

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-05-22 08:43:10]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1495442314484910,
"parent_rev": "d322245ad1ab3a7deb21b4c1598f6de60d8a8778", "commit_rev":
"02a11f55e9c6ec533b9c811f6f7cc2c255ef4694"}

[commit-bot: I haz the power, 2017-05-22 08:43:25]

Description was changed from

==========
Read container_instance_id.

In case of race, ArcSession may stop by an event for unrelated
container instance.
To avoid such a situation, this CL reads the container instance id,
which is newly provided by session_manager, and let ArcSession check it.

BUG=665316
TEST=Ran on device. Ran trybots.
==========

to

==========
Read container_instance_id.

In case of race, ArcSession may stop by an event for unrelated
container instance.
To avoid such a situation, this CL reads the container instance id,
which is newly provided by session_manager, and let ArcSession check it.

BUG=665316
TEST=Ran on device. Ran trybots.

Review-Url: https://codereview.chromium.org/2887363003
Cr-Commit-Position: refs/heads/master@{#473527}
Committed:
https://chromium.googlesource.com/chromium/src/+/02a11f55e9c6ec533b9c811f6f7c...
==========

[commit-bot: I haz the power, 2017-05-22 08:43:28]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/02a11f55e9c6ec533b9c811f6f7c...