device/bluetooth: Add I/O watcher for audio data retrieval triggered by state change.

device/bluetooth: Add I/O watcher for audio data retrieval triggered by state change.

This CL implements the state change handling and data retrieval
during BT audio streaming. BluetoothAudioSinkDataAvailable() is
added in order to deliver audio data to the user application. File
descriptor watcher operations are bound to state change events from
BluetoothMediaTransportClient. Note that the ownership of the output
file descriptor is transferred from BlueZ to
BluetoothAudioSinkChromeOS. This CL also implements Acquire/
TryAcquire of FakeBluetoothMediaTransportClient. Corresponding tests
are added to address new changes and TODOs.

BUG=441581
TEST=device_unittests --gtest_filter=*AudioSink*

Committed: https://crrev.com/512445e1c5fd77acfbce6d5540b8e938d1702f29
Cr-Commit-Position: refs/heads/master@{#320571}

[Miao, 2015-03-11 07:16:10]

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

[Miao, 2015-03-11 07:16:32]

This CL is ready for review.

[armansito, 2015-03-12 03:42:55]

Mostly nits, looks good overall!

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

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.cc:253:
callback.Run(fd.TakeValue(), read_mtu, write_mtu);
You don't want to do this here, I would leave it up to the caller of Acquire to
take ownership of the file descriptor. The problem is, if you call TakeValue
here and the caller of Acquire doesn't do anything with it, the file descriptor
would never be closed and hence get leaked. Instead, I would just return a
pointer to the dbus::FileDescriptor class and let the caller explicitly take
ownership of the fd.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/bluetooth_medi...
File chromeos/dbus/bluetooth_media_transport_client.h (left):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.h:101: typedef
base::Callback<void(const dbus::FileDescriptor& fd,
I would pass a "dbus::FileDescriptor*" here instead of const int fd. Please see
my comment in bluetooth_media_transport_client.cc.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
File chromeos/dbus/fake_bluetooth_media_transport_client.cc (left):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:178: return;
nit: new line after 'return'

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
File chromeos/dbus/fake_bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:134: ObjectPath
endpoint_path = GetEndpointPath(object_path);
const ObjectPath&

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:156: }
Maybe add a DCHECK here that both fds[0] and fd[1] are positive.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:169: ObjectPath
endpoint_path = GetEndpointPath(object_path);
const ObjectPath&

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:170: Transport* transport
= GetTransport(endpoint_path);
Maybe you want a helper like "GetTransportByPath" which returns a Transport
object using a transport path.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:197:
SetState(endpoint_path, "active");
A lot of the code between TryAcquire and Acquire is the same. Perhaps you can
unify them in a helper?

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:245: ObjectPath
transport_path = transport->path;
const ObjectPath&

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:289: VLOG(1) <<
"WriteData";
Maybe log the number of bytes here? Might be useful for debugging.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:294: VLOG(1) <<
"WriteData - write operation rejected on endpoint: "
You should probably log here that this was rejected because the state isn't
"active". Will save you time later when reading the logs :)

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:300: VLOG(1) <<
"WriteData - invalid file descriptors";
nit: say "invalid input file descriptor"?

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:305:
write(transport->input_fd->GetPlatformFile(), bytes.data(), bytes.size());
I wonder if you should prevent sending more bytes than the current read MTU? At
least this would make the behavior consistent with BlueZ (but I guess you
wouldn't be able to test invalid behavior in that case, so maybe what you have
already is better).

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:317: auto it =
endpoint_to_transport_map_.find(endpoint_path);
const auto& it

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:330: auto it =
transport_to_endpoint_map_.find(transport_path);
const auto& it

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
File chromeos/dbus/fake_bluetooth_media_transport_client.h (right):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.h:80: // Writes bytes to the
input file descriptor, |input_fd|, accosiated with a
nit: s/accosiated/associated. Try pronouncing your version, it sounds funny.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.h:81: // Transport object
which is bound to |endpoint_path|.
nit: s/Transport/transport/. Lowercase, because you probably don't want to be
referring to "struct Transport" which is private to this class.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.h:107: // will be
initialized when Acquire/TryAcquire is called.
I would also say here that this is the internal end of the socketpair that gets
created for simulation purposes.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
File device/bluetooth/bluetooth_audio_sink.h (right):

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink.h:76: // Callled when there is audio data
available. |audio_sink| indicates the
nit: Called

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink.h:78: // is the number of the bytes in
|data|.
nit: s/number of the bytes/number of bytes/

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink.h:78: // is the number of the bytes in
|data|.
I would explain here that this method provides the raw audio data which hasn't
been processed or assembled in any way and that the RTP units and SBC decoding
needs to happen here.

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

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:351: read_failed_logged_ =
false;
I would rename this variable to something like |read_has_failed_|, so that it's
a bit more generic and you could reuse it for things other than logging in the
future.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:384: file_.reset();
Add a comment here saying that this will close the fd.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:472: VLOG(1) << "Unregisterd -
endpoint: " << endpoint_path_.value();
nit: s/Unregisterd/Unregistered/

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:498: // Avoids unnecessary
memory reallocation if read MTU doesn't change.
Nice!

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

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.h:87: void
OnFileCanWriteWithoutBlocking(int fd) override;
These can be private.

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

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos_unittest.cc:837:
EXPECT_EQ(observer_->total_read_, data_one.size());
It would be cool if you could also check that the data you read matches the data
that you sent over the wire. You can simply copy the data in the observer method
to a local variable in the test harness, just like you do with |total_read_|.

[Miao, 2015-03-12 22:33:32]

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

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.cc:253:
callback.Run(fd.TakeValue(), read_mtu, write_mtu);
On 2015/03/12 03:42:54, armansito wrote:
> You don't want to do this here, I would leave it up to the caller of Acquire
to
> take ownership of the file descriptor. The problem is, if you call TakeValue
> here and the caller of Acquire doesn't do anything with it, the file
descriptor
> would never be closed and hence get leaked. Instead, I would just return a
> pointer to the dbus::FileDescriptor class and let the caller explicitly take
> ownership of the fd.

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/bluetooth_medi...
File chromeos/dbus/bluetooth_media_transport_client.h (left):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/bluetooth_medi...
chromeos/dbus/bluetooth_media_transport_client.h:101: typedef
base::Callback<void(const dbus::FileDescriptor& fd,
On 2015/03/12 03:42:54, armansito wrote:
> I would pass a "dbus::FileDescriptor*" here instead of const int fd. Please
see
> my comment in bluetooth_media_transport_client.cc.

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
File chromeos/dbus/fake_bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:134: ObjectPath
endpoint_path = GetEndpointPath(object_path);
On 2015/03/12 03:42:54, armansito wrote:
> const ObjectPath&

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:156: }
On 2015/03/12 03:42:54, armansito wrote:
> Maybe add a DCHECK here that both fds[0] and fd[1] are positive.

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:169: ObjectPath
endpoint_path = GetEndpointPath(object_path);
On 2015/03/12 03:42:54, armansito wrote:
> const ObjectPath&

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:197:
SetState(endpoint_path, "active");
On 2015/03/12 03:42:54, armansito wrote:
> A lot of the code between TryAcquire and Acquire is the same. Perhaps you can
> unify them in a helper?

Acquire and TryAcquire have different error messages for |error_callback|, so I
prefer not to unify them.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:245: ObjectPath
transport_path = transport->path;
On 2015/03/12 03:42:54, armansito wrote:
> const ObjectPath&

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:289: VLOG(1) <<
"WriteData";
On 2015/03/12 03:42:54, armansito wrote:
> Maybe log the number of bytes here? Might be useful for debugging.

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:294: VLOG(1) <<
"WriteData - write operation rejected on endpoint: "
On 2015/03/12 03:42:54, armansito wrote:
> You should probably log here that this was rejected because the state isn't
> "active". Will save you time later when reading the logs :)

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:300: VLOG(1) <<
"WriteData - invalid file descriptors";
On 2015/03/12 03:42:54, armansito wrote:
> nit: say "invalid input file descriptor"?

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:305:
write(transport->input_fd->GetPlatformFile(), bytes.data(), bytes.size());
On 2015/03/12 03:42:54, armansito wrote:
> I wonder if you should prevent sending more bytes than the current read MTU?
At
> least this would make the behavior consistent with BlueZ (but I guess you
> wouldn't be able to test invalid behavior in that case, so maybe what you have
> already is better).
I'll keep this for now.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:317: auto it =
endpoint_to_transport_map_.find(endpoint_path);
On 2015/03/12 03:42:54, armansito wrote:
> const auto& it

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:317: auto it =
endpoint_to_transport_map_.find(endpoint_path);
On 2015/03/12 03:42:54, armansito wrote:
> const auto& it

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:330: auto it =
transport_to_endpoint_map_.find(transport_path);
On 2015/03/12 03:42:54, armansito wrote:
> const auto& it

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
File chromeos/dbus/fake_bluetooth_media_transport_client.h (right):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.h:80: // Writes bytes to the
input file descriptor, |input_fd|, accosiated with a
On 2015/03/12 03:42:54, armansito wrote:
> nit: s/accosiated/associated. Try pronouncing your version, it sounds funny.

:P

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.h:81: // Transport object
which is bound to |endpoint_path|.
On 2015/03/12 03:42:54, armansito wrote:
> nit: s/Transport/transport/. Lowercase, because you probably don't want to be
> referring to "struct Transport" which is private to this class.

Done.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.h:107: // will be
initialized when Acquire/TryAcquire is called.
On 2015/03/12 03:42:54, armansito wrote:
> I would also say here that this is the internal end of the socketpair that
gets
> created for simulation purposes.

Done.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
File device/bluetooth/bluetooth_audio_sink.h (right):

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink.h:76: // Callled when there is audio data
available. |audio_sink| indicates the
On 2015/03/12 03:42:55, armansito wrote:
> nit: Called

Done.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink.h:78: // is the number of the bytes in
|data|.
On 2015/03/12 03:42:55, armansito wrote:
> I would explain here that this method provides the raw audio data which hasn't
> been processed or assembled in any way and that the RTP units and SBC decoding
> needs to happen here.

Done.

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

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:351: read_failed_logged_ =
false;
On 2015/03/12 03:42:55, armansito wrote:
> I would rename this variable to something like |read_has_failed_|, so that
it's
> a bit more generic and you could reuse it for things other than logging in the
> future.

Done.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:384: file_.reset();
On 2015/03/12 03:42:55, armansito wrote:
> Add a comment here saying that this will close the fd.

Done.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:472: VLOG(1) << "Unregisterd -
endpoint: " << endpoint_path_.value();
On 2015/03/12 03:42:55, armansito wrote:
> nit: s/Unregisterd/Unregistered/

Done.

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.cc:498: // Avoids unnecessary
memory reallocation if read MTU doesn't change.
On 2015/03/12 03:42:55, armansito wrote:
> Nice!

:)

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

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos.h:87: void
OnFileCanWriteWithoutBlocking(int fd) override;
On 2015/03/12 03:42:55, armansito wrote:
> These can be private.

Done.

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

https://codereview.chromium.org/993273002/diff/1/device/bluetooth/bluetooth_a...
device/bluetooth/bluetooth_audio_sink_chromeos_unittest.cc:837:
EXPECT_EQ(observer_->total_read_, data_one.size());
On 2015/03/12 03:42:55, armansito wrote:
> It would be cool if you could also check that the data you read matches the
data
> that you sent over the wire. You can simply copy the data in the observer
method
> to a local variable in the test harness, just like you do with |total_read_|.

Added!

[armansito, 2015-03-12 23:59:08]

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
File chromeos/dbus/fake_bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:197:
SetState(endpoint_path, "active");
On 2015/03/12 22:33:31, Miao wrote:
> On 2015/03/12 03:42:54, armansito wrote:
> > A lot of the code between TryAcquire and Acquire is the same. Perhaps you
can
> > unify them in a helper?
> 
> Acquire and TryAcquire have different error messages for |error_callback|, so
I
> prefer not to unify them.

I don't think that's a good enough excuse to have so much duplicate code.

https://codereview.chromium.org/993273002/diff/1/chromeos/dbus/fake_bluetooth...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:305:
write(transport->input_fd->GetPlatformFile(), bytes.data(), bytes.size());
On 2015/03/12 22:33:31, Miao wrote:
> On 2015/03/12 03:42:54, armansito wrote:
> > I wonder if you should prevent sending more bytes than the current read MTU?
> At
> > least this would make the behavior consistent with BlueZ (but I guess you
> > wouldn't be able to test invalid behavior in that case, so maybe what you
have
> > already is better).
> I'll keep this for now.

sgtm

[armansito, 2015-03-13 00:00:31]

https://codereview.chromium.org/993273002/diff/20001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/993273002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:199: callback.Run(new
dbus::FileDescriptor(fds[1]), kReadMtu, kWriteMtu);
You're potentially leaking memory here, don't do this. Simply initialize a
FileDescriptor on the stack and pass a pointer to it.

[Miao, 2015-03-13 00:41:41]

https://codereview.chromium.org/993273002/diff/20001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_media_transport_client.cc (right):

https://codereview.chromium.org/993273002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_media_transport_client.cc:199: callback.Run(new
dbus::FileDescriptor(fds[1]), kReadMtu, kWriteMtu);
On 2015/03/13 00:00:31, armansito wrote:
> You're potentially leaking memory here, don't do this. Simply initialize a
> FileDescriptor on the stack and pass a pointer to it.

Done.

[armansito, 2015-03-13 01:31:42]

Looks a lot better, lgtm with one comment.

https://codereview.chromium.org/993273002/diff/40001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_media_transport_client.h (right):

https://codereview.chromium.org/993273002/diff/40001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_media_transport_client.h:122: void
AcquireDelegate(bool try_flag,
I'd rename this to AcquireInternal to be more consistent with the rest of the
code and because the term Delegate doesn't really apply here.

[Miao, 2015-03-13 19:21:00]

https://codereview.chromium.org/993273002/diff/40001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_media_transport_client.h (right):

https://codereview.chromium.org/993273002/diff/40001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_media_transport_client.h:122: void
AcquireDelegate(bool try_flag,
On 2015/03/13 01:31:42, armansito wrote:
> I'd rename this to AcquireInternal to be more consistent with the rest of the
> code and because the term Delegate doesn't really apply here.

Done.

[Miao, 2015-03-13 19:21:16]

The CQ bit was checked by mcchou@chromium.org

[Miao, 2015-03-13 19:21:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
armansito@chromium.org
Link to the patchset: https://codereview.chromium.org/993273002/#ps60001 (title:
" ")

[commit-bot: I haz the power, 2015-03-13 19:21:36]

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

[commit-bot: I haz the power, 2015-03-13 21:00:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-13 21:01:09]

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