Remove TODOs that are out of date.

device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc

 // 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 "device/bluetooth/bluetooth_remote_gatt_characteristic_android.h"
 
 #include <memory>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
@@ skipping 147 matching lines @@
           base::android::ToJavaByteArray(env, new_value))) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(error_callback,
                               BluetoothRemoteGattService::GATT_ERROR_FAILED));
     return;
   }
 
   write_pending_ = true;
   write_callback_ = callback;
   write_error_callback_ = error_callback;
+  write_value_ = new_value;
 }
 
 void BluetoothRemoteGattCharacteristicAndroid::OnChanged(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller,
     const JavaParamRef<jbyteArray>& value) {
   base::android::JavaByteArrayToByteVector(env, value, &value_);
   adapter_->NotifyGattCharacteristicValueChanged(this, value_);
 }
 
@@ skipping 28 matching lines @@
   write_pending_ = false;
 
   // Clear callbacks before calling to avoid reentrancy issues.
   base::Closure write_callback = write_callback_;
   ErrorCallback write_error_callback = write_error_callback_;
   write_callback_.Reset();
   write_error_callback_.Reset();
 
   if (status == 0  // android.bluetooth.BluetoothGatt.GATT_SUCCESS
       && !write_callback.is_null()) {
+    value_ = write_value_;
+    adapter_->NotifyGattCharacteristicValueChanged(this, value_);

[ortuno, 2016/08/29 16:02:53]

I don't think we want to do this, see http://crbug.com/614534#c1

[tommyt, 2016/08/29 17:44:36]

If we don't want to do this, what is the intention of the two TODOs that I've
removed with this change? Are they no longer relevant/wanted? If so, I can
upload a patch that only removes the TODOs and makes no other change to the
code.

[ortuno, 2016/08/29 18:05:23]

I believe the first one is out of date. The patch that added that TODO didn't
call NotifyGattCharacteristicValueChanged once notifications were enabled:
http://crrev.com/1422463012

Regarding this TODO. I would just remove it. The current implementation matches
the bluez and mac implementations so there isn't really anything left to do. We
have http://crbug.com/614534 remind us to decide if we want to change the API to
update value on writes. scheib: WDYT?

[scheib, 2016/08/29 22:36:39]

I agree, let's not update the value cache. tommyt, would you take a look at
https://bugs.chromium.org/p/chromium/issues/detail?id=614534#c2 to update the
comments and parameter name as well to clean up the confusion? Perhaps even
explicitly call out that write doesn't cache the value?

     write_callback.Run();
-    // TODO(https://crbug.com/545682): Call GattCharacteristicValueChanged.
   } else if (!write_error_callback.is_null()) {
     write_error_callback.Run(
         BluetoothRemoteGattServiceAndroid::GetGattErrorCode(status));
   }
 }
 
 void BluetoothRemoteGattCharacteristicAndroid::CreateGattRemoteDescriptor(
     JNIEnv* env,
     const JavaParamRef<jobject>& caller,
     const JavaParamRef<jstring>& instanceId,
@@ skipping 62 matching lines @@
 void BluetoothRemoteGattCharacteristicAndroid::EnsureDescriptorsCreated()
     const {
   if (!descriptors_.empty())
     return;
 
   Java_ChromeBluetoothRemoteGattCharacteristic_createDescriptors(
       AttachCurrentThread(), j_characteristic_);
 }
 
 }  // namespace device