Index: third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp |
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp |
index a3fccdf812cf473a4df53b47e5038142f049ea44..6c09d4acfb0192f6bcec5153ae7d89c46f4df24d 100644 |
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp |
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp |
@@ -15,14 +15,16 @@ |
#include "modules/bluetooth/BluetoothError.h" |
#include "modules/bluetooth/BluetoothRemoteGATTService.h" |
#include "modules/bluetooth/BluetoothUUID.h" |
-#include "mojo/public/cpp/bindings/associated_interface_ptr.h" |
#include <utility> |
namespace blink { |
BluetoothRemoteGATTServer::BluetoothRemoteGATTServer(ExecutionContext* context, |
BluetoothDevice* device) |
- : ContextLifecycleObserver(context), m_device(device), m_connected(false) {} |
+ : ContextLifecycleObserver(context), |
+ m_clientBinding(this), |
+ m_device(device), |
+ m_connected(false) {} |
BluetoothRemoteGATTServer* BluetoothRemoteGATTServer::Create( |
ExecutionContext* context, |
@@ -35,7 +37,12 @@ void BluetoothRemoteGATTServer::contextDestroyed(ExecutionContext*) { |
} |
void BluetoothRemoteGATTServer::GATTServerDisconnected() { |
- DispatchDisconnected(); |
+ // This function can only be called if |m_clientBinding| is bound and |
+ // |m_clientBinding| can only be bound if |m_connected| is true. |
ortuno
2017/03/30 04:20:48
I might be missing something here but m_clientBind
juncai
2017/03/30 18:44:57
Done.
|
+ // Therefore we skip Step 2 of Responding to Disconnection. |
+ // https://webbluetoothcg.github.io/web-bluetooth/#disconnection-events |
+ DCHECK(m_clientBinding.is_bound()); |
+ CleanupDisconnectedDevice(true /* dispatchEvent */); |
} |
void BluetoothRemoteGATTServer::AddToActiveAlgorithms( |
@@ -53,35 +60,40 @@ bool BluetoothRemoteGATTServer::RemoveFromActiveAlgorithms( |
return true; |
} |
-void BluetoothRemoteGATTServer::DisconnectIfConnected() { |
- if (m_connected) { |
- SetConnected(false); |
- ClearActiveAlgorithms(); |
- mojom::blink::WebBluetoothService* service = |
- m_device->bluetooth()->Service(); |
- service->RemoteServerDisconnect(m_device->id()); |
- } |
-} |
- |
-void BluetoothRemoteGATTServer::CleanupDisconnectedDeviceAndFireEvent() { |
+void BluetoothRemoteGATTServer::CleanupDisconnectedDevice(bool dispatchEvent) { |
+ // This implements the specification "clean up the disconnected device" |
+ // algorithm: |
+ // https://webbluetoothcg.github.io/web-bluetooth/#clean-up-the-disconnected-device |
DCHECK(m_connected); |
SetConnected(false); |
ClearActiveAlgorithms(); |
- m_device->ClearAttributeInstanceMapAndFireEvent(); |
+ m_device->ClearAttributeInstanceMap(); |
+ // TODO(juncai): Below this line there are steps in the above specification |
+ // which we are not explicitly handling here (clearing representedService, |
+ // notification & indications). Need to add comment here explaining why no |
ortuno
2017/03/30 04:20:48
Clearing the represented services, characteristics
juncai
2017/03/30 18:44:57
Done.
|
+ // explicit actions are needed. |
+ if (dispatchEvent) { |
+ m_device->dispatchEvent( |
+ Event::createBubble(EventTypeNames::gattserverdisconnected)); |
+ } |
} |
-void BluetoothRemoteGATTServer::DispatchDisconnected() { |
+void BluetoothRemoteGATTServer::HandleClientConnectionError() { |
if (!m_connected) { |
return; |
} |
- CleanupDisconnectedDeviceAndFireEvent(); |
+ CleanupDisconnectedDevice(true /* dispatchEvent */); |
} |
void BluetoothRemoteGATTServer::Dispose() { |
- DisconnectIfConnected(); |
- // The pipe to this object must be closed when is marked unreachable to |
- // prevent messages from being dispatched before lazy sweeping. |
- m_clientBindings.CloseAllBindings(); |
+ if (!m_connected) { |
+ return; |
+ } |
+ // The object is being garbage collected or the context is being destroyed, |
+ // so no need to dispatch a gattserverdisconnected event. |
+ CleanupDisconnectedDevice(false /* dispatchEvent */); |
+ DCHECK(m_clientBinding.is_bound()); |
+ m_clientBinding.Close(); |
} |
DEFINE_TRACE(BluetoothRemoteGATTServer) { |
@@ -109,27 +121,36 @@ ScriptPromise BluetoothRemoteGATTServer::connect(ScriptState* scriptState) { |
ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState); |
ScriptPromise promise = resolver->promise(); |
- mojom::blink::WebBluetoothService* service = m_device->bluetooth()->Service(); |
- mojom::blink::WebBluetoothServerClientAssociatedPtrInfo ptrInfo; |
- auto request = mojo::MakeRequest(&ptrInfo); |
- m_clientBindings.AddBinding(this, std::move(request)); |
- |
- service->RemoteServerConnect( |
- m_device->id(), std::move(ptrInfo), |
- convertToBaseCallback( |
- WTF::bind(&BluetoothRemoteGATTServer::ConnectCallback, |
- wrapPersistent(this), wrapPersistent(resolver)))); |
+ if (m_connected) { |
ortuno
2017/03/30 04:20:48
hmm I think this is breaking spec'ed behavior e.g.
juncai
2017/03/30 18:44:57
hmm... I am not sure I understand it. When the sec
ortuno
2017/03/31 06:07:28
According to step 5.2 of the connect() algorithm i
|
+ resolver->resolve(this); |
+ } else { |
+ mojom::blink::WebBluetoothService* service = |
+ m_device->bluetooth()->Service(); |
+ mojom::blink::WebBluetoothServerClientAssociatedPtrInfo ptrInfo; |
+ m_clientBinding.Bind(&ptrInfo); |
+ m_clientBinding.set_connection_error_handler(convertToBaseCallback( |
ortuno
2017/03/30 04:20:48
Since you are binding this here and in the browser
juncai
2017/03/30 18:44:57
Yes, only if |m_connected| is true. And in that si
ortuno
2017/03/31 06:07:28
I think the fact that the client is receiving conn
|
+ WTF::bind(&BluetoothRemoteGATTServer::HandleClientConnectionError, |
+ wrapWeakPersistent(this)))); |
+ |
+ service->RemoteServerConnect( |
+ m_device->id(), std::move(ptrInfo), |
+ convertToBaseCallback( |
+ WTF::bind(&BluetoothRemoteGATTServer::ConnectCallback, |
+ wrapPersistent(this), wrapPersistent(resolver)))); |
+ } |
return promise; |
} |
void BluetoothRemoteGATTServer::disconnect(ScriptState* scriptState) { |
- if (!m_connected) |
+ ClearActiveAlgorithms(); |
dcheng
2017/03/28 02:25:01
Just curious: how come we need to ClearActiveAlgor
ortuno
2017/03/30 04:20:48
I asked the same in the spec a while ago :) The an
juncai
2017/03/30 18:44:57
comment added.
Done.
juncai
2017/03/30 18:44:57
Thanks Gio for the explanation!
|
+ if (!m_connected) { |
return; |
- CleanupDisconnectedDeviceAndFireEvent(); |
- m_clientBindings.CloseAllBindings(); |
- mojom::blink::WebBluetoothService* service = m_device->bluetooth()->Service(); |
- service->RemoteServerDisconnect(m_device->id()); |
+ } |
+ CleanupDisconnectedDevice(true /* dispatchEvent */); |
+ DCHECK(m_clientBinding.is_bound()); |
+ // Implicitly signals disconnect via the connection_error_handler. |
+ m_clientBinding.Close(); |
} |
// Callback that allows us to resolve the promise with a single service or |