Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(315)

Issue 2437943002: Ship WebBluetooth out of origin trial (Closed)

Created:
4 years, 2 months ago by juncai
Modified:
4 years, 1 month ago
CC:
ortuno, asvitkine+watch_chromium.org, blink-reviews, blink-reviews-bindings_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, jochen+watch_chromium.org, Jeffrey Yasskin, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, ortuno+watch_chromium.org, Peter Beverloo, scheib+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/discussion This CL does the following to move WebBluetooth out of origin trial: 1. Remove the --enable-web-bluetooth flag. 2. Use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. Remove the origin trial for WebBluetooth. 4. Two files are added: third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt Because currently Web Bluetooth is not supported on Linux or Windows, and in those platforms the Web Bluetooth bindings are not exposed. So in order to pass the Layout tests on Linux and Windows, the new added expected file: "global-interface-listing-expected.txt" doesn't contain Web Bluetooth interfaces. TEST=Manual testing required: In each scenario below, to "Execute Test": For each URL https://googlechrome.github.io/samples/web-bluetooth/device-information-characteristics.html https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Perform the following: Open Chrome Developer Tools Console Execute: "navigator.bluetooth && navigator.bluetooth.requestDevice({filters: [{services: ['human_interface_device']}]})" "WebBluetooth works" when this response is returned: "Uncaught (in promise) DOMException: requestDevice() called with a filter containing a blacklisted UUID." (the request for a blacklisted UUID is correctly rejected by the browser). 1. Run chrome without any flag: ChromeOS, Android, MacOS: Execute Test and expect: WebBluetooth works. Linux, Windows: Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: --enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Committed: https://crrev.com/3ad09e105c07e4b22423866f4d16ed5adfe6db45 Cr-Commit-Position: refs/heads/master@{#428265}

Patch Set 1 : ship WebBluetooth out of origin trial #

Patch Set 2 : fix browser tests #

Patch Set 3 : fixed layout tests #

Patch Set 4 : fixed layout tests on Linux and Windows #

Total comments: 8

Patch Set 5 : address comments #

Total comments: 12

Patch Set 6 : address comments #

Patch Set 7 : updated expected file #

Patch Set 8 : add OWNERS #

Patch Set 9 : rebase #

Patch Set 10 : rebase and remove Bluetooth uuids #

Total comments: 4

Patch Set 11 : address comments #

Total comments: 2

Patch Set 12 : address more comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -96 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/policy/policy_browsertest.cc View 1 2 3 4 5 6 1 chunk +6 lines, -3 lines 0 comments Download
M chrome/browser/web_bluetooth_browsertest.cc View 1 2 3 4 1 chunk +6 lines, -3 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -11 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -1 line 0 comments Download
M content/child/runtime_features.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +6 lines, -3 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M content/public/common/content_switches.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -3 lines 0 comments Download
M content/shell/app/shell_main_delegate.cc View 1 chunk +0 lines, -2 lines 0 comments Download
A + third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/OWNERS View 1 2 3 4 5 6 7 0 chunks +-1 lines, --1 lines 0 comments Download
A + third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 6 0 chunks +-1 lines, --1 lines 0 comments Download
A + third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/OWNERS View 1 2 3 4 5 6 7 0 chunks +-1 lines, --1 lines 0 comments Download
A + third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 6 0 chunks +-1 lines, --1 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 2 chunks +58 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/ConditionalFeaturesForModules.cpp View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/Bluetooth.h View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp View 1 2 2 chunks +0 lines, -37 lines 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/Bluetooth.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothCharacteristicProperties.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.idl View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothUUID.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/NavigatorBluetooth.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/web/ChromeClientImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -5 lines 0 comments Download

Messages

Total messages: 94 (68 generated)
juncai
Please take a look.
4 years, 2 months ago (2016-10-20 17:47:06 UTC) #14
scheib
I modified the description some https://www.diffchecker.com/n5MKZUZN Also, add to the description an explanation that third_party/WebKit/LayoutTests/platform/win ...
4 years, 2 months ago (2016-10-21 01:02:10 UTC) #23
juncai
https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/policy/policy_browsertest.cc#newcode3418 chrome/browser/policy/policy_browsertest.cc:3418: command_line->AppendSwitch( On 2016/10/21 01:02:10, scheib wrote: > Keep a ...
4 years, 2 months ago (2016-10-21 16:57:54 UTC) #30
scheib
LGTM
4 years, 2 months ago (2016-10-21 18:26:46 UTC) #31
juncai
pastarmovj@chromium.org: Please review changes in //chrome/browser/policy/policy_browsertest.cc jam@chromium.org: Please review changes in //content //third_party
4 years, 2 months ago (2016-10-21 21:20:25 UTC) #35
jam
On 2016/10/21 21:20:25, juncai wrote: > mailto:pastarmovj@chromium.org: Please review changes in > //chrome/browser/policy/policy_browsertest.cc > > ...
4 years, 2 months ago (2016-10-21 22:21:21 UTC) #36
scheib
You will need one of API_OWNERS https://cs.chromium.org/chromium/src/third_party/WebKit/API_OWNERS Because of the change to webexposed: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/virtual/stable/webexposed/OWNERS
4 years, 2 months ago (2016-10-21 22:31:07 UTC) #37
juncai
rbyers@chromium.org: Please review changes in //third_party/WebKit
4 years, 2 months ago (2016-10-21 22:41:39 UTC) #39
haraken
bindings/ LGTM
4 years, 2 months ago (2016-10-22 00:55:05 UTC) #40
scheib
juncai, you should also replicate https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/virtual/stable/webexposed/OWNERS into each new subdirectory created under LayoutTests/platform.
4 years, 2 months ago (2016-10-22 16:27:32 UTC) #42
Rick Byers
I reviewed WebKit except WebKit/Source/modules/bluetooth (since I'm not an expert there but scheib@, who is ...
4 years, 1 month ago (2016-10-24 16:38:43 UTC) #43
pastarmovj
policy lgtm.
4 years, 1 month ago (2016-10-25 09:30:05 UTC) #44
juncai
Hi dpranke@, would like to get some thoughts from you about adding: third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt to ...
4 years, 1 month ago (2016-10-26 15:54:56 UTC) #56
Rick Byers
On 2016/10/26 15:54:56, juncai wrote: > Hi dpranke@, would like to get some thoughts from ...
4 years, 1 month ago (2016-10-26 16:32:43 UTC) #57
Rick Byers
https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt#newcode250 third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids On 2016/10/26 15:54:55, juncai wrote: > On ...
4 years, 1 month ago (2016-10-26 17:05:05 UTC) #58
Dirk Pranke
It looks like this baseline gets changed a few times a week these days, and ...
4 years, 1 month ago (2016-10-26 17:06:34 UTC) #59
juncai
https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt#newcode250 third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids On 2016/10/26 17:05:05, Rick Byers wrote: > ...
4 years, 1 month ago (2016-10-26 17:37:38 UTC) #66
Rick Byers
On 2016/10/26 17:37:38, juncai wrote: > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt > File > third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt > (right): > > ...
4 years, 1 month ago (2016-10-26 18:25:04 UTC) #67
juncai
https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt#newcode250 third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids Removed BluetoothDevice's "uuids" property.
4 years, 1 month ago (2016-10-27 18:38:29 UTC) #72
Rick Byers
WebKit LGTM I just took a look at the content changes too - couple minor ...
4 years, 1 month ago (2016-10-27 20:37:54 UTC) #75
juncai
https://codereview.chromium.org/2437943002/diff/170001/content/browser/frame_host/render_frame_host_impl.cc File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/2437943002/diff/170001/content/browser/frame_host/render_frame_host_impl.cc#newcode2184 content/browser/frame_host/render_frame_host_impl.cc:2184: switches::kEnableExperimentalWebPlatformFeatures); On 2016/10/27 20:37:53, Rick Byers wrote: > Note ...
4 years, 1 month ago (2016-10-27 21:54:26 UTC) #78
ortuno
https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_features.cc File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_features.cc#newcode60 content/child/runtime_features.cc:60: WebRuntimeFeatures::enableWebBluetooth(true); Now that you are doing this here couldn't ...
4 years, 1 month ago (2016-10-27 22:58:33 UTC) #80
juncai
https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_features.cc File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_features.cc#newcode60 content/child/runtime_features.cc:60: WebRuntimeFeatures::enableWebBluetooth(true); On 2016/10/27 22:58:33, ortuno wrote: > Now that ...
4 years, 1 month ago (2016-10-28 02:59:51 UTC) #87
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/2437943002/210001
4 years, 1 month ago (2016-10-28 03:00:43 UTC) #90
commit-bot: I haz the power
Committed patchset #12 (id:210001)
4 years, 1 month ago (2016-10-28 03:09:40 UTC) #92
commit-bot: I haz the power
4 years, 1 month ago (2016-10-28 03:10:48 UTC) #94
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/3ad09e105c07e4b22423866f4d16ed5adfe6db45
Cr-Commit-Position: refs/heads/master@{#428265}

Powered by Google App Engine
This is Rietveld 408576698