third_party/WebKit/LayoutTests/external/wpt/webusb/usbFeaturePolicy.https.sub.html - Issue 2815003005: Integrate WebUSB with Feature Policy

Side by Side Diff: third_party/WebKit/LayoutTests/external/wpt/webusb/usbFeaturePolicy.https.sub.html

Issue 2815003005: Integrate WebUSB with Feature Policy (Closed)

Patch Set: Ready for review Created 3 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« device/usb/mojo/device_impl.cc ('K') | « third_party/WebKit/LayoutTests/external/wpt/webusb/resources/check-availability.html ('k') | third_party/WebKit/Source/modules/webusb/USB.cpp » ('j') | third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <head>
	lunalu1 2017/05/03 15:01:21 "File names should generally be somewhat descripti Show quoted text >>>Please correct me if I am wrong. According to the WPT guide (http://web-platform-tests.org/writing-tests/general-guidelines.html): "File names should generally be somewhat descriptive of what is being tested; very generic names like 001.html are discouraged. A common format is test-topic-001.html, where test-topic is a short identifier that describes the test." Maybe we can consider rename this test to be usb-feature-policy.sub.html Show quoted text >>>Question, why do we need ".https" here? So please ignore me if I am wrong. Reilly Grant (use Gerrit) 2017/05/04 01:16:50 Done. Show quoted text On 2017/05/03 15:01:21, loonybear wrote: > >>>Please correct me if I am wrong. According to the WPT guide > (http://web-platform-tests.org/writing-tests/general-guidelines.html): > "File names should generally be somewhat descriptive of what is being tested; > very generic names like 001.html are discouraged. A common format is > test-topic-001.html, where test-topic is a short identifier that describes the > test." > > Maybe we can consider rename this test to be usb-feature-policy.sub.html Done. Show quoted text > Question, why do we need ".https" here? This API is only available in secure contexts.
	4 <meta charset="utf-8">
	lunalu1 2017/05/03 15:01:21 We also need more tests for this with feature poli We also need more tests for this with feature policy: case 1: disabled in the http header So create a test file named usb-disabled-by-feature-policy.sub.html with header policy usb-disabled-by-feature-policy.sub.html.headers: Feature-Policy: {"usb": []} case 2: enabled for all (this is the default policy for usb), same as above. Feature-Policy: {"usb": ['']} case 3: enabled for self, same as above. Feature-Policy: {"usb": ['self']} In all 3 cases, we should test if usb is enabled/disabled in same-origin and cross-origin which you have done in your 2 async_tests. Ideally, there should also be test where usb is enabled through iframe allow attribute with relocation (with and without reloading the document) too. I am working on some examples of how to write those tests. So maybe you can add those tests later once I commit the examples. Reilly Grant (use Gerrit)* 2017/05/04 01:16:50 Done. Show quoted text On 2017/05/03 15:01:21, loonybear wrote: > We also need more tests for this with feature policy: > case 1: disabled in the http header > So create a test file named usb-disabled-by-feature-policy.sub.html > with header policy usb-disabled-by-feature-policy.sub.html.headers: > Feature-Policy: {"usb": []} > case 2: enabled for all (this is the default policy for usb), same as above. > Feature-Policy: {"usb": ['*']} > case 3: enabled for self, same as above. > Feature-Policy: {"usb": ['self']} > > In all 3 cases, we should test if usb is enabled/disabled in same-origin and > cross-origin which you have done in your 2 async_tests. > > Ideally, there should also be test where usb is enabled through iframe allow > attribute with relocation (with and without reloading the document) too. I am > working on some examples of how to write those tests. So maybe you can add those > tests later once I commit the examples. Done.
	5 <title>WebUSB Feature Policy Tests</title>

	6 <link rel="help" href="https://wicg.github.io/webusb/">
	lunalu1 2017/05/03 15:01:21 By the way, I assume you tested this file followin By the way, I assume you tested this file following the instructions (http://web-platform-tests.org/running-tests/index.html). If not, I encourage you to do so. Reilly Grant (use Gerrit) 2017/05/04 01:16:50 Yes. Show quoted text On 2017/05/03 15:01:21, loonybear wrote: > By the way, I assume you tested this file following the instructions > (http://web-platform-tests.org/running-tests/index.html). If not, I encourage > you to do so. Yes.
	7 <script src=/resources/testharness.js></script>

	8 <script src=/resources/testharnessreport.js></script>
	iclelland.google 2017/05/02 14:42:52 @lunalu, you've been looking that the use of testh @lunalu, you've been looking that the use of testharnessreport.js in the other Feature Policy tests, for crbug.com/716085 -- can you take a look at this test to see if it's going to need to be changed as well?
	9 </head>

	10 <body>

	11 <script>

	12 'use strict';

	13

	14 promise_test(

	15 () => navigator.usb.getDevices(),

	16 'WebUSB is available in the top-level document.');

	17

	18 async_test(t => {
	lunalu1 2017/05/03 15:01:21 The 3 async_tests look very similar in format. We The 3 async_tests look very similar in format. We could write one generic test method and call it 3 times. But given this is just a test file. It doesn't have to be done.
	19 let frame = document.createElement('iframe');

	20 frame.src = '/webusb/resources/check-availability.html';

	21

	22 window.addEventListener('message', t.step_func(evt => {

	23 if (evt.source == frame.contentWindow) {

	24 assert_equals(evt.data, '#OK');

	25 document.body.removeChild(frame);

	26 t.done();

	27 }

	28 }));

	29

	30 document.body.appendChild(frame);

	31 }, 'WebUSB is available in same-origin iframes.');

	32

	33 async_test(t => {

	34 let frame = document.createElement('iframe');

	35 frame.src = 'https://{{domains[www]}}:{{ports[https][0]}}/webusb/resources/c heck-availability.html';

	36

	37 window.addEventListener('message', t.step_func(evt => {

	38 if (evt.source == frame.contentWindow) {

	39 assert_equals(evt.data, '#MISSING');

	40 document.body.removeChild(frame);

	41 t.done();

	42 }

	43 }));

	44

	45 document.body.appendChild(frame);

	46 }, 'WebUSB is not available cross-origin iframes.');

	47

	48 async_test(t => {

	49 let frame = document.createElement('iframe');

	50 frame.src = 'https://{{domains[www]}}:{{ports[https][0]}}/webusb/resources/c heck-availability.html';

	51 frame.allow = 'usb';

	52

	53 window.addEventListener('message', t.step_func(evt => {

	54 if (evt.source == frame.contentWindow) {

	55 assert_equals(evt.data, '#OK');

	56 document.body.removeChild(frame);

	57 t.done();

	58 }

	59 }));

	60

	61 document.body.appendChild(frame);

	62 }, 'WebUSB can be enabled in cross-origin iframes using Feature Policy.');

	63 </script>

	64 </body>

	65 </html>
	lunalu1 2017/05/03 15:01:21 I don't think <head> <html> and <body> are needed I don't think <head> <html> and <body> are needed for layout tests. But I could be wrong. Reilly Grant (use Gerrit) 2017/05/04 01:16:50 <body> is required but the rest are not. Show quoted text On 2017/05/03 15:01:21, loonybear wrote: > I don't think <head> <html> and <body> are needed for layout tests. But I could > be wrong. <body> is required but the rest are not.
OLD	NEW