1928702 - Reimplement showNotification() via IPC

Status: RESOLVED FIXED

(Core :: DOM: Notifications, task)

Description

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Because I found that focusing on showNotification() reduces a lot of complexity in bug 1891807. As the patch stack there proved that the IPC design works for both, we can implement one without worrying that the design won't match another.

Comment 1

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 1: Add skeleton PNotification r=asuth

Comment 2

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 2: Create PNotification via PBackground r=asuth

Comment 3

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 3: Move more functions to NotificationUtils r=asuth

Moving functions that can be called from both Notification and IPC actors, that are needed for ActorDestroy.

Comment 4

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 4: Implement ActorDestroy r=asuth

Comment 5

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 5: Implement RecvShow r=asuth

Comment 6

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 6: Move NotificationObserver functions to NotificationUtils r=asuth

Comment 7

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 7: Settle ShowResolver r=asuth

Comment 8

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 8: Implement permission check r=asuth

Comment 9

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 9: Implement alertclickcallback behavior r=asuth

Comment 10

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 10: Implement alertfinished behavior r=asuth

Comment 11

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 11: Migrate ShowPersistentNotification to use IPC r=asuth

Comment 12

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 12: Add extra coverage for Glean metrics r=asuth

Comment 13

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

macOS specific WPT failures: https://treeherder.mozilla.org/jobs?repo=try&test_paths=dom%2Fnotification&test_paths=notifications&revision=7f96ff02125f9c03a35c6ed467c9ee8d1de341eb&selectedTaskRun=fOiVUm_aQFWrhW1_4odgxg.0

(Caused by Close() doing it's unregister via ActorDestroy when called via alertfinished, but at that point it's already being unregistered. Skip unregister somehow while still doing unpersist.)

New: https://treeherder.mozilla.org/jobs?repo=try&revision=a4f525b9610c6180b27dd3efa5f0d7a12b631edc

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93937caf6d13
Part 1: Add skeleton PNotification r=asuth
https://hg.mozilla.org/integration/autoland/rev/bb63792c3333
Part 2: Create PNotification via PBackground r=asuth
https://hg.mozilla.org/integration/autoland/rev/984f6e687ff5
Part 3: Move more functions to NotificationUtils r=asuth
https://hg.mozilla.org/integration/autoland/rev/5a94b945a3ad
Part 4: Implement ActorDestroy r=asuth
https://hg.mozilla.org/integration/autoland/rev/a497ddb030f6
Part 5: Implement RecvShow r=asuth
https://hg.mozilla.org/integration/autoland/rev/3daeacd08336
Part 6: Move NotificationObserver functions to NotificationUtils r=asuth
https://hg.mozilla.org/integration/autoland/rev/f318fe78ff14
Part 7: Settle ShowResolver r=asuth
https://hg.mozilla.org/integration/autoland/rev/56ab49acff35
Part 8: Implement permission check r=asuth
https://hg.mozilla.org/integration/autoland/rev/33642b1c547b
Part 9: Implement alertclickcallback behavior r=asuth
https://hg.mozilla.org/integration/autoland/rev/622a66cdac2e
Part 10: Implement alertfinished behavior r=asuth
https://hg.mozilla.org/integration/autoland/rev/984db15bc9e2
Part 11: Migrate ShowPersistentNotification to use IPC r=asuth
https://hg.mozilla.org/integration/autoland/rev/1bb3c36a15c3
Part 12: Add extra coverage for Glean metrics r=asuth

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49035 for changes under testing/web-platform/tests

Comment 16

[Norisz Fay [:noriszfay]]

Backed out for causing gv-junit failures on OpenWindowTest#openWindowNullResult

Backout link

Push with failures

Failure log

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 18

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1928702 - Part 13: Adjust junit test to delegate before clicking notification r=Roger,#geckoview-reviewers

Now that showNotification resolves from parent process, adding delegate after a resolved showNotification is too late.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc25e5c94813
Part 1: Add skeleton PNotification r=asuth
https://hg.mozilla.org/integration/autoland/rev/1f3365d25033
Part 2: Create PNotification via PBackground r=asuth
https://hg.mozilla.org/integration/autoland/rev/22abfa77d4fc
Part 3: Move more functions to NotificationUtils r=asuth
https://hg.mozilla.org/integration/autoland/rev/807f1d1ada66
Part 4: Implement ActorDestroy r=asuth
https://hg.mozilla.org/integration/autoland/rev/fca3921d09a5
Part 5: Implement RecvShow r=asuth
https://hg.mozilla.org/integration/autoland/rev/bde8ca8fd963
Part 6: Move NotificationObserver functions to NotificationUtils r=asuth
https://hg.mozilla.org/integration/autoland/rev/0f64abe35d62
Part 7: Settle ShowResolver r=asuth
https://hg.mozilla.org/integration/autoland/rev/10bb5f41f44b
Part 8: Implement permission check r=asuth
https://hg.mozilla.org/integration/autoland/rev/11a3c2c86a8e
Part 9: Implement alertclickcallback behavior r=asuth
https://hg.mozilla.org/integration/autoland/rev/c942992fa981
Part 10: Implement alertfinished behavior r=asuth
https://hg.mozilla.org/integration/autoland/rev/bda570180adf
Part 11: Migrate ShowPersistentNotification to use IPC r=asuth
https://hg.mozilla.org/integration/autoland/rev/3c174ea10f04
Part 12: Add extra coverage for Glean metrics r=asuth

Comment 20

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/cc25e5c94813
https://hg.mozilla.org/mozilla-central/rev/1f3365d25033
https://hg.mozilla.org/mozilla-central/rev/22abfa77d4fc
https://hg.mozilla.org/mozilla-central/rev/807f1d1ada66
https://hg.mozilla.org/mozilla-central/rev/fca3921d09a5
https://hg.mozilla.org/mozilla-central/rev/bde8ca8fd963
https://hg.mozilla.org/mozilla-central/rev/0f64abe35d62
https://hg.mozilla.org/mozilla-central/rev/10bb5f41f44b
https://hg.mozilla.org/mozilla-central/rev/11a3c2c86a8e
https://hg.mozilla.org/mozilla-central/rev/c942992fa981
https://hg.mozilla.org/mozilla-central/rev/bda570180adf
https://hg.mozilla.org/mozilla-central/rev/3c174ea10f04

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged