The issue is that BluetoothRemoteGATTCharacteristic is not an ActiveScriptWrappable and thus, when a V8 garbage collection is run the V8 wrapper around this object and its attached event listeners may be cleaned up. The solution is to make this object ActiveScriptWrappable and have it report pending activity whenever notifications are active.

This pattern was copied from the Sensor object, which returns false from HasPendingActivity when it is in the idle state. This may not be correct because as I understand it this means that if you call stopNotifications() and let the object go out of scope then if you get it back later the event listeners will be gone. Given that the Web Bluetooth spec says that we're supposed to be returning the same characteristic objects that could mean that the event listeners should be preserved.

Could issue 826716 be related to this?

I was thinking the same thing but when this happens the event listener gets removed entirely so it should stop getting notifications completely rather than just getting delayed.

It's really weird that the v8 wrapper and listener get gc'ed even thought the C++ object is still alive. What happens if the wrapper gets gc'ed and the user retrieves the characteristic again? Would we just create another wrapper?

Would it make sense to return true from HasPendingActivity until Dispose is called? That way the lifetime of the v8 wrapper is attached to that of the C++ object.

#5: :( I was hoping this would also fix that one

I chatted about this with jochen@ and this behavior is expected. Construction and destruction of the wrappers shouldn't be visible to script but in this case it is. ActiveScriptWrappable is the answer when we need to keep something alive longer than script owns it.

After thinking about it over the weekend I decided that my concern in comment #2 is correct and we should return true from HasPendingActivity() as long as there are registered event listeners. Returning true until Dispose() is called is unnecessary. Once the last reference to the object is dropped and the event listeners are removed script has no way to observe the lifetime of the object and it can be freed.

sgtm

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22ea6534637b29eeac29d6020d2b4ba1acea43d4

commit 22ea6534637b29eeac29d6020d2b4ba1acea43d4
Author: Reilly Grant <reillyg@chromium.org>
Date: Thu May 31 01:40:32 2018

Make BluetoothRemoteGATTCharacteristic an ActiveScriptWrappable

This patch makes BluetoothRemoteGATTCharacteristic implement
ActiveScriptWrappable so that when event listeners are registered the V8
wrappers will not be destroyed. This prevents event listeners from being
silently destroyed if script does not maintain a reference to the
characteristic object.

Bug:  846977 
Change-Id: I235e55a1fef41f81b45b1b281036ce24bc5d0279
Reviewed-on: https://chromium-review.googlesource.com/1079629
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563113}
[add] https://crrev.com/22ea6534637b29eeac29d6020d2b4ba1acea43d4/third_party/WebKit/LayoutTests/bluetooth/characteristic/notifications/gc-with-event-listener.html
[modify] https://crrev.com/22ea6534637b29eeac29d6020d2b4ba1acea43d4/third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_characteristic.cc
[modify] https://crrev.com/22ea6534637b29eeac29d6020d2b4ba1acea43d4/third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_characteristic.h
[modify] https://crrev.com/22ea6534637b29eeac29d6020d2b4ba1acea43d4/third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_characteristic.idl