Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(48)

Issue 2707823002: bluetooth: Save characteristic value when notification arrives (Closed)

Created:
3 years, 10 months ago by ortuno
Modified:
3 years, 9 months ago
Reviewers:
scheib
CC:
agrieve+watch_chromium.org, chromium-reviews, ortuno+watch_chromium.org, scheib+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

bluetooth: Save characteristic value when notification arrives "Once a notification is arrived, work is posted to another thread which then reads the latest characteristic value. If multiple notifications arrive fast, this could result in race conditions where all the onCharacteristicChanged callbacks are first triggered, then the three tasks posted are executed which all extract the latest notified value instead of the individual values." -- emil.len...@gmail.com [1] Post a task when sending GATT events http://crrev.com/2708513002 [2.1] Close and null out BluetoothGatt on the UI Thread http://crrev.com/2702163002 [2.2] Copy characteristic's value before posting task. (This patch). BUG=647673 Review-Url: https://codereview.chromium.org/2707823002 Cr-Commit-Position: refs/heads/master@{#455037} Committed: https://chromium.googlesource.com/chromium/src/+/afc9c240ef4f83bf23dfeec704f231f08c5c8121

Patch Set 1 #

Patch Set 2 : Merge branch 'bluetooth-notifications-crash' into bluetooth-and-notifications-wrong-value #

Patch Set 3 : fix test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -4 lines) Patch
M device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java View 2 chunks +4 lines, -1 line 0 comments Download
M device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java View 1 chunk +2 lines, -3 lines 0 comments Download
M device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
M device/bluetooth/test/test_bluetooth_adapter_observer.h View 2 chunks +6 lines, -0 lines 0 comments Download
M device/bluetooth/test/test_bluetooth_adapter_observer.cc View 2 chunks +2 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 22 (17 generated)
ortuno
jyasskin: PTAL
3 years, 10 months ago (2017-02-21 10:45:15 UTC) #6
ortuno
scheib: PTAL
3 years, 9 months ago (2017-02-27 23:06:57 UTC) #8
scheib
lgtm
3 years, 9 months ago (2017-03-04 18:54:35 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2707823002/40001
3 years, 9 months ago (2017-03-07 05:12:16 UTC) #19
commit-bot: I haz the power
3 years, 9 months ago (2017-03-07 05:22:57 UTC) #22
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/afc9c240ef4f83bf23dfeec704f2...

Powered by Google App Engine
This is Rietveld 408576698