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

Issue 767500 link

Starred by 0 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Retry BLE advertisement registration/unregistration until it succeeds

Project Member Reported by khorimoto@chromium.org, Sep 21 2017

Issue description

With the upcoming fixes for the advertisement registration/unregistration failures, we can expect some attempts to fail. We should handle these cases by retrying them until they succeed.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 22 2017

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

commit 8b97e93abe1af2658a0443285a211ba3fda707ee
Author: Kyle Horimoto <khorimoto@google.com>
Date: Fri Sep 22 03:38:26 2017

[CrOS Tether] Handle failed advertisement registration/unregistration.

Now, when RegisterAdvertisement() or Unregister() fail, the operation is
retried until it succeeds. This fixes several related issues which
result from getting stuck thinking that an advertisement is registered
or unregistered when it actually is not.

Bug:  767500 , 672263
Change-Id: I73886e104dc64fbc3f307e3c834de873ddf85914
Reviewed-on: https://chromium-review.googlesource.com/677715
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503626}
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_advertisement_device_queue_unittest.cc
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_advertiser.cc
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_advertiser.h
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_advertiser_unittest.cc
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_connection_manager.cc
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_connection_manager_unittest.cc
[delete] https://crrev.com/4780f1dd5e9596893f597d18e12f208792a20127/chromeos/components/tether/ble_constants.cc
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_constants.h
[modify] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/ble_scanner_unittest.cc
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/error_tolerant_ble_advertisement.cc
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/error_tolerant_ble_advertisement.h
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/error_tolerant_ble_advertisement_impl.cc
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/error_tolerant_ble_advertisement_impl.h
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/error_tolerant_ble_advertisement_impl_unittest.cc
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/fake_error_tolerant_ble_advertisement.cc
[add] https://crrev.com/8b97e93abe1af2658a0443285a211ba3fda707ee/chromeos/components/tether/fake_error_tolerant_ble_advertisement.h

Labels: Merge-Request-61 Merge-Request-62
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 22 2017

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

commit d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2
Author: Kyle Horimoto <khorimoto@google.com>
Date: Fri Sep 22 04:38:13 2017

[CrOS Tether] Shut down BleAdvertiser asynchronously if necessary.

This ensures that if BleAdvertiser is in the process of unregistering an
advertisement when the Tether component shuts down, we give it
sufficient time to clean up its advertisements.

This works around a Bluetooth bug which sometimes causes advertisement
unregistration to fail.

Bug:  767500 , 672263
Change-Id: I567be109ad481923259ff39b799a63b00f8ddee7
Reviewed-on: https://chromium-review.googlesource.com/678255
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503659}
[modify] https://crrev.com/d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2/chromeos/components/tether/ble_advertiser.cc
[modify] https://crrev.com/d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2/chromeos/components/tether/ble_advertiser.h
[modify] https://crrev.com/d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2/chromeos/components/tether/ble_advertiser_unittest.cc
[modify] https://crrev.com/d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2/chromeos/components/tether/error_tolerant_ble_advertisement_impl.cc
[modify] https://crrev.com/d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2/chromeos/components/tether/initializer_impl.h

Comment 4 by ketakid@google.com, Sep 23 2017

Labels: -Merge-Request-61 -Merge-Request-62 Merge-Approved-62 Merge-Approved-61
Approving merge to M61 and M62.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 23 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe3480a41b582420ceeff22e33b604b16bd4b537

commit fe3480a41b582420ceeff22e33b604b16bd4b537
Author: Kyle Horimoto <khorimoto@google.com>
Date: Sat Sep 23 00:57:18 2017

[CrOS Tether] Handle failed advertisement registration/unregistration.

Now, when RegisterAdvertisement() or Unregister() fail, the operation is
retried until it succeeds. This fixes several related issues which
result from getting stuck thinking that an advertisement is registered
or unregistered when it actually is not.

TBR=khorimoto@google.com

(cherry picked from commit 8b97e93abe1af2658a0443285a211ba3fda707ee)

Bug:  767500 , 672263
Change-Id: I73886e104dc64fbc3f307e3c834de873ddf85914
Reviewed-on: https://chromium-review.googlesource.com/677715
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#503626}
Reviewed-on: https://chromium-review.googlesource.com/679915
Cr-Commit-Position: refs/branch-heads/3202@{#412}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_advertisement_device_queue_unittest.cc
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_advertiser.cc
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_advertiser.h
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_advertiser_unittest.cc
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_connection_manager.cc
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_connection_manager_unittest.cc
[delete] https://crrev.com/fe0b4705e061058a996fcd2616325da73bca3959/chromeos/components/tether/ble_constants.cc
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_constants.h
[modify] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/ble_scanner_unittest.cc
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/error_tolerant_ble_advertisement.cc
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/error_tolerant_ble_advertisement.h
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/error_tolerant_ble_advertisement_impl.cc
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/error_tolerant_ble_advertisement_impl.h
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/error_tolerant_ble_advertisement_impl_unittest.cc
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/fake_error_tolerant_ble_advertisement.cc
[add] https://crrev.com/fe3480a41b582420ceeff22e33b604b16bd4b537/chromeos/components/tether/fake_error_tolerant_ble_advertisement.h

Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 23 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba4906eccb20bf6131892b8d727b4993d1c7c01e

commit ba4906eccb20bf6131892b8d727b4993d1c7c01e
Author: Kyle Horimoto <khorimoto@google.com>
Date: Sat Sep 23 01:00:41 2017

[CrOS Tether] Handle failed advertisement registration/unregistration.

Now, when RegisterAdvertisement() or Unregister() fail, the operation is
retried until it succeeds. This fixes several related issues which
result from getting stuck thinking that an advertisement is registered
or unregistered when it actually is not.

TBR=khorimoto@google.com

(cherry picked from commit 8b97e93abe1af2658a0443285a211ba3fda707ee)

Bug:  767500 , 672263
Change-Id: I73886e104dc64fbc3f307e3c834de873ddf85914
Reviewed-on: https://chromium-review.googlesource.com/677715
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#503626}
Reviewed-on: https://chromium-review.googlesource.com/680022
Cr-Commit-Position: refs/branch-heads/3163@{#1268}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_advertisement_device_queue_unittest.cc
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_advertiser.cc
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_advertiser.h
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_advertiser_unittest.cc
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_connection_manager.cc
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_connection_manager_unittest.cc
[delete] https://crrev.com/bf5021550bdb73b43bbc15c788492dbdc6b3daf6/chromeos/components/tether/ble_constants.cc
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_constants.h
[modify] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/ble_scanner_unittest.cc
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/error_tolerant_ble_advertisement.cc
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/error_tolerant_ble_advertisement.h
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/error_tolerant_ble_advertisement_impl.cc
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/error_tolerant_ble_advertisement_impl.h
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/error_tolerant_ble_advertisement_impl_unittest.cc
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/fake_error_tolerant_ble_advertisement.cc
[add] https://crrev.com/ba4906eccb20bf6131892b8d727b4993d1c7c01e/chromeos/components/tether/fake_error_tolerant_ble_advertisement.h

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 23 2017

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

commit be5b64b4a3bd8037699f4d9981d584e6881d0733
Author: Kyle Horimoto <khorimoto@google.com>
Date: Sat Sep 23 19:14:32 2017

[CrOS Tether] Shut down BleAdvertiser asynchronously if necessary.

This ensures that if BleAdvertiser is in the process of unregistering an
advertisement when the Tether component shuts down, we give it
sufficient time to clean up its advertisements.

This works around a Bluetooth bug which sometimes causes advertisement
unregistration to fail.

TBR=khorimoto@google.com

(cherry picked from commit d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2)

Bug:  767500 , 672263
Change-Id: I567be109ad481923259ff39b799a63b00f8ddee7
Reviewed-on: https://chromium-review.googlesource.com/678255
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#503659}
Reviewed-on: https://chromium-review.googlesource.com/679957
Cr-Commit-Position: refs/branch-heads/3202@{#416}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/be5b64b4a3bd8037699f4d9981d584e6881d0733/chromeos/components/tether/ble_advertiser.cc
[modify] https://crrev.com/be5b64b4a3bd8037699f4d9981d584e6881d0733/chromeos/components/tether/ble_advertiser.h
[modify] https://crrev.com/be5b64b4a3bd8037699f4d9981d584e6881d0733/chromeos/components/tether/ble_advertiser_unittest.cc
[modify] https://crrev.com/be5b64b4a3bd8037699f4d9981d584e6881d0733/chromeos/components/tether/error_tolerant_ble_advertisement_impl.cc
[modify] https://crrev.com/be5b64b4a3bd8037699f4d9981d584e6881d0733/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/be5b64b4a3bd8037699f4d9981d584e6881d0733/chromeos/components/tether/initializer_impl.h

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 23 2017

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

commit 848410fb6870f6831f3df31cc682e814b32d8102
Author: Kyle Horimoto <khorimoto@google.com>
Date: Sat Sep 23 19:15:57 2017

[CrOS Tether] Shut down BleAdvertiser asynchronously if necessary.

This ensures that if BleAdvertiser is in the process of unregistering an
advertisement when the Tether component shuts down, we give it
sufficient time to clean up its advertisements.

This works around a Bluetooth bug which sometimes causes advertisement
unregistration to fail.

TBR=khorimoto@google.com

(cherry picked from commit d844d6e8025631f66b9e48b1bb2f6210d1f1b8b2)

Bug:  767500 , 672263
Change-Id: I567be109ad481923259ff39b799a63b00f8ddee7
Reviewed-on: https://chromium-review.googlesource.com/678255
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#503659}
Reviewed-on: https://chromium-review.googlesource.com/679958
Cr-Commit-Position: refs/branch-heads/3163@{#1273}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/848410fb6870f6831f3df31cc682e814b32d8102/chromeos/components/tether/ble_advertiser.cc
[modify] https://crrev.com/848410fb6870f6831f3df31cc682e814b32d8102/chromeos/components/tether/ble_advertiser.h
[modify] https://crrev.com/848410fb6870f6831f3df31cc682e814b32d8102/chromeos/components/tether/ble_advertiser_unittest.cc
[modify] https://crrev.com/848410fb6870f6831f3df31cc682e814b32d8102/chromeos/components/tether/error_tolerant_ble_advertisement_impl.cc
[modify] https://crrev.com/848410fb6870f6831f3df31cc682e814b32d8102/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/848410fb6870f6831f3df31cc682e814b32d8102/chromeos/components/tether/initializer_impl.h

Comment 10 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 11 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Status: Verified (was: Fixed)

Sign in to add a comment