chromeos/dbus: Add verbose log to media-related clients and service provider.

chromeos/dbus: Add verbose log to media-related clients and service provider.

This CL adds some VLOG to BluetoothMediaClient,
BluetoothMediaTransportClient, BluetoothMediaEndpointServiceProvider
and BluetoothAudioSinkChromeOS.

BUG=429016
TEST=None

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

[Miao, 2015-02-28 00:41:29]

mcchou@chromium.org changed reviewers:
+ armansito@chromium.org, benchan@chromium.org

[Miao, 2015-02-28 00:41:29]

This CL is ready for review.

[armansito, 2015-02-28 00:50:29]

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
File chromeos/dbus/bluetooth_media_client.cc (right):

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_client.cc:106: VLOG(1) << "RegisterEndpoint:
endpoint:" << endpoint_path.value();
nit: Add space after "endpoint: ", otherwise the object path will print
immediately after the colon. Also two colons make look a bit weird, I'd do:

"RegisterEndpoint - endpoint: " << ...

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_client.cc:160: VLOG(1) << "UnregisterEndpoint:
endpoint:" << endpoint_path.value();
nit: same as above.

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
File chromeos/dbus/bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.cc:132: VLOG(1) << "Acquire:
transport:" << object_path.value();
nit: Same comments as in bluetooth_media_client.cc, here and below.

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
File device/bluetooth/bluetooth_audio_sink_chromeos.cc (right):

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:32: "not unregistered"};
Define helper functions that do the string conversion instead.

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:45: VLOG(1) << "Bluetooth
audio sink: Error code: " << kErrorCode[error_code]
Should this be "UnregisterErrorCallback - Error code: "?

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:185: VLOG(1) << "Bluetooth
audio sink: media transport removed: "
"Bluetooth audio sink - media transport removed: "

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:393: VLOG(1) << "Bluetooth
audio sink: error " << error_name << ": "
"UnregisterError - error_name: " << error_name << ", message: " << error_message

[Ben Chan, 2015-02-28 01:09:22]

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
File device/bluetooth/bluetooth_audio_sink_chromeos.cc (right):

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:32: "not unregistered"};
On 2015/02/28 00:50:28, armansito wrote:
> Define helper functions that do the string conversion instead.

Don't define global constants of non-POD types
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Static_and_Gl...).
Use helper functions as Arman suggested. That way, you can handle bogus enum
value that may fail out of the range of the array (or otherwise, it may result
in a crash)

[Miao, 2015-02-28 02:19:33]

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
File chromeos/dbus/bluetooth_media_client.cc (right):

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_client.cc:106: VLOG(1) << "RegisterEndpoint:
endpoint:" << endpoint_path.value();
On 2015/02/28 00:50:28, armansito wrote:
> nit: Add space after "endpoint: ", otherwise the object path will print
> immediately after the colon. Also two colons make look a bit weird, I'd do:
> 
> "RegisterEndpoint - endpoint: " << ...

Done.

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_client.cc:160: VLOG(1) << "UnregisterEndpoint:
endpoint:" << endpoint_path.value();
On 2015/02/28 00:50:28, armansito wrote:
> nit: same as above.

Done.

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
File chromeos/dbus/bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.cc:132: VLOG(1) << "Acquire:
transport:" << object_path.value();
On 2015/02/28 00:50:28, armansito wrote:
> nit: Same comments as in bluetooth_media_client.cc, here and below.

Done.

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.cc:155: VLOG(1) << "TryAcquire:
transport:" << object_path.value();
Modified.

https://codereview.chromium.org/963983002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.cc:179: VLOG(1) << "Release:
transport:" << object_path.value();
Modified.

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
File device/bluetooth/bluetooth_audio_sink_chromeos.cc (right):

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:32: "not unregistered"};
On 2015/02/28 00:50:28, armansito wrote:
> Define helper functions that do the string conversion instead.

Done.

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:45: VLOG(1) << "Bluetooth
audio sink: Error code: " << kErrorCode[error_code]
On 2015/02/28 00:50:28, armansito wrote:
> Should this be "UnregisterErrorCallback - Error code: "?

Done.

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:185: VLOG(1) << "Bluetooth
audio sink: media transport removed: "
On 2015/02/28 00:50:28, armansito wrote:
> "Bluetooth audio sink - media transport removed: "

Done.

https://codereview.chromium.org/963983002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:393: VLOG(1) << "Bluetooth
audio sink: error " << error_name << ": "
On 2015/02/28 00:50:28, armansito wrote:
> "UnregisterError - error_name: " << error_name << ", message: " <<
error_message

Done.

[Ben Chan, 2015-02-28 02:25:36]

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_audio_sink_chromeos.cc (right):

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:35: std::string verbose_state;
switch (state) {
  case BluetoothAudioSink::STATE_INVALID:
    return "invalid";

  case ...:
    return ...;

  default:
    NOTREACHED();  // something wrong!
    return std::string(state);
}

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:53: std::string verbose_error;
ditto

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:133: VLOG(1) << "Bluetooth
audio sink: Unregister called";
seems inconsistent with the messages above and below

[armansito, 2015-02-28 02:28:25]

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_audio_sink_chromeos.cc (right):

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:173: VLOG(1) << "Bluetooth
audio sink - bluetooth adapter present changed: "
nit: always capitalize Bluetooth. Here and below.

[Miao, 2015-02-28 03:36:20]

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_audio_sink_chromeos.cc (right):

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:35: std::string verbose_state;
On 2015/02/28 02:25:35, Ben Chan wrote:
> switch (state) {
>   case BluetoothAudioSink::STATE_INVALID:
>     return "invalid";
> 
>   case ...:
>     return ...;
> 
>   default:
>     NOTREACHED();  // something wrong!
>     return std::string(state);
> }

Done.

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:53: std::string verbose_error;
On 2015/02/28 02:25:36, Ben Chan wrote:
> ditto

Done.

https://codereview.chromium.org/963983002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:133: VLOG(1) << "Bluetooth
audio sink: Unregister called";
On 2015/02/28 02:25:35, Ben Chan wrote:
> seems inconsistent with the messages above and below

Done.

[armansito, 2015-02-28 03:37:23]

The CQ bit was checked by armansito@chromium.org

[armansito, 2015-02-28 03:37:24]

lgtm

[commit-bot: I haz the power, 2015-02-28 03:37:52]

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

[commit-bot: I haz the power, 2015-02-28 04:48:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-28 04:49:30]

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