Remove the ARC Bridge Service

Remove the ARC Bridge Service

No longer needed since ARC-related messages are being
migrated to session_manager.

BUG=b:24339743
Committed: https://crrev.com/c78e795e89b7f15eebcadde00e91688815b03124
Cr-Commit-Position: refs/heads/master@{#362778}

[Luis Héctor Chávez, 2015-10-23 02:39:25]

lhchavez@chromium.org changed reviewers:
+ denniskempin@chromium.org, stevenjb@chromium.org

[Luis Héctor Chávez, 2015-10-23 02:39:25]

PTAL

This is required by

https://codereview.chromium.org/1412863004/

[denniskempin (chromium), 2015-10-23 12:22:10]

Can you update the commit message to reflect that this is not a pure renaming?

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
File chromeos/dbus/arc_instance_client.cc (right):

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
chromeos/dbus/arc_instance_client.cc:32: void OnBoolDBusMethod(const
BoolDBusMethodCallback& callback,
dbus already has a way of passing back errors, isn't the additional boolean
unnecessary then? The method could just return DBUS_METHOD_CALL_FAILURE.
See:
https://chrome-internal-review.googlesource.com/#/c/234260/10/chromeos-base/a...

[stevenjb, 2015-10-23 16:34:45]

Is there a document for ArcInstanceClient somewhere?

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
File chromeos/dbus/arc_instance_client.cc (right):

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
chromeos/dbus/arc_instance_client.cc:65: base::Bind(&OnBoolDBusMethod,
callback));
To elaborate the above comment, typically we would use
proxy_->CallMethodWithErrorCallback() which passes in an error callback handler
which receives a dbus::ErrorResponse containing the error code.

Note: it is fine having this method just taking a single callback that receives
true or false (although a return code might be more useful in the future), but
we should use DBus error codes between processes.

[Luis Héctor Chávez, 2015-10-23 17:33:14]

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
File chromeos/dbus/arc_instance_client.cc (right):

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
chromeos/dbus/arc_instance_client.cc:32: void OnBoolDBusMethod(const
BoolDBusMethodCallback& callback,
On 2015/10/23 12:22:10, denniskempin wrote:
> dbus already has a way of passing back errors, isn't the additional boolean
> unnecessary then? The method could just return DBUS_METHOD_CALL_FAILURE.
> See:
>
https://chrome-internal-review.googlesource.com/#/c/234260/10/chromeos-base/a...

What I originally wanted was to have an extra flag to indicate availablity (so
the call could be successful, but the instance would not be available), but now
that I think about it, it's simpler to just go with the failure.

https://codereview.chromium.org/1421023002/diff/1/chromeos/dbus/arc_instance_...
chromeos/dbus/arc_instance_client.cc:65: base::Bind(&OnBoolDBusMethod,
callback));
On 2015/10/23 16:34:45, stevenjb wrote:
> To elaborate the above comment, typically we would use
> proxy_->CallMethodWithErrorCallback() which passes in an error callback
handler
> which receives a dbus::ErrorResponse containing the error code.
> 
> Note: it is fine having this method just taking a single callback that
receives
> true or false (although a return code might be more useful in the future), but
> we should use DBus error codes between processes.

I went with Dennis' suggestion and just return the DBus status to the callback.

[Luis Héctor Chávez, 2015-10-23 17:36:21]

Description was changed from

==========
Rename the ARC Bridge Service to the ARC Instance Service

BUG=b:24339743
==========

to

==========
Replace the ARC Bridge Service with the ARC Instance Service

Going forward, the ARC Bridge Service will be part of
Chromium, so a DBus client for it will be unnecessary.
The ARC Bridge's responsibilities will be now split, and a
Chrome OS service will be created to replace it: the ARC
Instance Service.

BUG=b:24339743
==========

[satorux1, 2015-10-27 07:21:28]

satorux@chromium.org changed reviewers:
+ satorux@chromium.org

[satorux1, 2015-10-27 07:21:29]

https://codereview.chromium.org/1421023002/diff/20001/chromeos/dbus/arc_bridg...
File chromeos/dbus/arc_bridge_client.h (left):

https://codereview.chromium.org/1421023002/diff/20001/chromeos/dbus/arc_bridg...
chromeos/dbus/arc_bridge_client.h:19: // Client for the Arc Bridge Service
could you add some description about the ARC bridge service? Might be nice to
document relationship with ARC Instance service.

https://codereview.chromium.org/1421023002/diff/20001/chromeos/dbus/arc_insta...
File chromeos/dbus/arc_instance_client.h (right):

https://codereview.chromium.org/1421023002/diff/20001/chromeos/dbus/arc_insta...
chromeos/dbus/arc_instance_client.h:19: // Client for the Arc Instance Service
could you add some description about the ARC instance service? Might be nice to
document relationship with ARC bridge service.

[Luis Héctor Chávez, 2015-10-27 15:57:33]

https://codereview.chromium.org/1421023002/diff/20001/chromeos/dbus/arc_insta...
File chromeos/dbus/arc_instance_client.h (right):

https://codereview.chromium.org/1421023002/diff/20001/chromeos/dbus/arc_insta...
chromeos/dbus/arc_instance_client.h:19: // Client for the Arc Instance Service
On 2015/10/27 07:21:28, satorux1 wrote:
> could you add some description about the ARC instance service? Might be nice
to
> document relationship with ARC bridge service.

Done.

[Luis Héctor Chávez, 2015-11-03 22:49:57]

Description was changed from

==========
Replace the ARC Bridge Service with the ARC Instance Service

Going forward, the ARC Bridge Service will be part of
Chromium, so a DBus client for it will be unnecessary.
The ARC Bridge's responsibilities will be now split, and a
Chrome OS service will be created to replace it: the ARC
Instance Service.

BUG=b:24339743
==========

to

==========
Remove the ARC Bridge Service

No longer needed since ARC-related messages are being
migrated to session_manager.

BUG=b:24339743
==========

[Luis Héctor Chávez, 2015-11-03 22:50:31]

Code was moved around and now will live in a different service. PTAL

[stevenjb, 2015-11-06 21:39:58]

lgtm

[Luis Héctor Chávez, 2015-12-02 19:21:11]

The CQ bit was checked by lhchavez@chromium.org

[commit-bot: I haz the power, 2015-12-02 19:21:29]

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

[commit-bot: I haz the power, 2015-12-02 20:06:29]

Description was changed from

==========
Remove the ARC Bridge Service

No longer needed since ARC-related messages are being
migrated to session_manager.

BUG=b:24339743
==========

to

==========
Remove the ARC Bridge Service

No longer needed since ARC-related messages are being
migrated to session_manager.

BUG=b:24339743
==========

[commit-bot: I haz the power, 2015-12-02 20:06:30]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-02 20:07:17]

Description was changed from

==========
Remove the ARC Bridge Service

No longer needed since ARC-related messages are being
migrated to session_manager.

BUG=b:24339743
==========

to

==========
Remove the ARC Bridge Service

No longer needed since ARC-related messages are being
migrated to session_manager.

BUG=b:24339743

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

[commit-bot: I haz the power, 2015-12-02 20:07:19]

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