Re: bluetooth: Move First Device Chooser to tests (issue 2110813002 by ortuno@chromium.org)

0 views
Skip to first unread message

ort...@chromium.org

unread,
Jun 29, 2016, 1:58:12 PM6/29/16
to pe...@chromium.org, pal...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org
Reviewers: Peter Beverloo, palmer
CL: https://codereview.chromium.org/2110813002/

Message:
peter: PTAL at content/shell
palmer: PTAT at t/W/p/p/m/bluetooth/web_bluetooth.mojom

Description:
bluetooth: Move First Device Chooser to tests

Now that we have a chooser on all supported platforms we can move the
First Device Chooser fallback to test, the only place where it's needed.

BUG=524780

Base URL: https://chromium.googlesource.com/chromium/src.git@my-origin

Affected files (+64, -103 lines):
M content/browser/bluetooth/bluetooth_device_chooser_controller.cc
D content/browser/bluetooth/first_device_bluetooth_chooser.h
D content/browser/bluetooth/first_device_bluetooth_chooser.cc
M content/content_browser.gypi
M content/content_shell.gypi
M content/shell/BUILD.gn
M content/shell/browser/layout_test/blink_test_controller.cc
A content/shell/browser/layout_test/layout_test_first_device_bluetooth_chooser.h
A + content/shell/browser/layout_test/layout_test_first_device_bluetooth_chooser.cc
M third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp
M third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom


pe...@chromium.org

unread,
Jun 29, 2016, 2:45:10 PM6/29/16
to ort...@chromium.org, pal...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org

pal...@chromium.org

unread,
Jun 29, 2016, 8:31:32 PM6/29/16
to ort...@chromium.org, pe...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org

ort...@chromium.org

unread,
Jun 29, 2016, 10:25:53 PM6/29/16
to pal...@chromium.org, pe...@chromium.org, scheib+...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org

sch...@chromium.org

unread,
Jun 30, 2016, 6:18:11 PM6/30/16
to ort...@chromium.org, pal...@chromium.org, pe...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org
LGTM


https://codereview.chromium.org/2110813002/diff/1/content/shell/browser/layout_test/blink_test_controller.cc
File content/shell/browser/layout_test/blink_test_controller.cc (right):

https://codereview.chromium.org/2110813002/diff/1/content/shell/browser/layout_test/blink_test_controller.cc#newcode396
content/shell/browser/layout_test/blink_test_controller.cc:396:
std::unique_ptr<BluetoothChooser>
BlinkTestController::RunBluetoothChooser(
Out of scope of this change, but why isn't this 'GetBluetoothChooser'?

https://codereview.chromium.org/2110813002/

ort...@chromium.org

unread,
Jun 30, 2016, 6:32:37 PM6/30/16
to pal...@chromium.org, pe...@chromium.org, scheib+...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org

https://codereview.chromium.org/2110813002/diff/1/content/shell/browser/layout_test/blink_test_controller.cc
File content/shell/browser/layout_test/blink_test_controller.cc (right):

https://codereview.chromium.org/2110813002/diff/1/content/shell/browser/layout_test/blink_test_controller.cc#newcode396
content/shell/browser/layout_test/blink_test_controller.cc:396:
std::unique_ptr<BluetoothChooser>
BlinkTestController::RunBluetoothChooser(
On 2016/06/30 at 22:18:11, scheib wrote:
> Out of scope of this change, but why isn't this 'GetBluetoothChooser'?

Not sure... It was added a long time ago and no one ever asked jyasskin
to change it: https://codereview.chromium.org/1286063002

https://codereview.chromium.org/2110813002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Jun 30, 2016, 6:33:08 PM6/30/16
to ort...@chromium.org, pal...@chromium.org, pe...@chromium.org, scheib+...@chromium.org, commi...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Jun 30, 2016, 8:07:44 PM6/30/16
to ort...@chromium.org, pal...@chromium.org, pe...@chromium.org, scheib+...@chromium.org, commi...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org
Committed patchset #1 (id:1)

https://codereview.chromium.org/2110813002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Jun 30, 2016, 8:08:01 PM6/30/16
to ort...@chromium.org, pal...@chromium.org, pe...@chromium.org, scheib+...@chromium.org, commi...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Jun 30, 2016, 8:12:23 PM6/30/16
to ort...@chromium.org, pal...@chromium.org, pe...@chromium.org, scheib+...@chromium.org, commi...@chromium.org, a...@chromium.org, aba...@chromium.org, ben+...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, chromium...@chromium.org, da...@chromium.org, dari...@chromium.org, dglazko...@chromium.org, har...@chromium.org, j...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, mlamouri+wa...@chromium.org, ortuno...@chromium.org, pe...@chromium.org, qsr+...@chromium.org, scheib...@chromium.org, viettrung...@chromium.org, yzshen...@chromium.org
Patchset 1 (id:??) landed as
https://crrev.com/1045554135e81958e4340f9a2eb9fb088618e6d5
Cr-Commit-Position: refs/heads/master@{#403358}

https://codereview.chromium.org/2110813002/
Reply all
Reply to author
Forward
0 new messages