Bug 240579

Summary:	PROGRESSION (r294272): Correction to guard in Platform file caused multiple tests failures
Product:	WebKit	Reporter:	Karl Rackler <rackler>
Component:	New Bugs	Assignee:	Karl Rackler <rackler>
Status:	REOPENED ---
Severity:	Normal	CC:	bfulgham, jenner, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority:	P2	Keywords:	InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
See Also:	https://bugs.webkit.org/show_bug.cgi?id=240469

Description Karl Rackler 2022-05-18 08:30:18 PDT

Description:
http/tests/workers/service/shownotification-allowed-document.html
http/tests/workers/service/shownotification-allowed.html
http/tests/workers/service/shownotification-denied.html
http/tests/workers/service/shownotification-invalid-data.html

The first failure I saw on the dashboard was on 5/16/2022 at r294279. r294272 changed !defined(ENABLE_NOTIFICATIONS) from (PLATFORM(MAC) || PLATFORM(IOS)) to (PLATFORM(MAC) only, which seems to have started the issue.

REPRODUCTION STEPS
I can reproduce this on r294280, but unable to reproduce it on 294264 or earlier.  I cannot isolate and repro down to the commit that introduced the change due to a lack of commits to test with in the 294264-294280 range.   

Command: 
run-webkit-tests --root 294280 --ios-simulator --exit-after-n-failures 1 --exit-after-n-crashes-or-timeouts 1 --iterations 50 --no-retry http/tests/workers/service/shownotification-invalid-data.html

Result: 
Regressions: Unexpected text-only failures (1)
  http/tests/workers/service/shownotification-invalid-data.html [ Failure ]

History:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&test=http%2Ftests%2Fworkers%2Fservice%2Fshownotification-allowed-document.html&test=http%2Ftests%2Fworkers%2Fservice%2Fshownotification-allowed.html&test=http%2Ftests%2Fworkers%2Fservice%2Fshownotification-denied.html&test=http%2Ftests%2Fworkers%2Fservice%2Fshownotification-invalid-data.html&platform=ios&limit=50000

Diff:
http/tests/workers/service/shownotification-allowed-document.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/workers/service/shownotification-allowed-document-diff.txt

http/tests/workers/service/shownotification-allowed.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/workers/service/shownotification-allowed-diff.txt

http/tests/workers/service/shownotification-denied.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/workers/service/shownotification-denied-diff.txt

http/tests/workers/service/shownotification-invalid-data.html 
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/workers/service/shownotification-invalid-data-diff.txt

Comment 1 Radar WebKit Bug Importer 2022-05-18 08:30:38 PDT

<rdar://problem/93504716>

Comment 2 Karl Rackler 2022-05-18 08:40:55 PDT

I have marked this test as a consistent failure while this issue is investigated.

Comment 3 Karl Rackler 2022-05-18 08:44:38 PDT

Pull request: https://github.com/WebKit/WebKit/pull/724

Comment 4 EWS 2022-05-18 08:50:15 PDT

Test gardening commit r294398 (250692@main): <https://commits.webkit.org/250692@main>

Reviewed commits have been landed. Closing PR #724 and removing active labels.

Comment 5 Karl Rackler 2022-05-18 09:42:34 PDT

These tests are also affected by r294272, causing them to consistently timeout.

Description:
http/tests/workers/service/getnotifications-stop.html
http/tests/workers/service/getnotifications.html

History:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=http%2Ftests%2Fworkers%2Fservice%2Fgetnotifications-stop.html&test=http%2Ftests%2Fworkers%2Fservice%2Fgetnotifications.html&limit=50000

Diff:
http/tests/workers/service/getnotifications-stop.html
--- /Volumes/Data/worker/Apple-iPadOS-15-Simulator-Release-WK2-Tests/build/layout-test-results/http/tests/workers/service/getnotifications-stop-expected.txt
+++ /Volumes/Data/worker/Apple-iPadOS-15-Simulator-Release-WK2-Tests/build/layout-test-results/http/tests/workers/service/getnotifications-stop-actual.txt
@@ -1,57 +1,9 @@
+FAIL: Timed out waiting for notifyDone to be called
+
 This tests that persistent notifications created by a service worker are visible to getNotifications() even after Notification objects are stopped
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS Notification.permission is "granted"
-Got notifications
-There are 0 notifications
+FAIL Notification.permission should be granted. Threw exception ReferenceError: Can't find variable: Notification
 
-Notification was shown
-Notification was shown
-Notification was shown
-Got notifications
-There are 3 notifications
-Title: Hello
-Body: Body1
-Tag: tag-a
-Data: null
-Title: There
-Body: Body2
-Tag: tag-b
-Data: null
-Title: Buddy
-Body: Body3
-Tag: tag-b
-Data: Data3
-
-Got notifications
-There are 1 notifications
-Title: Hello
-Body: Body1
-Tag: tag-a
-Data: null
-
-Got notifications
-There are 2 notifications
-Title: There
-Body: Body2
-Tag: tag-b
-Data: null
-Title: Buddy
-Body: Body3
-Tag: tag-b
-Data: Data3
-
-Loading iframe
-Get notifications from iframe
-Remove iframes to stop notifications
-Retrieving notifications from page registration object - 3
-Notification: Hello / Body1 / tag-a / null
-Notification: There / Body2 / tag-b / null
-Notification: Buddy / Body3 / tag-b / Data3
-Retrieving notifications from page registration object - end
-PASS successfullyParsed is true
-
-TEST COMPLETE
-

http/tests/workers/service/getnotifications.html
--- /Volumes/Data/worker/Apple-iPadOS-15-Simulator-Release-WK2-Tests/build/layout-test-results/http/tests/workers/service/getnotifications-expected.txt
+++ /Volumes/Data/worker/Apple-iPadOS-15-Simulator-Release-WK2-Tests/build/layout-test-results/http/tests/workers/service/getnotifications-actual.txt
@@ -1,54 +1,9 @@
+FAIL: Timed out waiting for notifyDone to be called
+
 This tests that persistent notifications created by a service worker are visible to getNotifications()
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS Notification.permission is "granted"
-Got notifications
-There are 0 notifications
+FAIL Notification.permission should be granted. Threw exception ReferenceError: Can't find variable: Notification
 
-Notification was shown
-Notification was shown
-Notification was shown
-Got notifications
-There are 3 notifications
-Title: Hello
-Body: Body1
-Tag: tag-a
-Data: null
-Title: There
-Body: Body2
-Tag: tag-b
-Data: null
-Title: Buddy
-Body: Body3
-Tag: tag-b
-Data: Data3
-
-Got notifications
-There are 1 notifications
-Title: Hello
-Body: Body1
-Tag: tag-a
-Data: null
-
-Got notifications
-There are 2 notifications
-Title: There
-Body: Body2
-Tag: tag-b
-Data: null
-Title: Buddy
-Body: Body3
-Tag: tag-b
-Data: Data3
-
-Retrieving notifications from page registration object - 3
-Notification: Hello / Body1 / tag-a / null
-Notification: There / Body2 / tag-b / null
-Notification: Buddy / Body3 / tag-b / Data3
-Retrieving notifications from page registration object - end
-PASS successfullyParsed is true
-
-TEST COMPLETE
-

I have marked this test as a timeout while this issue is investigated.

Comment 6 Karl Rackler 2022-05-18 09:53:35 PDT

Pull request: https://github.com/WebKit/WebKit/pull/731

Comment 7 EWS 2022-05-18 09:56:16 PDT

Test gardening commit r294400 (250694@main): <https://commits.webkit.org/250694@main>

Reviewed commits have been landed. Closing PR #731 and removing active labels.

Comment 8 Robert Jenner 2022-05-18 10:47:31 PDT

Pull request: https://github.com/WebKit/WebKit/pull/736

Comment 9 EWS 2022-05-18 10:59:45 PDT

Test gardening commit r294406 (250700@main): <https://commits.webkit.org/250700@main>

Reviewed commits have been landed. Closing PR #736 and removing active labels.

Comment 10 Robert Jenner 2022-05-18 11:35:41 PDT

I have disabled these tests for iOS as mentioned in this thread: 
https://a1391192.slack.com/archives/GK0ATJ6FJ/p1652893991995359

I believe that should resolve this bug.

Comment 11 Karl Rackler 2022-05-18 14:57:35 PDT

This also affects the following: 

Description:
http/tests/push-api/subscribe-default-permissions-iframe-cross-origin.html
http/tests/push-api/subscribe-default-permissions-iframe-same-origin.html
http/tests/push-api/subscribe-default-permissions.html
http/tests/push-api/subscribe-deny-permissions-on-prompt.html
http/tests/push-api/subscribe-grant-permissions.html

History:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&test=http%2Ftests%2Fpush-api%2Fsubscribe-default-permissions-iframe-cross-origin.html&test=http%2Ftests%2Fpush-api%2Fsubscribe-default-permissions-iframe-same-origin.html&test=http%2Ftests%2Fpush-api%2Fsubscribe-default-permissions.html&test=http%2Ftests%2Fpush-api%2Fsubscribe-deny-permissions-on-prompt.html&test=http%2Ftests%2Fpush-api%2Fsubscribe-grant-permissions.html&platform=ios&limit=50000

Diff:
http/tests/push-api/subscribe-default-permissions-iframe-cross-origin.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/push-api/subscribe-default-permissions-iframe-cross-origin-diff.txt

http/tests/push-api/subscribe-default-permissions-iframe-same-origin.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/push-api/subscribe-default-permissions-iframe-same-origin-diff.txt

http/tests/push-api/subscribe-default-permissions.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/push-api/subscribe-default-permissions-diff.txt

http/tests/push-api/subscribe-deny-permissions-on-prompt.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/push-api/subscribe-deny-permissions-on-prompt-diff.txt

http/tests/push-api/subscribe-grant-permissions.html
https://build.webkit.org/results/Apple-iPadOS-15-Simulator-Release-WK2-Tests/250623@main%20(1751)/http/tests/push-api/subscribe-grant-permissions-diff.txt

Skipping tests affected in this bug for iOS per guidance.

Comment 12 Karl Rackler 2022-05-18 15:10:11 PDT

Re-opening for pull request https://github.com/WebKit/WebKit/pull/749

Comment 13 EWS 2022-05-18 15:17:32 PDT

Test gardening commit r294441 (250717@main): <https://commits.webkit.org/250717@main>

Reviewed commits have been landed. Closing PR #749 and removing active labels.

Comment 14 Karl Rackler 2022-05-20 13:14:49 PDT

This also affects the following: 

Description:
http/wpt/push-api/pushEvent.any.serviceworker.html
http/wpt/push-api/pushManager.any.html
http/wpt/push-api/pushManager.any.serviceworker.html

History:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&test=http%2Fwpt%2Fpush-api%2FpushEvent.any.serviceworker.html&test=http%2Fwpt%2Fpush-api%2FpushManager.any.html&test=http%2Fwpt%2Fpush-api%2FpushManager.any.serviceworker.html&platform=ios&limit=50000

Diff:
https://build.webkit.org/results/Apple-iOS-15-Simulator-Release-WK2-Tests/250800@main%20(2672)/http/wpt/push-api/pushEvent.any.serviceworker-diff.txt

https://build.webkit.org/results/Apple-iOS-15-Simulator-Release-WK2-Tests/250800@main%20(2672)/http/wpt/push-api/pushManager.any-diff.txt
https://build.webkit.org/results/Apple-iOS-15-Simulator-Release-WK2-Tests/250800@main%20(2672)/http/wpt/push-api/pushManager.any.serviceworker-diff.txt

Per the guidance, skip these tests.

Comment 15 Karl Rackler 2022-05-20 13:18:27 PDT

Re-opening for pull request https://github.com/WebKit/WebKit/pull/849

Comment 16 EWS 2022-05-20 13:21:40 PDT

Test gardening commit r294571 (250812@main): <https://commits.webkit.org/250812@main>

Reviewed commits have been landed. Closing PR #849 and removing active labels.