LayoutTests/http/tests/notifications/click-window-focus-document.html - Issue 1312623003: Flakyness: fix notifications/ tests.

Side by Side Diff: LayoutTests/http/tests/notifications/click-window-focus-document.html

Issue 1312623003: Flakyness: fix notifications/ tests. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@webmidi-flakyness

Patch Set: add missing file Created 5 years, 4 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:

View unified diff | Download patch

« LayoutTests/http/tests/notifications/click-document.html ('K') | « LayoutTests/http/tests/notifications/click-document.html ('k') | LayoutTests/http/tests/notifications/close-document.html » ('j') | LayoutTests/http/tests/notifications/close-document.html » ('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 <head>	3 <head>

4 <title>Notifications: Should be able to focus window in onclick event.</titl e>	4 <title>Notifications: Should be able to focus window in onclick event.</titl e>

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

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

	7 <script src="../resources/permissions-helper.js"></script>

7 </head>	8 </head>

8 <body>	9 <body>

9 <script>	10 <script>

10 // Tests that Document-bound notifications are able to focus a window in	11 // Tests that Document-bound notifications are able to focus a window in

11 // their onclick event. When the test is being run manually, grant	12 // their onclick event. When the test is being run manually, grant

12 // Notification permission and click on the notification.	13 // Notification permission and click on the notification.

13 if (window.testRunner) {	14 if (window.testRunner) {

14 testRunner.setCanOpenWindows();	15 testRunner.setCanOpenWindows();

15 testRunner.setCloseRemainingWindowsWhenComplete();	16 testRunner.setCloseRemainingWindowsWhenComplete();

16 testRunner.setPermission('notifications', 'granted', location.origin, location.origin);

17 }	17 }

18	18

19 async_test(function(test) {	19 async_test(function(test) {

20 if (Notification.permission != 'granted') {	20 PermissionsHelper.setPermission('notifications', 'granted').then(test. step_func(function() {
	johnme 2015/08/26 12:27:32 This test supports being run manually (notice how This test supports being run manually (notice how it checks for window.testRunner). But here you unconditionally call PermissionsHelper.setPermission, which unconditionally calls testRunner.setPermission if permission hasn't yet been set, rather than reaching the nice assert below. Ditto for the other tests that say they can be run manually. mlamouri (slow - plz ping) 2015/08/31 16:31:42 In order to run manually this test requires the pe Show quoted text On 2015/08/26 at 12:27:32, johnme wrote: > This test supports being run manually (notice how it checks for window.testRunner). But here you unconditionally call PermissionsHelper.setPermission, which unconditionally calls testRunner.setPermission if permission hasn't yet been set, rather than reaching the nice assert below. > > Ditto for the other tests that say they can be run manually. In order to run manually this test requires the permission to be granted before the test runs. If you do so, testRunner from PermissionHelpers.setPermission() will not be called. AFAICT, the behaviour is unchanged. johnme 2015/09/02 10:28:24 Sort of; if you forget to grant notification permi Show quoted text On 2015/08/31 16:31:42, Mounir Lamouri wrote: > On 2015/08/26 at 12:27:32, johnme wrote: > > This test supports being run manually (notice how it checks for > window.testRunner). But here you unconditionally call > PermissionsHelper.setPermission, which unconditionally calls > testRunner.setPermission if permission hasn't yet been set, rather than reaching > the nice assert below. > > > > Ditto for the other tests that say they can be run manually. > > In order to run manually this test requires the permission to be granted before > the test runs. If you do so, testRunner from PermissionHelpers.setPermission() > will not be called. AFAICT, the behaviour is unchanged. Sort of; if you forget to grant notification permission in advance, you used to get a clear "No permission has been granted for displaying notifications." assert, whereas now you get "Uncaught ReferenceError: testRunner is not defined". Perhaps PermissionsHelper could reject the promise in such cases with "Could not set permission ${permissionName} because testRunner is not available. If running this test manually, perhaps you need to set permissions beforehand?"
21 assert_unreached('No permission has been granted for displaying no tifications.');	21 if (Notification.permission != 'granted') {

22 return;	22 assert_unreached('No permission has been granted for displayin g notifications.');

23 }	23 return;

	24 }

24	25

25 window.addEventListener('focus', function() {	26 window.addEventListener('focus', test.step_func(function() {

26 test.done();	27 test.done();

27 });	28 }));

28	29

29 var childWindow = window.open('about:blank');	30 var childWindow = window.open('about:blank');

30 childWindow.focus();	31 childWindow.focus();

31	32

32 var notification = new Notification('My Notification');	33 var notification = new Notification('My Notification');

33 notification.addEventListener('show', function() {	34 notification.addEventListener('show', test.step_func(function() {

34 if (window.testRunner)	35 if (window.testRunner)

35 testRunner.simulateWebNotificationClick('My Notification');	36 testRunner.simulateWebNotificationClick('My Notification') ;

36 });	37 }));

37	38

38 notification.addEventListener('click', function() {	39 notification.addEventListener('click', test.step_func(function() {

39 window.focus();	40 window.focus();

40 });	41 }));

41	42

42 notification.addEventListener('error', function() {	43 notification.addEventListener('error', function() {
	johnme 2015/08/26 12:27:32 Why is this one not a test.step_func? Why is this one not a test.step_func? mlamouri (slow - plz ping) 2015/08/31 16:31:42 Because it doesn't need to be called. Show quoted text On 2015/08/26 at 12:27:32, johnme wrote: > Why is this one not a test.step_func? Because it doesn't need to be called.
43 assert_unreached('The error event should not be thrown.');	44 assert_unreached('The error event should not be thrown.');

44 });	45 });

45	46 }));

46 }, 'Clicking on a notification enables it to focus the window it was creat ed from.');	47 }, 'Clicking on a notification enables it to focus the window it was creat ed from.');

47 </script>	48 </script>

48 <script src="resources/click-focus-test.js"></script>	49 <script src="resources/click-focus-test.js"></script>

49 </body>	50 </body>

50 </html>	51 </html>

OLD	NEW