Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(24)

Issue 2506163002: Increase Web Bluetooth device scanning duration (Closed)

Created:
2 years, 6 months ago by juncai
Modified:
2 years, 6 months ago
CC:
ortuno, asvitkine+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, jam, ortuno+watch_chromium.org, scheib+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Increase Web Bluetooth device scanning duration This CL increases the Web Bluetooth device scanning duration from 10 seconds to 60 seconds. Also added UMA for the event that the re-scan link is pressed and the duration to find at lease one Bluetooth device. BUG=419413 Committed: https://crrev.com/acdbb3c267d8d4ea19d0faf4ef34b5179f3c58fa Cr-Commit-Position: refs/heads/master@{#433074}

Patch Set 1 : increase Web Bluetooth device scanning duration #

Total comments: 4

Patch Set 2 : address comments #

Total comments: 4

Patch Set 3 : address comments #

Total comments: 6

Patch Set 4 : updated comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -6 lines) Patch
M content/browser/bluetooth/bluetooth_device_chooser_controller.h View 1 2 chunks +4 lines, -0 lines 0 comments Download
M content/browser/bluetooth/bluetooth_device_chooser_controller.cc View 1 2 8 chunks +25 lines, -5 lines 0 comments Download
M content/browser/bluetooth/bluetooth_metrics.h View 1 chunk +1 line, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 3 chunks +15 lines, -1 line 0 comments Download

Messages

Total messages: 34 (19 generated)
juncai
haraken@chromium.org: Please review changes in //tools/metrics/histograms/histograms.xml scheib@chromium.org: Please review changes in //chrome/browser/ui/bluetooth/ //content/browser/bluetooth/
2 years, 6 months ago (2016-11-16 22:16:49 UTC) #5
haraken
> mailto:haraken@chromium.org: Please review changes in > //tools/metrics/histograms/histograms.xml (I'm allowed to review UseCounter changes only.) ...
2 years, 6 months ago (2016-11-17 04:55:54 UTC) #9
scheib
https://codereview.chromium.org/2506163002/diff/1/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right): https://codereview.chromium.org/2506163002/diff/1/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc#newcode36 chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:36: UMA_HISTOGRAM_MEDIUM_TIMES("Bluetooth.Web.RequestDevice.ScanningDuration", "UMA_HISTOGRAM_LONG_TIMES" instead I think. We've discussed possibly going ...
2 years, 6 months ago (2016-11-17 05:19:59 UTC) #10
juncai
https://codereview.chromium.org/2506163002/diff/1/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right): https://codereview.chromium.org/2506163002/diff/1/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc#newcode36 chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:36: UMA_HISTOGRAM_MEDIUM_TIMES("Bluetooth.Web.RequestDevice.ScanningDuration", On 2016/11/17 05:19:59, scheib wrote: > "UMA_HISTOGRAM_LONG_TIMES" instead ...
2 years, 6 months ago (2016-11-17 21:15:30 UTC) #13
scheib
Need to record the scan has stopped when we go idle (and reset the start ...
2 years, 6 months ago (2016-11-17 22:23:06 UTC) #14
juncai
https://codereview.chromium.org/2506163002/diff/20001/content/browser/bluetooth/bluetooth_device_chooser_controller.cc File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right): https://codereview.chromium.org/2506163002/diff/20001/content/browser/bluetooth/bluetooth_device_chooser_controller.cc#newcode241 content/browser/bluetooth/bluetooth_device_chooser_controller.cc:241: scanning_start_time_.reset(); On 2016/11/17 22:23:06, scheib wrote: > Why do ...
2 years, 6 months ago (2016-11-17 22:44:42 UTC) #16
scheib
LGTM
2 years, 6 months ago (2016-11-18 00:04:18 UTC) #18
Ilya Sherman
metrics lgtm
2 years, 6 months ago (2016-11-18 00:04:53 UTC) #19
ortuno
drive-by https://codereview.chromium.org/2506163002/diff/40001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2506163002/diff/40001/tools/metrics/histograms/histograms.xml#newcode4828 tools/metrics/histograms/histograms.xml:4828: + Records how long it takes for finding ...
2 years, 6 months ago (2016-11-18 00:06:52 UTC) #21
scheib
Oops, I overlooked: https://codereview.chromium.org/2506163002/diff/40001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2506163002/diff/40001/tools/metrics/histograms/histograms.xml#newcode4828 tools/metrics/histograms/histograms.xml:4828: + Records how long it takes ...
2 years, 6 months ago (2016-11-18 00:07:30 UTC) #22
juncai
https://codereview.chromium.org/2506163002/diff/40001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2506163002/diff/40001/tools/metrics/histograms/histograms.xml#newcode4828 tools/metrics/histograms/histograms.xml:4828: + Records how long it takes for finding at ...
2 years, 6 months ago (2016-11-18 00:30:02 UTC) #25
scheib
Looks even gooder to me now
2 years, 6 months ago (2016-11-18 00:38:22 UTC) #29
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/2506163002/60001
2 years, 6 months ago (2016-11-18 00:38:42 UTC) #30
commit-bot: I haz the power
Committed patchset #4 (id:60001)
2 years, 6 months ago (2016-11-18 03:48:29 UTC) #32
commit-bot: I haz the power
2 years, 6 months ago (2016-11-18 03:51:33 UTC) #34
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/acdbb3c267d8d4ea19d0faf4ef34b5179f3c58fa
Cr-Commit-Position: refs/heads/master@{#433074}

Powered by Google App Engine
This is Rietveld 408576698