arc-bridge: Expose the Mojo version number of the interfaces

arc-bridge: Expose the Mojo version number of the interfaces

Every time a Mojo interface is changed and a new MinVersion tag is
added, the version of the interface is bumped. This change exposes
that version number so that callers on the other side of the bridge
can know if it is safe to call that method or if it will cause the
message pipe to be closed.

BUG=569446
TEST=components_unittests, unit_tests

Committed: https://crrev.com/c85072324da12165103eea15ed508422702c3b8c
Cr-Commit-Position: refs/heads/master@{#367583}

[Luis Héctor Chávez, 2015-12-29 00:01:34]

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

[Luis Héctor Chávez, 2015-12-29 00:01:34]

PTAL

[Luis Héctor Chávez, 2015-12-29 00:03:09]

Description was changed from

==========
arc-bridge: Expose the Mojo version number of the interfaces

There have been cases where the instance crashes. This change makes it
so that if there is a crash and the connection gets lost, the instance
will be cleanly shut down and restarted afterwards.

BUG=569446
TEST=components_unittests, unit_tests
==========

to

==========
arc-bridge: Expose the Mojo version number of the interfaces

Every time a Mojo interface is changed and a new MinVersion tag is
added, the version of the interface is bumped. This change exposes
that version number so that callers on the other side of the bridge
can know if it is safe to call that method or if it will cause the
message pipe to be closed.

BUG=569446
TEST=components_unittests, unit_tests
==========

[hidehiko, 2015-12-29 13:49:21]

https://codereview.chromium.org/1551763002/diff/1/components/arc/arc_bridge_s...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1551763002/diff/1/components/arc/arc_bridge_s...
components/arc/arc_bridge_service.h:216: AppInstancePtr temporary_app_ptr_;
Instead of making temporary_XXX_ptr_ fields, you can pass the ptr to the bound
methods' arguments, I believe.

[Luis Héctor Chávez, 2015-12-29 16:51:09]

https://codereview.chromium.org/1551763002/diff/1/components/arc/arc_bridge_s...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1551763002/diff/1/components/arc/arc_bridge_s...
components/arc/arc_bridge_service.h:216: AppInstancePtr temporary_app_ptr_;
On 2015/12/29 13:49:20, hidehiko wrote:
> Instead of making temporary_XXX_ptr_ fields, you can pass the ptr to the bound
> methods' arguments, I believe.

That won't work. It would need something like

app_ptr.QueryVersion(base::Bind(...,
                                base::Passed(&app_ptr)));

By the time QueryVersion() is executed, |app_ptr| points to invalid data (or
rather, an empty InterfacePtr<...>), so it crashes on a DCHECK.

Alternatively, passing the InterfacePtr as a pointer, which seems even worse.

[hidehiko, 2016-01-05 08:05:52]

lgtm

https://codereview.chromium.org/1551763002/diff/1/components/arc/arc_bridge_s...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1551763002/diff/1/components/arc/arc_bridge_s...
components/arc/arc_bridge_service.h:216: AppInstancePtr temporary_app_ptr_;
On 2015/12/29 16:51:09, Luis Héctor Chávez wrote:
> On 2015/12/29 13:49:20, hidehiko wrote:
> > Instead of making temporary_XXX_ptr_ fields, you can pass the ptr to the
bound
> > methods' arguments, I believe.
> 
> That won't work. It would need something like
> 
> app_ptr.QueryVersion(base::Bind(...,
>                                 base::Passed(&app_ptr)));
> 
> By the time QueryVersion() is executed, |app_ptr| points to invalid data (or
> rather, an empty InterfacePtr<...>), so it crashes on a DCHECK.
> 
> Alternatively, passing the InterfacePtr as a pointer, which seems even worse.

Ah, you're right. QueryVersion is a member of InterfacePtr... Sorry for wrong
suggestion.

[Luis Héctor Chávez, 2016-01-05 17:51:34]

The CQ bit was checked by lhchavez@chromium.org

[commit-bot: I haz the power, 2016-01-05 17:52:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1551763002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1551763002/1

[commit-bot: I haz the power, 2016-01-05 18:22:34]

Description was changed from

==========
arc-bridge: Expose the Mojo version number of the interfaces

Every time a Mojo interface is changed and a new MinVersion tag is
added, the version of the interface is bumped. This change exposes
that version number so that callers on the other side of the bridge
can know if it is safe to call that method or if it will cause the
message pipe to be closed.

BUG=569446
TEST=components_unittests, unit_tests
==========

to

==========
arc-bridge: Expose the Mojo version number of the interfaces

Every time a Mojo interface is changed and a new MinVersion tag is
added, the version of the interface is bumped. This change exposes
that version number so that callers on the other side of the bridge
can know if it is safe to call that method or if it will cause the
message pipe to be closed.

BUG=569446
TEST=components_unittests, unit_tests
==========

[commit-bot: I haz the power, 2016-01-05 18:22:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-05 18:23:38]

Description was changed from

==========
arc-bridge: Expose the Mojo version number of the interfaces

Every time a Mojo interface is changed and a new MinVersion tag is
added, the version of the interface is bumped. This change exposes
that version number so that callers on the other side of the bridge
can know if it is safe to call that method or if it will cause the
message pipe to be closed.

BUG=569446
TEST=components_unittests, unit_tests
==========

to

==========
arc-bridge: Expose the Mojo version number of the interfaces

Every time a Mojo interface is changed and a new MinVersion tag is
added, the version of the interface is bumped. This change exposes
that version number so that callers on the other side of the bridge
can know if it is safe to call that method or if it will cause the
message pipe to be closed.

BUG=569446
TEST=components_unittests, unit_tests

Committed: https://crrev.com/c85072324da12165103eea15ed508422702c3b8c
Cr-Commit-Position: refs/heads/master@{#367583}
==========

[commit-bot: I haz the power, 2016-01-05 18:23:42]

Patchset 1 (id:??) landed as
https://crrev.com/c85072324da12165103eea15ed508422702c3b8c
Cr-Commit-Position: refs/heads/master@{#367583}