|
|
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. |
DescriptionShip 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 #Messages
Total messages: 94 (68 generated)
Description was changed from ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. BUG=651261 ========== to ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "Web Bluetooth is not enabled on this platform. To find out how to enable it and the current implementation status visit https://goo.gl/HKa2If" WebBluetooth bindings are available, but the feature is blocked. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
Description was changed from ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "Web Bluetooth is not enabled on this platform. To find out how to enable it and the current implementation status visit https://goo.gl/HKa2If" WebBluetooth bindings are available, but the feature is blocked. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "Web Bluetooth is not enabled on this platform. To find out how to enable it and the current implementation status visit https://goo.gl/HKa2If" WebBluetooth bindings are available, but the feature is blocked. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
Description was changed from ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "Web Bluetooth is not enabled on this platform. To find out how to enable it and the current implementation status visit https://goo.gl/HKa2If" WebBluetooth bindings are available, but the feature is blocked. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "undefined", bindings are not exposed. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
juncai@chromium.org changed reviewers: + scheib@chromium.org
Please take a look.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "undefined", bindings are not exposed. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "undefined", bindings are not exposed. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: --enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Ship WebBluetooth out of origin trial This CL does the following to move WebBluetooth out of origin trial: 1. remove the --enable-we-bluetooth flag. 2. use --enable-experimental-web-platform-features flag to enable WebBluetooth on non-shipped platforms. 3. remove the origin trial for WebBluetooth. TEST=Manual testing required: In each scenario below, to "Execute Test" 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: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: "undefined", bindings are not exposed. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: "undefined", bindings are not exposed. 2. Run Chrome with flag: --enable-experimental-web-platform-features ChromeOS, Android, MacOS, Linux, Windows: https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... Execute Test and expect: WebBluetooth works. https://g-ortuno.github.io/web-bluetooth-sandbox/not-allowed.html Execute Test and expect: WebBluetooth works. BUG=651261 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Ship WebBluetooth out of origin trial 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. TEST=Manual testing required: In each scenario below, to "Execute Test": For each URL https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... 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 ==========
I modified the description some https://www.diffchecker.com/n5MKZUZN Also, add to the description an explanation that third_party/WebKit/LayoutTests/platform/win win7 linux versions of virtual/stable/webexposed/global-interface-listing-expected.txt are being added and why. https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/policy/p... File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/policy/p... chrome/browser/policy/policy_browsertest.cc:3418: command_line->AppendSwitch( Keep a comment with a TODO(570344,507419): Remove this switch once Web Bluetooth is supported on Linux and Windows. https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/web_blue... File chrome/browser/web_bluetooth_browsertest.cc (right): https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/web_blue... chrome/browser/web_bluetooth_browsertest.cc:34: command_line->AppendSwitch( comment as above. https://codereview.chromium.org/2437943002/diff/50001/content/browser/frame_h... File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/2437943002/diff/50001/content/browser/frame_h... content/browser/frame_host/render_frame_host_impl.cc:2227: bool enable_web_bluetooth = base::CommandLine::ForCurrentProcess()->HasSwitch( Let's explain this with // Web Bluetooth is shipped on Android, ChromeOS & MacOS, experimental otherwise. https://codereview.chromium.org/2437943002/diff/50001/content/child/runtime_f... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/50001/content/child/runtime_f... content/child/runtime_features.cc:72: bool enable_web_bluetooth = enableExperimentalWebPlatformFeatures; Let's explain this with // Web Bluetooth is shipped on Android, ChromeOS & MacOS, experimental otherwise.
Description was changed from ========== Ship WebBluetooth out of origin trial 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. TEST=Manual testing required: In each scenario below, to "Execute Test": For each URL https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial 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. 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 third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt are added. TEST=Manual testing required: In each scenario below, to "Execute Test": For each URL https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... 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 ==========
Description was changed from ========== Ship WebBluetooth out of origin trial 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. 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 third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt are added. TEST=Manual testing required: In each scenario below, to "Execute Test": For each URL https://googlechrome.github.io/samples/web-bluetooth/device-information-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial 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. 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 third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt are added. 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-chara... 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 ==========
Description was changed from ========== Ship WebBluetooth out of origin trial 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. 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 third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt are added. 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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial 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. 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 third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt are added. 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-chara... 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 ==========
Description was changed from ========== Ship WebBluetooth out of origin trial 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. 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 third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt are added. 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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial 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. Three 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 third_party/WebKit/LayoutTests/platform/win7/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-chara... 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 ==========
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/policy/p... File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/policy/p... chrome/browser/policy/policy_browsertest.cc:3418: command_line->AppendSwitch( On 2016/10/21 01:02:10, scheib wrote: > Keep a comment with a TODO(570344,507419): Remove this switch once Web Bluetooth > is supported on Linux and Windows. Done. https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/web_blue... File chrome/browser/web_bluetooth_browsertest.cc (right): https://codereview.chromium.org/2437943002/diff/50001/chrome/browser/web_blue... chrome/browser/web_bluetooth_browsertest.cc:34: command_line->AppendSwitch( On 2016/10/21 01:02:10, scheib wrote: > comment as above. Done. https://codereview.chromium.org/2437943002/diff/50001/content/browser/frame_h... File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/2437943002/diff/50001/content/browser/frame_h... content/browser/frame_host/render_frame_host_impl.cc:2227: bool enable_web_bluetooth = base::CommandLine::ForCurrentProcess()->HasSwitch( On 2016/10/21 01:02:10, scheib wrote: > Let's explain this with > // Web Bluetooth is shipped on Android, ChromeOS & MacOS, experimental > otherwise. Done. https://codereview.chromium.org/2437943002/diff/50001/content/child/runtime_f... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/50001/content/child/runtime_f... content/child/runtime_features.cc:72: bool enable_web_bluetooth = enableExperimentalWebPlatformFeatures; On 2016/10/21 01:02:10, scheib wrote: > Let's explain this with > // Web Bluetooth is shipped on Android, ChromeOS & MacOS, experimental > otherwise. Done.
LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
juncai@chromium.org changed reviewers: + jam@chromium.org, pastarmovj@chromium.org
pastarmovj@chromium.org: Please review changes in //chrome/browser/policy/policy_browsertest.cc jam@chromium.org: Please review changes in //content //third_party
On 2016/10/21 21:20:25, juncai wrote: > mailto:pastarmovj@chromium.org: Please review changes in > //chrome/browser/policy/policy_browsertest.cc > > mailto:jam@chromium.org: Please review changes in > //content lgtm > //third_party I'm not a webkit owner, pick someone from that directory
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/s...
juncai@chromium.org changed reviewers: + rbyers@chromium.org
rbyers@chromium.org: Please review changes in //third_party/WebKit
bindings/ LGTM
Description was changed from ========== Ship WebBluetooth out of origin trial 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. Three 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 third_party/WebKit/LayoutTests/platform/win7/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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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. Three 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 third_party/WebKit/LayoutTests/platform/win7/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-chara... 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 ==========
juncai, you should also replicate https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/virtual/s... into each new subdirectory created under LayoutTests/platform.
I reviewed WebKit except WebKit/Source/modules/bluetooth (since I'm not an expert there but scheib@, who is and is also an OWNER has already reviewed). https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt (left): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt:1: CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use 'OfflineAudioContext' instead. It looks like this is the first time we've needed platform-specific expectations for this test. I think that's probably going to be a pain for devs since the expectations are changed whenever a new API is shipped. Eg. I think people are used to just being able to automatically update the expectations from their dev box. How would you feel about doing a bit of extra work to try to improve this test to minimize the annoyance? Two possible ideas off the top of my head: 1) Augment the test runner to support the idea of platform-specific diffs. Then rather than duplicate the expected output for this whole test, you could just store the diff which removes the bluetooth APIs from the expected output on linux and windows. OR 2) Split the test into two. Have a list of interfaces for "platform specific APIs" and avoid walking them in the main test. Then a platform-specific version of the test would enumerate just those interfaces. I think this would be nearly trivial and would probably be enough to eliminate most of the pain in cases like this. https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt (left): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt:1: CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use 'OfflineAudioContext' instead. Does 'win7' really not follow automatically from 'win'? Why can't you just have 'linux' and 'win'? Though (depending on my other question) perhaps this is irrelevant. https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids I don't see a "uuids" property on BluetoothDevice in the spec, what's up with it? https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:240: WebBluetooth status=experimental It's confusing having a status=experimental feature being something we're actually shipping. Could you, for example, rename this to WebBluetoothLinuxWindows or something - make it just about the remaining platforms? At a minimum please add a comment here saying that WebBluetooth is enabled by default on Android, ChromeOS and Mac.
policy lgtm.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Description was changed from ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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. Three 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 third_party/WebKit/LayoutTests/platform/win7/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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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. Three 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-chara... 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 ==========
Description was changed from ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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. Three 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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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-chara... 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 ==========
juncai@chromium.org changed reviewers: + dpranke@chromium.org
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 deal with the platform-specific tests for WebBluetooth API. Thanks! https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt (left): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt:1: CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use 'OfflineAudioContext' instead. On 2016/10/24 16:38:43, Rick Byers wrote: > It looks like this is the first time we've needed platform-specific expectations > for this test. I think that's probably going to be a pain for devs since the > expectations are changed whenever a new API is shipped. Eg. I think people are > used to just being able to automatically update the expectations from their dev > box. > > How would you feel about doing a bit of extra work to try to improve this test > to minimize the annoyance? Two possible ideas off the top of my head: > > 1) Augment the test runner to support the idea of platform-specific diffs. Then > rather than duplicate the expected output for this whole test, you could just > store the diff which removes the bluetooth APIs from the expected output on > linux and windows. > > OR > > 2) Split the test into two. Have a list of interfaces for "platform specific > APIs" and avoid walking them in the main test. Then a platform-specific version > of the test would enumerate just those interfaces. I think this would be nearly > trivial and would probably be enough to eliminate most of the pain in cases like > this. Since we already have a //third_party/WebKit/LayoutTests/platform/ directory that deals with platform-specific tests, I feel that splitting the tests may over-complicate things. +dpranke@ for some thoughts. https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt (left): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt:1: CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use 'OfflineAudioContext' instead. On 2016/10/24 16:38:43, Rick Byers wrote: > Does 'win7' really not follow automatically from 'win'? Why can't you just have > 'linux' and 'win'? Though (depending on my other question) perhaps this is > irrelevant. Done. https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids On 2016/10/24 16:38:43, Rick Byers wrote: > I don't see a "uuids" property on BluetoothDevice in the spec, what's up with > it? uuids property is deprecated recently, so it is removed from the BluethoothDevice spec. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/blueto... https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:240: WebBluetooth status=experimental On 2016/10/24 16:38:43, Rick Byers wrote: > It's confusing having a status=experimental feature being something we're > actually shipping. Could you, for example, rename this to > WebBluetoothLinuxWindows or something - make it just about the remaining > platforms? At a minimum please add a comment here saying that WebBluetooth is > enabled by default on Android, ChromeOS and Mac. Done.
On 2016/10/26 15:54:56, juncai wrote: > 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 deal with the platform-specific tests for WebBluetooth API. Thanks! > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt > (left): > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt:1: > CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use > 'OfflineAudioContext' instead. > On 2016/10/24 16:38:43, Rick Byers wrote: > > It looks like this is the first time we've needed platform-specific > expectations > > for this test. I think that's probably going to be a pain for devs since the > > expectations are changed whenever a new API is shipped. Eg. I think people > are > > used to just being able to automatically update the expectations from their > dev > > box. > > > > How would you feel about doing a bit of extra work to try to improve this test > > to minimize the annoyance? Two possible ideas off the top of my head: > > > > 1) Augment the test runner to support the idea of platform-specific diffs. > Then > > rather than duplicate the expected output for this whole test, you could just > > store the diff which removes the bluetooth APIs from the expected output on > > linux and windows. > > > > OR > > > > 2) Split the test into two. Have a list of interfaces for "platform specific > > APIs" and avoid walking them in the main test. Then a platform-specific > version > > of the test would enumerate just those interfaces. I think this would be > nearly > > trivial and would probably be enough to eliminate most of the pain in cases > like > > this. > > Since we already have a //third_party/WebKit/LayoutTests/platform/ directory > that deals with platform-specific tests, I feel that splitting the tests may > over-complicate things. +dpranke@ for some thoughts. IMHO it'll be way less complicated for people to update only a single expected file in the 99% case where their API is shipping on all platforms, and completely ignore these platform-specific files (and I thought it would be pretty trivial to implement that split). But if you want to land as-is and see if people complain I can live with that (worst case I can do the split myself) . > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt > (left): > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/platform/win7/virtual/stable/webexposed/global-interface-listing-expected.txt:1: > CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use > 'OfflineAudioContext' instead. > On 2016/10/24 16:38:43, Rick Byers wrote: > > Does 'win7' really not follow automatically from 'win'? Why can't you just > have > > 'linux' and 'win'? Though (depending on my other question) perhaps this is > > irrelevant. > > Done. > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt > (right): > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: > getter uuids > On 2016/10/24 16:38:43, Rick Byers wrote: > > I don't see a "uuids" property on BluetoothDevice in the spec, what's up with > > it? > > uuids property is deprecated recently, so it is removed from the > BluethoothDevice spec. > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/blueto... > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Sour... > File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right): > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Sour... > third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:240: WebBluetooth > status=experimental > On 2016/10/24 16:38:43, Rick Byers wrote: > > It's confusing having a status=experimental feature being something we're > > actually shipping. Could you, for example, rename this to > > WebBluetoothLinuxWindows or something - make it just about the remaining > > platforms? At a minimum please add a comment here saying that WebBluetooth is > > enabled by default on Android, ChromeOS and Mac. > > Done.
https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... 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 2016/10/24 16:38:43, Rick Byers wrote: > > I don't see a "uuids" property on BluetoothDevice in the spec, what's up with > > it? > > uuids property is deprecated recently, so it is removed from the > BluethoothDevice spec. > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/blueto... Why would we want to ship it then? Can't we straight out remove it now (since we haven't actually shipped web bluetooth yet)?
It looks like this baseline gets changed a few times a week these days, and rarely by the same person. Given that it's not hard to rebaseline things now (with webkit-patch rebaseline-cl) it doesn't seem like it's the end of the world to have a single file, but I could also see that splitting it into two files might make life easier. I would not add support for platform-specific diffs; that's too much complexity both in the implementation and the user interface to be justified for just this. lgtm either way, I don't have a strong feeling here but I'm not affected by this, either.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... 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: > On 2016/10/26 15:54:55, juncai wrote: > > On 2016/10/24 16:38:43, Rick Byers wrote: > > > I don't see a "uuids" property on BluetoothDevice in the spec, what's up > with > > > it? > > > > uuids property is deprecated recently, so it is removed from the > > BluethoothDevice spec. > > > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/blueto... > > Why would we want to ship it then? Can't we straight out remove it now (since > we haven't actually shipped web bluetooth yet)? Since WebBluetooth API is now in origin trial, and there are users already using this API. So in order to avoid breaking user's code, we deprecate this uuids property first, and then will remove it after M56 branch. There is a bug tracking it: https://bugs.chromium.org/p/chromium/issues/detail?id=653317&desc=2
On 2016/10/26 17:37:38, juncai wrote: > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt > (right): > > https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... > 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: > > On 2016/10/26 15:54:55, juncai wrote: > > > On 2016/10/24 16:38:43, Rick Byers wrote: > > > > I don't see a "uuids" property on BluetoothDevice in the spec, what's up > > with > > > > it? > > > > > > uuids property is deprecated recently, so it is removed from the > > > BluethoothDevice spec. > > > > > > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/blueto... > > > > Why would we want to ship it then? Can't we straight out remove it now (since > > we haven't actually shipped web bluetooth yet)? > > Since WebBluetooth API is now in origin trial, and there are users already using > this API. So in order to avoid breaking user's code, we deprecate this uuids > property first, and then will remove it after M56 branch. There is a bug > tracking it: > https://bugs.chromium.org/p/chromium/issues/detail?id=653317&desc=2 For the record, discussion about this is happening here: https://groups.google.com/a/chromium.org/forum/#!topic/blink-api-owners-discu... Also the earlier question about webexposed test results here: https://groups.google.com/a/chromium.org/forum/#!topic/blink-api-owners-discu...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids Removed BluetoothDevice's "uuids" property.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
WebKit LGTM I just took a look at the content changes too - couple minor suggestions there for simplifications, but I'll leave that up to you. https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2437943002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:250: getter uuids On 2016/10/27 18:38:29, juncai wrote: > Removed BluetoothDevice's "uuids" property. Thanks! https://codereview.chromium.org/2437943002/diff/170001/content/browser/frame_... File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/2437943002/diff/170001/content/browser/frame_... content/browser/frame_host/render_frame_host_impl.cc:2184: switches::kEnableExperimentalWebPlatformFeatures); Note that this won't necessarily match blink's view of this feature. Eg. someone could run with --enable-blink-features=WebBluetooth or --enable-experimental-web-platform-features --disable-blink-features=WebBluetooth. It would be simpler if you could always do the below code, but it would (for now) generally go unused on Windows and Linux. https://codereview.chromium.org/2437943002/diff/170001/content/child/runtime_... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/170001/content/child/runtime_... content/child/runtime_features.cc:74: #if defined(OS_CHROMEOS) || defined(OS_ANDROID) || defined(OS_MACOSX) please move this into SetRuntimeFeatureDefaultsForPlatform along with all the other per-platform feature settings. Note that there's no need to rely on the enableExperimentalWebPlatformFeatures (that's handled above) - just do: #if defined(OS_CHROMEOS) || ... WebRuntimeFeatures::enabledWebBluetooth(true); #endif
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2437943002/diff/170001/content/browser/frame_... File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/2437943002/diff/170001/content/browser/frame_... content/browser/frame_host/render_frame_host_impl.cc:2184: switches::kEnableExperimentalWebPlatformFeatures); On 2016/10/27 20:37:53, Rick Byers wrote: > Note that this won't necessarily match blink's view of this feature. Eg. > someone could run with --enable-blink-features=WebBluetooth or > --enable-experimental-web-platform-features > --disable-blink-features=WebBluetooth. > > It would be simpler if you could always do the below code, but it would (for > now) generally go unused on Windows and Linux. Done. https://codereview.chromium.org/2437943002/diff/170001/content/child/runtime_... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/170001/content/child/runtime_... content/child/runtime_features.cc:74: #if defined(OS_CHROMEOS) || defined(OS_ANDROID) || defined(OS_MACOSX) On 2016/10/27 20:37:54, Rick Byers wrote: > please move this into SetRuntimeFeatureDefaultsForPlatform along with all the > other per-platform feature settings. Note that there's no need to rely on the > enableExperimentalWebPlatformFeatures (that's handled above) - just do: > #if defined(OS_CHROMEOS) || ... > WebRuntimeFeatures::enabledWebBluetooth(true); > #endif > Done.
ortuno@chromium.org changed reviewers: + haraken@chromium.org, ortuno@chromium.org
https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_... content/child/runtime_features.cc:60: WebRuntimeFeatures::enableWebBluetooth(true); Now that you are doing this here couldn't you just do: if (RuntimeEnabledFeatures::webBluetoothEnabled()) { BluetoothSupplement::provideTo(frame, client->bluetooth()); } in https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/ChromeClie... Just a suggestion, no need to delay landing the patch for this.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by juncai@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2437943002/diff/190001/content/child/runtime_... content/child/runtime_features.cc:60: WebRuntimeFeatures::enableWebBluetooth(true); On 2016/10/27 22:58:33, ortuno wrote: > Now that you are doing this here couldn't you just do: > > if (RuntimeEnabledFeatures::webBluetoothEnabled()) { > BluetoothSupplement::provideTo(frame, client->bluetooth()); > } > > in > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/ChromeClie... > > Just a suggestion, no need to delay landing the patch for this. Done.
The CQ bit was checked by juncai@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, scheib@chromium.org, pastarmovj@chromium.org, jam@chromium.org, dpranke@chromium.org, rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2437943002/#ps210001 (title: "address more comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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-chara... 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 ==========
Message was sent while issue was closed.
Committed patchset #12 (id:210001)
Message was sent while issue was closed.
Description was changed from ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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-chara... 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 ========== to ========== Ship WebBluetooth out of origin trial Intent to Ship: Web Bluetooth https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Ono3RWkejAA/... 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-chara... 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} ==========
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/3ad09e105c07e4b22423866f4d16ed5adfe6db45 Cr-Commit-Position: refs/heads/master@{#428265} |