New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 632476 link

Starred by 2 users
Status: Verified
Owner:
puthik@chromium.org
Closed: Sep 2016
Cc:
snanda@chromium.org
r...@chromium.org
█ ortuno@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

arc: Arc shouldn't use IsNotifying to decide whether or not to subscribe to notifications.

Project Member Reported by ortuno@chromium.org, Jul 28 2016 
If Arc++ needs to keep receiving notifications it needs to unconditionally acquire a NotifySession. The Bluetooth API uses a ref-counting model to write to the descriptor. If other clients drop their NotifySessions, notifications will stop.

For example:

1. A website requests to start notifications.
2. The counter is increased to 1 and we write to the CCC descriptor.
3. Arc checks IsNotifying(), which returns true so it doesn't acquire a notification.
4. User navigates away from websites, therefore dropping it's NotifySession.
5. We write to the CCC descriptor to stop notifications.


Please fix, this is blocking refactoring of the device APIs and web bluetooth.
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 29 2016 

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

commit 3a796de6b43352fb756aa1c9c24a8bc5fc481553
Author: puthik <puthik@chromium.org>
Date: Fri Jul 29 18:18:56 2016

arc: bluetooth: Always acquire a NotifySession

If Arc++ needs to keep receiving notifications it needs to
unconditionally acquire a NotifySession. The Bluetooth API
uses a ref-counting model to write to the descriptor. If
other clients drop their NotifySessions, notifications will stop.

BUG= 632476 
TEST=Build

Review-Url: https://codereview.chromium.org/2189183002
Cr-Commit-Position: refs/heads/master@{#408692}

[modify] https://crrev.com/3a796de6b43352fb756aa1c9c24a8bc5fc481553/components/arc/bluetooth/arc_bluetooth_bridge.cc

Comment 2 by ortuno@chromium.org, Jul 29 2016

Status: Fixed (was: Assigned)
Thanks!

Comment 3 by puthik@chromium.org, Aug 22 2016

Components: Platform>ARC
Labels: Merge-Request-53 M-53
Status: Started (was: Fixed)
Ask for merge request since this fix the bug for M53

Comment 4 by dimu@chromium.org, Aug 22 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 23 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/300e592e0f2f27aff3fff532d17e3a298adaf7ce

commit 300e592e0f2f27aff3fff532d17e3a298adaf7ce
Author: Rahul Chaturvedi <rkc@google.com>
Date: Tue Aug 23 01:50:30 2016

arc: bluetooth: Always acquire a NotifySession

If Arc++ needs to keep receiving notifications it needs to
unconditionally acquire a NotifySession. The Bluetooth API
uses a ref-counting model to write to the descriptor. If
other clients drop their NotifySessions, notifications will stop.

BUG= 632476 
TEST=Build

Review-Url: https://codereview.chromium.org/2189183002
Cr-Commit-Position: refs/heads/master@{#408692}
(cherry picked from commit 3a796de6b43352fb756aa1c9c24a8bc5fc481553)

Review URL: https://codereview.chromium.org/2270613002 .

Cr-Commit-Position: refs/branch-heads/2785@{#719}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/300e592e0f2f27aff3fff532d17e3a298adaf7ce/components/arc/bluetooth/arc_bluetooth_bridge.cc

Comment 6 by st...@chromium.org, Aug 24 2016

Cc: st...@chromium.org

Comment 7 by st...@chromium.org, Aug 24 2016

Cc: -r...@chromium.org

Comment 8 by puthik@chromium.org, Sep 7 2016

Status: Fixed (was: Started)

Comment 9 by dhadd...@chromium.org, Oct 18 2016

Status: Verified (was: Fixed)

Comment 10 by st...@chromium.org, Mar 3 2017

Cc: r...@chromium.org

Comment 11 by st...@chromium.org, Mar 3 2017

Cc: -st...@chromium.org
►

Sign in to add a comment