Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.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 "modules/bluetooth/BluetoothRemoteGATTCharacteristic.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMDataView.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/events/Event.h"
 #include "modules/bluetooth/BluetoothCharacteristicProperties.h"
 #include "modules/bluetooth/BluetoothError.h"
 #include "modules/bluetooth/BluetoothSupplement.h"
 #include "public/platform/modules/bluetooth/WebBluetooth.h"
 
 namespace blink {
 
 namespace {
 
-PassRefPtr<DOMDataView> ConvertWebVectorToDataView(
-    const WebVector<uint8_t>& webVector)
+DOMDataView* ConvertWebVectorToDataView(const WebVector<uint8_t>& webVector)
 {
     static_assert(sizeof(*webVector.data()) == 1, "uint8_t should be a single byte");
-    RefPtr<DOMArrayBuffer> domBuffer = DOMArrayBuffer::create(webVector.data(), webVector.size());
-    RefPtr<DOMDataView> domDataView = DOMDataView::create(domBuffer, 0, webVector.size());
-    return domDataView;
+    DOMArrayBuffer* domBuffer = DOMArrayBuffer::create(webVector.data(), webVector.size());
+    return DOMDataView::create(domBuffer, 0, webVector.size());
 }
 
 } // anonymous namespace
 
 BluetoothRemoteGATTCharacteristic::BluetoothRemoteGATTCharacteristic(ExecutionContext* context, PassOwnPtr<WebBluetoothRemoteGATTCharacteristicInit> webCharacteristic)
     : ActiveDOMObject(context)
     , m_webCharacteristic(webCharacteristic)
     , m_stopped(false)
 {
     m_properties = BluetoothCharacteristicProperties::create(m_webCharacteristic->characteristicProperties);
     // See example in Source/platform/heap/ThreadState.h
     ThreadState::current()->registerPreFinalizer(this);
 }
 
 BluetoothRemoteGATTCharacteristic* BluetoothRemoteGATTCharacteristic::take(ScriptPromiseResolver* resolver, PassOwnPtr<WebBluetoothRemoteGATTCharacteristicInit> webCharacteristic)
 {
     if (!webCharacteristic) {
         return nullptr;
     }
     BluetoothRemoteGATTCharacteristic* characteristic = new BluetoothRemoteGATTCharacteristic(resolver->getExecutionContext(), webCharacteristic);
     // See note in ActiveDOMObject about suspendIfNeeded.
     characteristic->suspendIfNeeded();
     return characteristic;
 }
 
-void BluetoothRemoteGATTCharacteristic::setValue(
-    const PassRefPtr<DOMDataView>& domDataView)
+void BluetoothRemoteGATTCharacteristic::setValue(DOMDataView* domDataView)
 {
     m_value = domDataView;
 }
 
 void BluetoothRemoteGATTCharacteristic::dispatchCharacteristicValueChanged(
     const WebVector<uint8_t>& value)
 {
-    RefPtr<DOMDataView> domDataView = ConvertWebVectorToDataView(value);
-    this->setValue(domDataView);
+    this->setValue(ConvertWebVectorToDataView(value));
     dispatchEvent(Event::create(EventTypeNames::characteristicvaluechanged));
 }
 
 void BluetoothRemoteGATTCharacteristic::stop()
 {
     notifyCharacteristicObjectRemoved();
 }
 
 void BluetoothRemoteGATTCharacteristic::dispose()
 {
@@ skipping 32 matching lines @@
 
 class ReadValueCallback : public WebBluetoothReadValueCallbacks {
 public:
     ReadValueCallback(BluetoothRemoteGATTCharacteristic* characteristic, ScriptPromiseResolver* resolver) : m_webCharacteristic(characteristic), m_resolver(resolver) {}
 
     void onSuccess(const WebVector<uint8_t>& value) override
     {
         if (!m_resolver->getExecutionContext() || m_resolver->getExecutionContext()->activeDOMObjectsAreStopped())
             return;
 
-        RefPtr<DOMDataView> domDataView = ConvertWebVectorToDataView(value);
-        if (m_webCharacteristic) {
+        DOMDataView* domDataView = ConvertWebVectorToDataView(value);
+        if (m_webCharacteristic)
             m_webCharacteristic->setValue(domDataView);
-        }
+
         m_resolver->resolve(domDataView);

[ortuno, 2016/04/10 17:53:37]

Just to make sure I understand this: Calling m_webCharacteristic->setValue()
with the pointer to the data view will make Oilpan increase the ref count for
the Data View. When we resolve that the promise with the pointer to the data
view, Oilpan will once again increase the ref count. If the characteristic
object gets GCed then Oilpan will decrease the ref count but it will keep the
object alive because of the reference we used when resolving the promise. Is all
of this correct?

I' just want to be sure that we won't run into use after delete if the
characteristic object is GCed but someone is still holding on to the pointer
used to resolve the promise.

[sof, 2016/04/10 18:12:51]

DOMDataView is on the Oilpan heap, so there's no ref counting going on for it.
That DOMDataView will only be eligible for GCing once it is no longer reachable,
i.e., when both this characteristic object and the promise have let go (or
themselves have become unreachable from the running renderer process.)

[ortuno, 2016/04/10 18:15:52]

Got it. Thanks! Bluetooth LGTM

     }
 
     void onError(const WebBluetoothError& e) override
     {
         if (!m_resolver->getExecutionContext() || m_resolver->getExecutionContext()->activeDOMObjectsAreStopped())
             return;
         m_resolver->reject(BluetoothError::take(m_resolver, e));
     }
 
 private:
@@ skipping 75 matching lines @@
 {
     WebBluetooth* webbluetooth = BluetoothSupplement::fromScriptState(scriptState);
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
     webbluetooth->stopNotifications(m_webCharacteristic->characteristicInstanceID, this, new CallbackPromiseAdapter<void, BluetoothError>(resolver));
     return promise;
 }
 
 DEFINE_TRACE(BluetoothRemoteGATTCharacteristic)
 {
+    visitor->trace(m_properties);
+    visitor->trace(m_value);
     RefCountedGarbageCollectedEventTargetWithInlineData<BluetoothRemoteGATTCharacteristic>::trace(visitor);
     ActiveDOMObject::trace(visitor);
-    visitor->trace(m_properties);
 }
 
 } // namespace blink