third_party/WebKit/LayoutTests/presentation/presentationrequest.html - Issue 2552343009: [Presentation API] Adds DOMString[] constructor to PresentationRequest.

Side by Side Diff: third_party/WebKit/LayoutTests/presentation/presentationrequest.html

Issue 2552343009: [Presentation API] Adds DOMString[] constructor to PresentationRequest. (Closed)

Patch Set: resolve code review comments from mlamouri Created 4 years ago

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

Jump to:

« third_party/WebKit/LayoutTests/presentation/presentation-navigation-multipleurls.html ('K') | « third_party/WebKit/LayoutTests/presentation/presentation-navigation-multipleurls.html ('k') | third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt » ('j') | third_party/WebKit/Source/modules/presentation/PresentationRequest.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <html>	2 <html>

3 <body>	3 <body>

4 <script src="../resources/testharness.js"></script>	4 <script src="../resources/testharness.js"></script>

5 <script src="../resources/testharnessreport.js"></script>	5 <script src="../resources/testharnessreport.js"></script>

6 <script src="../resources/gc.js"></script>	6 <script src="../resources/gc.js"></script>

7 <script>	7 <script>

8	8

9 async_test(function(t) {	9 var presentationUrl = "http://example.com";
	whywhat 2016/12/14 02:20:39 I think in general your presentation-api.html addi I think in general your presentation-api.html addition covers all the changes needed for this cl. We can maybe change the other tests to simply use a list of urls instead of a single one to update them to the preferred ctor but I don't feel there's a need to duplicate each test we have with a single URL atm.
10 var request = new PresentationRequest("http://example.com");	10 var presentationUrls = [presentationUrl, "cast://google.com/app_id=deadbeef"];

	11

	12 var testUserGesture = function(t, requestArgument) {
	mlamouri (slow - plz ping) 2016/12/13 13:50:53 Having the method called `testUserGesture` is conf Having the method called `testUserGesture` is confusing. I'm not sure where you get the user gesture token too. zhaobin 2016/12/14 01:51:53 Term "user gesture" may come from "The controlling Show quoted text On 2016/12/13 13:50:53, mlamouri wrote: > Having the method called `testUserGesture` is confusing. I'm not sure where you > get the user gesture token too. Term "user gesture" may come from "The controlling user agent should initiate presentation using the default presentation request only when the user has expressed an intention to do so via a user gesture, for example by clicking a button in the browser chrome." (https://w3c.github.io/presentation-api/) PresentationRequest.start() will pop MR dialog, and it needs a user gesture to continue (e.g. user selects a device). So the test is called testUserGesture... whywhat 2016/12/14 02:20:39 I think Mounir means that the name could be more s Show quoted text On 2016/12/14 at 01:51:53, zhaobin wrote: > On 2016/12/13 13:50:53, mlamouri wrote: > > Having the method called `testUserGesture` is confusing. I'm not sure where you > > get the user gesture token too. > > Term "user gesture" may come from "The controlling user agent should initiate presentation using the default presentation request only when the user has expressed an intention to do so via a user gesture, for example by clicking a button in the browser chrome." (https://w3c.github.io/presentation-api/) > > PresentationRequest.start() will pop MR dialog, and it needs a user gesture to continue (e.g. user selects a device). So the test is called testUserGesture... I think Mounir means that the name could be more specific, e.g. testRequestStartThrowsWithoutUserGesture or similar. That would avoid his second question about getting the gesture token (one would emulate a mouse click on a button for that but since you're testing for the promise to be rejected, the test must not get a token). zhaobin 2016/12/14 03:00:27 I misunderstood "token"...sorry for that. Show quoted text On 2016/12/14 02:20:39, whywhat wrote: > On 2016/12/14 at 01:51:53, zhaobin wrote: > > On 2016/12/13 13:50:53, mlamouri wrote: > > > Having the method called `testUserGesture` is confusing. I'm not sure where > you > > > get the user gesture token too. > > > > Term "user gesture" may come from "The controlling user agent should initiate > presentation using the default presentation request only when the user has > expressed an intention to do so via a user gesture, for example by clicking a > button in the browser chrome." (https://w3c.github.io/presentation-api/) > > > > PresentationRequest.start() will pop MR dialog, and it needs a user gesture to > continue (e.g. user selects a device). So the test is called testUserGesture... > > I think Mounir means that the name could be more specific, e.g. > testRequestStartThrowsWithoutUserGesture or similar. That would avoid his second > question about getting the gesture token (one would emulate a mouse click on a > button for that but since you're testing for the promise to be rejected, the > test must not get a token). I misunderstood "token"...sorry for that. whywhat 2016/12/14 03:53:14 No worries, IIUC it's a Blink specific terminology Show quoted text On 2016/12/14 at 03:00:27, zhaobin wrote: > On 2016/12/14 02:20:39, whywhat wrote: > > On 2016/12/14 at 01:51:53, zhaobin wrote: > > > On 2016/12/13 13:50:53, mlamouri wrote: > > > > Having the method called `testUserGesture` is confusing. I'm not sure where > > you > > > > get the user gesture token too. > > > > > > Term "user gesture" may come from "The controlling user agent should initiate > > presentation using the default presentation request only when the user has > > expressed an intention to do so via a user gesture, for example by clicking a > > button in the browser chrome." (https://w3c.github.io/presentation-api/) > > > > > > PresentationRequest.start() will pop MR dialog, and it needs a user gesture to > > continue (e.g. user selects a device). So the test is called testUserGesture... > > > > I think Mounir means that the name could be more specific, e.g. > > testRequestStartThrowsWithoutUserGesture or similar. That would avoid his second > > question about getting the gesture token (one would emulate a mouse click on a > > button for that but since you're testing for the promise to be rejected, the > > test must not get a token). > > I misunderstood "token"...sorry for that. No worries, IIUC it's a Blink specific terminology around the user gesture which is again a somewhat special term roughly meaning user has to tap/click on the page for some API to work :) We should've not used the term that might have been unfamiliar to you in the first place.
	13 var request = new PresentationRequest(requestArgument);

11 request.start().catch(t.step_func(function(e) {	14 request.start().catch(t.step_func(function(e) {
	whywhat 2016/12/14 02:20:39 nit: you can use t.step_func_done and remove t.don nit: you can use t.step_func_done and remove t.done() from the end. zhaobin 2016/12/14 03:00:27 Done. Show quoted text On 2016/12/14 02:20:39, whywhat wrote: > nit: you can use t.step_func_done and remove t.done() from the end. Done.
12 assert_true(e instanceof DOMException);	15 assert_true(e instanceof DOMException);

13 assert_equals(e.name, "InvalidAccessError");	16 assert_equals(e.name, "InvalidAccessError");

14 assert_equals(e.message, "PresentationRequest::start() requires user gesture .");	17 assert_equals(e.message, "PresentationRequest::start() requires user gesture .");

15 t.done();	18 t.done();

16 }));	19 }));
	whywhat 2016/12/14 02:20:39 you should probably also have then(t.unreached_fun you should probably also have then(t.unreached_func()) to verify the promise is not resolved. zhaobin 2016/12/14 03:00:26 Done. Show quoted text On 2016/12/14 02:20:39, whywhat wrote: > you should probably also have then(t.unreached_func()) to verify the promise is > not resolved. Done.
17 }, "Test that the PresentationRequest.start() requires user gesture.")	20 };

18	21

19 test(function() {	22 async_test(function(t) {
	whywhat 2016/12/14 02:20:39 ...or you could even use promise_test (https://git ...or you could even use promise_test (https://github.com/w3c/testharness.js/blob/master/docs/api.md#promise-tests) to make the test even shorter :) See https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/media/rem... for an example. zhaobin 2016/12/14 03:00:27 Done. Show quoted text On 2016/12/14 02:20:39, whywhat wrote: > ...or you could even use promise_test > (https://github.com/w3c/testharness.js/blob/master/docs/api.md#promise-tests) to > make the test even shorter :) > > See > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/media/rem... > for an example. Done.
20 navigator.presentation.defaultRequest = new PresentationRequest("http://exampl e.com");	23 testUserGesture(t, presentationUrl);

	24 }, "Test that the PresentationRequest.start() with one URL requires user gesture .")

	25

	26 async_test(function(t) {

	27 testUserGesture(t, presentationUrls);

	28 }, "Test that the PresentationRequest.start() with multiple URLs requires user g esture.")

	29

	30 var testGarbageCollection = function(requestArgument) {

	31 navigator.presentation.defaultRequest = new PresentationRequest(requestArgumen t);

21 navigator.presentation.defaultRequest.onconnectionavailable = function() { };	32 navigator.presentation.defaultRequest.onconnectionavailable = function() { };

22 gc();	33 gc();

23 assert_not_equals(navigator.presentation.defaultRequest.onconnectionavailable, undefined);	34 assert_not_equals(navigator.presentation.defaultRequest.onconnectionavailable, undefined);

24 }, "Test that navigator.presentation.defaultRequest.onconnectionavailable isn't reset after gc()");	35 };

	36

	37 test(function() {

	38 testGarbageCollection(presentationUrl);

	39 }, "Test that navigator.presentation.defaultRequest.onconnectionavailable with o ne URL isn't reset after gc().");

	40

	41 test(function() {

	42 testGarbageCollection(presentationUrls);

	43 }, "Test that navigator.presentation.defaultRequest.onconnectionavailable with m ultiple URLs isn't reset after gc().");

25	44

26 </script>	45 </script>

27 </body>	46 </body>

28 </html>	47 </html>

OLD	NEW