Fix getting notified twice after subscribe to notifications and call readValue

device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc

 // Copyright 2016 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 <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <utility>
 #include <vector>
@@ skipping 1083 matching lines @@
   characteristic->WriteRemoteCharacteristic(
       invalid_write_value, base::Bind(&BluetoothGattBlueZTest::SuccessCallback,
                                       base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(4, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED, last_service_error_);
   EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count());
 
   // Issue a read request. A successful read results in a

[ortuno, 2017/03/06 09:15:37]

Fix comment.

[juncai, 2017/03/08 23:20:39]

Done.

   // CharacteristicValueChanged notification.
   characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->GetBodySensorLocationPath()
           .value());
   ASSERT_TRUE(characteristic);
   EXPECT_EQ(
       fake_bluetooth_gatt_characteristic_client_->GetBodySensorLocationPath()
           .value(),
       characteristic->GetIdentifier());
   EXPECT_EQ(kBodySensorLocationUUID, characteristic->GetUUID());
   characteristic->ReadRemoteCharacteristic(
       base::Bind(&BluetoothGattBlueZTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(2, success_callback_count_);
   EXPECT_EQ(4, error_callback_count_);
-  EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
+  EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count());
   EXPECT_TRUE(ValuesEqual(characteristic->GetValue(), last_read_value_));
 
   // Test long-running actions.
   fake_bluetooth_gatt_characteristic_client_->SetExtraProcessing(1);
   characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->GetBodySensorLocationPath()
           .value());
   ASSERT_TRUE(characteristic);
   EXPECT_EQ(
       fake_bluetooth_gatt_characteristic_client_->GetBodySensorLocationPath()
           .value(),
       characteristic->GetIdentifier());
   EXPECT_EQ(kBodySensorLocationUUID, characteristic->GetUUID());
   characteristic->ReadRemoteCharacteristic(
       base::Bind(&BluetoothGattBlueZTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ServiceErrorCallback,
                  base::Unretained(this)));
 
   // Callback counts shouldn't change, this one will be delayed until after
   // tne next one.
   EXPECT_EQ(2, success_callback_count_);
   EXPECT_EQ(4, error_callback_count_);
-  EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
+  EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count());
 
   // Next read should error because IN_PROGRESS
   characteristic->ReadRemoteCharacteristic(
       base::Bind(&BluetoothGattBlueZTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(5, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
             last_service_error_);
 
   // But previous call finished.
   EXPECT_EQ(3, success_callback_count_);
-  EXPECT_EQ(2, observer.gatt_characteristic_value_changed_count());
+  EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());

[ortuno, 2017/03/06 09:15:37]

Please spend some time trying to understand tests before modifying them
otherwise we might break behavior in unintended ways or our implementation might
be buggy.

In this part of the test there are only Read requests so there shouldn't be a
characteristic value changed event. I think the following is happening:

1. We make first Read request which sets pending to true.
2. We make second Read request which sets pending to true.
3. Second read request fails with In Progress errors and sets pending to false.
4. First read request succeeds and notifies of value change since pending was
set to false we dispatch the even to all observers.

[juncai, 2017/03/08 23:20:39]

Done.

   EXPECT_TRUE(ValuesEqual(characteristic->GetValue(), last_read_value_));
   fake_bluetooth_gatt_characteristic_client_->SetExtraProcessing(0);
 
   // Test unauthorized actions.
   fake_bluetooth_gatt_characteristic_client_->SetAuthorized(false);
   characteristic->ReadRemoteCharacteristic(
       base::Bind(&BluetoothGattBlueZTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(3, success_callback_count_);
   EXPECT_EQ(6, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_NOT_AUTHORIZED,
             last_service_error_);
-  EXPECT_EQ(2, observer.gatt_characteristic_value_changed_count());
+  EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
   fake_bluetooth_gatt_characteristic_client_->SetAuthorized(true);
 
   // Test unauthenticated / needs login.
   fake_bluetooth_gatt_characteristic_client_->SetAuthenticated(false);
   characteristic->ReadRemoteCharacteristic(
       base::Bind(&BluetoothGattBlueZTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(3, success_callback_count_);
   EXPECT_EQ(7, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_NOT_PAIRED,
             last_service_error_);
-  EXPECT_EQ(2, observer.gatt_characteristic_value_changed_count());
+  EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
   fake_bluetooth_gatt_characteristic_client_->SetAuthenticated(true);
 }
 
 TEST_F(BluetoothGattBlueZTest, GattCharacteristicProperties) {
   fake_bluetooth_device_client_->CreateDevice(
       dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
       dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
   BluetoothDevice* device =
       adapter_->GetDevice(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress);
   ASSERT_TRUE(device);
@@ skipping 437 matching lines @@
 
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
   EXPECT_TRUE(characteristic->IsNotifying());
   EXPECT_EQ(1U, update_sessions_.size());
   EXPECT_TRUE(update_sessions_[0]->IsActive());
 }
 
 }  // namespace bluez