Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin.
It also optimized previous codes by renaming interfaces, data structures and so on.

BUG=579202
Committed: https://crrev.com/74459acf6390eed6009b8bf5edad68d7e03bf008
Cr-Commit-Position: refs/heads/master@{#378552}

[gogerald1, 2016-02-24 22:43:14]

Description was changed from

==========
implement gatt characteristic

BUG=
==========

to

==========
Implement BluetoothRemoteGattCharacteristicWin patch one

This CL implements partial of BluetoothRemoteGattCharacteristicWin. It also
optimized previous code by renaming. 


BUG=579202
==========

[gogerald1, 2016-02-24 23:12:20]

Patchset #3 (id:40001) has been deleted

[gogerald1, 2016-02-26 17:35:12]

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

[commit-bot: I haz the power, 2016-02-26 17:36:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728163006/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728163006/120001

[gogerald1, 2016-02-26 17:44:36]

Description was changed from

==========
Implement BluetoothRemoteGattCharacteristicWin patch one

This CL implements partial of BluetoothRemoteGattCharacteristicWin. It also
optimized previous code by renaming. 


BUG=579202
==========

to

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. It
also optimized previous codes by renaming interfaces and structures. 


BUG=579202
==========

[gogerald1, 2016-02-26 17:44:55]

Patchset #6 (id:120001) has been deleted

[gogerald1, 2016-02-26 17:44:59]

Patchset #5 (id:100001) has been deleted

[gogerald1, 2016-02-26 17:45:07]

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

[commit-bot: I haz the power, 2016-02-26 17:45:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728163006/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728163006/140001

[gogerald1, 2016-02-26 17:46:08]

Description was changed from

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. It
also optimized previous codes by renaming interfaces and structures. 


BUG=579202
==========

to

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. It
also optimized previous codes by renaming interfaces, data structures and so on.



BUG=579202
==========

[gogerald1, 2016-02-26 17:46:31]

gogerald@chromium.org changed reviewers:
+ scheib@chromium.org

[gogerald1, 2016-02-26 17:46:32]

Hi Vincent, PTAL.

[gogerald1, 2016-02-26 18:16:45]

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

[gogerald1, 2016-02-26 18:17:50]

Patchset #6 (id:160001) has been deleted

[commit-bot: I haz the power, 2016-02-26 18:18:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728163006/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728163006/140001

[gogerald1, 2016-02-26 18:20:09]

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

[commit-bot: I haz the power, 2016-02-26 18:21:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728163006/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728163006/180001

[gogerald1, 2016-02-26 18:32:13]

Description was changed from

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. It
also optimized previous codes by renaming interfaces, data structures and so on.



BUG=579202
==========

to

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. 
It also optimized previous codes by renaming interfaces, data structures and so
on. 


BUG=579202
==========

[commit-bot: I haz the power, 2016-02-26 19:45:34]

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

[commit-bot: I haz the power, 2016-02-26 19:45:35]

Dry run: This issue passed the CQ dry run.

[gogerald1, 2016-02-26 21:33:48]

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

[commit-bot: I haz the power, 2016-02-26 21:38:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728163006/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728163006/200001

[commit-bot: I haz the power, 2016-02-26 23:17:01]

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

[commit-bot: I haz the power, 2016-02-26 23:17:02]

Dry run: This issue passed the CQ dry run.

[scheib, 2016-03-01 00:06:23]

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_low_energy_win.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_low_energy_win.cc:777: USHORT actual_length =
required_length;
I overlooked that you did not address this in previous patch: 
https://codereview.chromium.org/1690133002/diff/80001/device/bluetooth/blueto...
Please respond to all review comments raised.

Please address here and in ::ReadIncludedServicesOfAService to improve
readability.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_low_energy_win_fake.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_low_energy_win_fake.cc:307: GattDescriptor*
descriptor = new GattDescriptor();
Create into a scoped ptr, and then std::move it into the container at last
statement of this method.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc:116:
std::vector<BluetoothGattDescriptor*> has_descriptors;
has_descriptors -> descriptors.
Has implies a Boolean type of true or false.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_characteristic_win.h (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_characteristic_win.h:96: // has been send
out to avoid duplicate notification.
sent

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc:31: service_path_ =
static_cast<BluetoothRemoteGattServiceWin*>(
Static casts are always a red flag and should be minimized. You can create a
GetWinService method, similar to e.g. GetAndroidAdapter().
https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/b...

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_descriptor_win.h (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_descriptor_win.h:48: BluetoothAdapterWin*
adapter_;
Do Characteristics and Descriptors need to access adapter_ often? In current
patches it seems that access is infrequent and that storing it on all objects
isn't needed. Instead GetAdapter() can use the parent_serivice_ &
parent_characteristic_;

[gogerald1, 2016-03-01 16:56:24]

Patchset #8 (id:220001) has been deleted

[gogerald1, 2016-03-01 16:59:59]

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

[gogerald1, 2016-03-01 17:00:13]

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_low_energy_win.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_low_energy_win.cc:777: USHORT actual_length =
required_length;
On 2016/03/01 00:06:22, scheib wrote:
> I overlooked that you did not address this in previous patch: 
>
https://codereview.chromium.org/1690133002/diff/80001/device/bluetooth/blueto...
> Please respond to all review comments raised.
> 
> Please address here and in ::ReadIncludedServicesOfAService to improve
> readability.

Done.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_low_energy_win_fake.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_low_energy_win_fake.cc:307: GattDescriptor*
descriptor = new GattDescriptor();
On 2016/03/01 00:06:22, scheib wrote:
> Create into a scoped ptr, and then std::move it into the container at last
> statement of this method.

Done.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc:116:
std::vector<BluetoothGattDescriptor*> has_descriptors;
On 2016/03/01 00:06:22, scheib wrote:
> has_descriptors -> descriptors.
> Has implies a Boolean type of true or false.

Done.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_characteristic_win.h (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_characteristic_win.h:96: // has been send
out to avoid duplicate notification.
On 2016/03/01 00:06:22, scheib wrote:
> sent

Done.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc:31: service_path_ =
static_cast<BluetoothRemoteGattServiceWin*>(
On 2016/03/01 00:06:22, scheib wrote:
> Static casts are always a red flag and should be minimized. You can create a
> GetWinService method, similar to e.g. GetAndroidAdapter().
>
https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/b...

Done.

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_remote_gatt_descriptor_win.h (right):

https://codereview.chromium.org/1728163006/diff/200001/device/bluetooth/bluet...
device/bluetooth/bluetooth_remote_gatt_descriptor_win.h:48: BluetoothAdapterWin*
adapter_;
On 2016/03/01 00:06:22, scheib wrote:
> Do Characteristics and Descriptors need to access adapter_ often? In current
> patches it seems that access is infrequent and that storing it on all objects
> isn't needed. Instead GetAdapter() can use the parent_serivice_ &
> parent_characteristic_;

Done.

[commit-bot: I haz the power, 2016-03-01 17:00:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728163006/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728163006/240001

[scheib, 2016-03-01 17:22:50]

Thanks, LGTM

[commit-bot: I haz the power, 2016-03-01 18:50:57]

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

[commit-bot: I haz the power, 2016-03-01 18:50:58]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[gogerald1, 2016-03-01 19:09:05]

The CQ bit was checked by gogerald@chromium.org

[commit-bot: I haz the power, 2016-03-01 19:11:01]

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

[commit-bot: I haz the power, 2016-03-01 20:45:46]

Description was changed from

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. 
It also optimized previous codes by renaming interfaces, data structures and so
on. 


BUG=579202
==========

to

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. 
It also optimized previous codes by renaming interfaces, data structures and so
on. 


BUG=579202
==========

[commit-bot: I haz the power, 2016-03-01 20:45:48]

Committed patchset #8 (id:240001)

[commit-bot: I haz the power, 2016-03-01 20:47:17]

Description was changed from

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin. 
It also optimized previous codes by renaming interfaces, data structures and so
on. 


BUG=579202
==========

to

==========
Implement BluetoothRemoteGattCharacteristicWin::GetDescriptors

This CL implements GetDescriptors() in BluetoothRemoteGattCharacteristicWin.
It also optimized previous codes by renaming interfaces, data structures and so
on.

BUG=579202

Committed: https://crrev.com/74459acf6390eed6009b8bf5edad68d7e03bf008
Cr-Commit-Position: refs/heads/master@{#378552}
==========

[commit-bot: I haz the power, 2016-03-01 20:47:18]

Patchset 8 (id:??) landed as
https://crrev.com/74459acf6390eed6009b8bf5edad68d7e03bf008
Cr-Commit-Position: refs/heads/master@{#378552}