bluetooth: Blink side implementation of getPrimaryService

Source/modules/bluetooth/BluetoothGATTService.cpp

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "modules/bluetooth/BluetoothGATTService.h"
+
+#include "public/platform/modules/bluetooth/WebBluetoothGATTRemoteServer.h"
+#include "wtf/OwnPtr.h"
+
+namespace blink {
+
+BluetoothGATTService::BluetoothGATTService(const WebBluetoothGATTService& webService)
+    : m_webService(webService)

[haraken, 2015/06/03 03:45:38]

We're deep-copying the webService here. Is this intentional?

[ortuno, 2015/06/03 17:02:18]

No. This pattern slipped under in all of our classes. I will submit a follow up
patch to fix all instances.

+{
+}
+
+BluetoothGATTService* BluetoothGATTService::take(ScriptPromiseResolver*, WebBluetoothGATTService* webServiceRawPointer)
+{
+    if (!webServiceRawPointer) {
+        return nullptr;
+    }
+    OwnPtr<WebBluetoothGATTService> webService = adoptPtr(webServiceRawPointer);
+    return new BluetoothGATTService(*webService);

[haraken, 2015/06/03 03:45:38]

Maybe you want to pass in webService.learPtr() here and let BluetoothGATTService
keep a raw pointer to the webService. Then the webService is explicitly deleted
in the following dispose().

(As discussed above, the mis-design of the lifetime management needs to be fixed
soon.)

[Jeffrey Yasskin, 2015/06/03 15:10:04]

I mentioned this in
https://codereview.chromium.org/1146163005/diff/20001/Source/modules/bluetoot...:
if we want to avoid the copy (which may not matter: the contents are all
primitives or refcounted WebStrings), the right thing to do seems to be to have
BluetoothGATTService store an OwnPtr, not need an explicit destructor to destroy
the raw pointer.

According to
https://code.google.com/p/chromium/codesearch/#chromium/src/third_party/WebKi...,
we never call both take() and dispose(), so there is no "following dispose()" if
take() was called.

[ortuno, 2015/06/03 17:02:19]

We've been mistakenly following this pattern in all our Blink classes. I plan to
submit a follow up patch to change all of them to what jyasskin described.

+}
+
+void BluetoothGATTService::dispose(WebBluetoothGATTService* webService)
+{
+    delete webService;
+}
+
+} // namespace blink