Refactor WebBluetoothServiceClient in the web_bluetooth.mojom (another version)

Refactor WebBluetoothServiceClient in the web_bluetooth.mojom (another version)

This is another version of:
https://codereview.chromium.org/2718583002/

This CL does the same thing as the above CL, but in a different 
approach. The main difference is in the web_bluetooth.mojom file, for 
the RemoteServerConnect() and RemoteCharacteristicStartNotifications(), 
instead of passing back an interface request, this CL just passes an
interface pointer to the Bluetooth service implementation.

This CL changes the WebBluetoothServiceClient in the web_bluetooth.mojom
into two clients:
WebBluetoothCharacteristicClient
WebBluetoothServerClient

BUG=682050

[juncai, 2017-03-16 01:13:02]

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

[juncai, 2017-03-16 01:13:38]

Description was changed from

==========
WebBluetoothServiceClient in the web_bluetooth.mojom another version


BUG=682050
==========

to

==========
WebBluetoothServiceClient in the web_bluetooth.mojom another version


BUG=682050
==========

[commit-bot: I haz the power, 2017-03-16 01:13:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2017-03-16 01:17:25]

Description was changed from

==========
WebBluetoothServiceClient in the web_bluetooth.mojom another version


BUG=682050
==========

to

==========
Refactor WebBluetoothServiceClient in the web_bluetooth.mojom (another version)

This is another version of:
https://codereview.chromium.org/2718583002/

This CL does the same thing as the above CL, but in a different approach. The
main difference is in the web_bluetooth.mojom file, for the
RemoteServerConnect() and RemoteCharacteristicStartNotifications(), instead of
passing back an interface request, this CL just passes an
interface pointer to the Bluetooth service implementation.

This CL changes the WebBluetoothServiceClient in the web_bluetooth.mojom
into two clients:
WebBluetoothCharacteristicClient
WebBluetoothServerClient

BUG=682050
==========

[juncai, 2017-03-16 01:17:57]

Description was changed from

==========
Refactor WebBluetoothServiceClient in the web_bluetooth.mojom (another version)

This is another version of:
https://codereview.chromium.org/2718583002/

This CL does the same thing as the above CL, but in a different approach. The
main difference is in the web_bluetooth.mojom file, for the
RemoteServerConnect() and RemoteCharacteristicStartNotifications(), instead of
passing back an interface request, this CL just passes an
interface pointer to the Bluetooth service implementation.

This CL changes the WebBluetoothServiceClient in the web_bluetooth.mojom
into two clients:
WebBluetoothCharacteristicClient
WebBluetoothServerClient

BUG=682050
==========

to

==========
Refactor WebBluetoothServiceClient in the web_bluetooth.mojom (another version)

This is another version of:
https://codereview.chromium.org/2718583002/

This CL does the same thing as the above CL, but in a different 
approach. The main difference is in the web_bluetooth.mojom file, for 
the RemoteServerConnect() and RemoteCharacteristicStartNotifications(), 
instead of passing back an interface request, this CL just passes an
interface pointer to the Bluetooth service implementation.

This CL changes the WebBluetoothServiceClient in the web_bluetooth.mojom
into two clients:
WebBluetoothCharacteristicClient
WebBluetoothServerClient

BUG=682050
==========

[juncai, 2017-03-16 01:28:18]

juncai@chromium.org changed reviewers:
+ dcheng@chromium.org, ortuno@chromium.org, scheib@chromium.org

[juncai, 2017-03-16 01:28:18]

Please take a look.

[commit-bot: I haz the power, 2017-03-16 02:38:21]

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

[commit-bot: I haz the power, 2017-03-16 02:38:22]

Dry run: This issue passed the CQ dry run.

[scheib, 2017-03-16 06:08:51]

I think this does look cleaner. Why didn't you upload the patch to the previous
review?

https://codereview.chromium.org/2751293002/diff/1/content/browser/bluetooth/f...
File content/browser/bluetooth/frame_connected_bluetooth_devices_unittest.cc
(right):

https://codereview.chromium.org/2751293002/diff/1/content/browser/bluetooth/f...
content/browser/bluetooth/frame_connected_bluetooth_devices_unittest.cc:43:
CreateWebBluetoothServerClientAssociatedPtr() {
This method is called often. It's name seems overly verbose. Perhaps just
CreateServerClient()

https://codereview.chromium.org/2751293002/diff/1/content/browser/bluetooth/f...
content/browser/bluetooth/frame_connected_bluetooth_devices_unittest.cc:133:
auto client = CreateWebBluetoothServerClientAssociatedPtr();
These locals "auto client" are OK, but also don't seem to be needed. Cant the
call to Create...ServerClient just be inlined?

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom
(right):

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom:262: //
Classes that implement this interface will be notified of device events.
We typically list the Bluetooth and GATT concepts in order of adapter, device,
gatt-server, service, characteristic, descriptor. Let's order these client
interfaces with the Server before the Characteristic.

[haraken, 2017-03-16 08:39:43]

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

[haraken, 2017-03-16 08:39:44]

Drive-by

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
(right):

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:36:
DisconnectIfConnected();

Do we want to call Dispose() as well?

[juncai, 2017-03-16 18:06:24]

I moved this CL to:
https://codereview.chromium.org/2718583002/
I also addressed this CL's comments in the above CL.

Sorry for the confusion. I will close this CL.

https://codereview.chromium.org/2751293002/diff/1/content/browser/bluetooth/f...
File content/browser/bluetooth/frame_connected_bluetooth_devices_unittest.cc
(right):

https://codereview.chromium.org/2751293002/diff/1/content/browser/bluetooth/f...
content/browser/bluetooth/frame_connected_bluetooth_devices_unittest.cc:43:
CreateWebBluetoothServerClientAssociatedPtr() {
On 2017/03/16 06:08:51, scheib wrote:
> This method is called often. It's name seems overly verbose. Perhaps just
> CreateServerClient()

Done.

https://codereview.chromium.org/2751293002/diff/1/content/browser/bluetooth/f...
content/browser/bluetooth/frame_connected_bluetooth_devices_unittest.cc:133:
auto client = CreateWebBluetoothServerClientAssociatedPtr();
On 2017/03/16 06:08:51, scheib wrote:
> These locals "auto client" are OK, but also don't seem to be needed. Cant the
> call to Create...ServerClient just be inlined?

Done.

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
(right):

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:36:
DisconnectIfConnected();
On 2017/03/16 08:39:43, haraken wrote:
> 
> Do we want to call Dispose() as well?

Done.

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom
(right):

https://codereview.chromium.org/2751293002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom:262: //
Classes that implement this interface will be notified of device events.
On 2017/03/16 06:08:51, scheib wrote:
> We typically list the Bluetooth and GATT concepts in order of adapter, device,
> gatt-server, service, characteristic, descriptor. Let's order these client
> interfaces with the Server before the Characteristic. 

Done.

[juncai, 2017-03-16 18:07:03]

On 2017/03/16 06:08:51, scheib wrote:
> I think this does look cleaner. Why didn't you upload the patch to the
previous
> review?

Thanks! I moved this CL to:
https://codereview.chromium.org/2718583002/