Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2751343003:
Add Intent Filter to listen for BLE state change (Closed)
DescriptionAdd Intent Filter to listen for BLE state change
This small change adds an intent listener to correctly handle when
bluetooth is turned off mid-broadcasting.
BUG=685856
Review-Url: https://codereview.chromium.org/2751343003
Cr-Commit-Position: refs/heads/master@{#457874}
Committed: https://chromium.googlesource.com/chromium/src/+/e59c7d48269c8900a94b9158680ca67a02c5e418
Patch Set 1 #
Total comments: 2
Patch Set 2 : Dans Nits 1 #
Total comments: 1
Patch Set 3 : Dans nits 2 #
Messages
Total messages: 17 (8 generated)
Description was changed from ========== Add Intent Filter to listen for BLE state change This small change adds an intent listener to correctly handle when bluetooth is turned off mid-broadcasting. BUG=685856 ========== to ========== Add Intent Filter to listen for BLE state change This small change adds an intent listener to correctly handle when bluetooth is turned off mid-broadcasting. BUG=685856 ==========
iankc@google.com changed reviewers: + cco3@chromium.org, mattreynolds@google.com
Real small change.
lgtm
mattreynolds@chromium.org changed reviewers: + mattreynolds@chromium.org
lgtm
iankc@google.com changed reviewers: + dfalcantara@chromium.org
Hey Dan, Here is a real small CL for a sharing featured called Physical Web Sharing. We're using the phone as a Bluetooth Low Energy Beacon and broadcasting a URL. The purpose of this change is to gracefully stop the broadcasting service when the Bluetooth State changes to off. We're using a broadcast receiver for it.
https://codereview.chromium.org/2751343003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java (right): https://codereview.chromium.org/2751343003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java:71: return; Take the return out, make the next thing an else if? The Intent should only ever have one action, but the IntentFilter can catch multiple.
https://codereview.chromium.org/2751343003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java (right): https://codereview.chromium.org/2751343003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java:71: return; On 2017/03/17 18:53:46, dfalcantara (load balance plz) wrote: > Take the return out, make the next thing an else if? The Intent should only > ever have one action, but the IntentFilter can catch multiple. Done.
lgtm % comments https://codereview.chromium.org/2751343003/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java (right): https://codereview.chromium.org/2751343003/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java:67: int state = intent.getIntExtra(BluetoothAdapter.EXTRA_STATE, -1); Sorry missed these the first time around: 1) I'm worried about -1 being used here, but they don't seem to have defined any error states that you can use in the BluetoothAdapter. Can you pull out the -1 as a constant? 2) Use IntentUtils.safeGetIntExtra() instead. We've gotten crashes from Facebook sending us unparcelable Intents.
The CQ bit was checked by iankc@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from cco3@chromium.org, mattreynolds@chromium.org, dfalcantara@chromium.org Link to the patchset: https://codereview.chromium.org/2751343003/#ps40001 (title: "Dans nits 2")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489780078353360,
"parent_rev": "8055a28acd2bf29534913610369be7ae1b0f894f", "commit_rev":
"e59c7d48269c8900a94b9158680ca67a02c5e418"}
Message was sent while issue was closed.
Description was changed from ========== Add Intent Filter to listen for BLE state change This small change adds an intent listener to correctly handle when bluetooth is turned off mid-broadcasting. BUG=685856 ========== to ========== Add Intent Filter to listen for BLE state change This small change adds an intent listener to correctly handle when bluetooth is turned off mid-broadcasting. BUG=685856 Review-Url: https://codereview.chromium.org/2751343003 Cr-Commit-Position: refs/heads/master@{#457874} Committed: https://chromium.googlesource.com/chromium/src/+/e59c7d48269c8900a94b9158680c... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/e59c7d48269c8900a94b9158680c... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
