Bug 238498

Summary: Implement persistent notification handling
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, beidson, cdumez, ews-watchlist, gyuyoung.kim, nham, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Description youenn fablet 2022-03-29 02:31:16 PDT 
Implement persistent notification handling
Comment 2 Radar WebKit Bug Importer 2022-03-29 02:33:46 PDT 
<rdar://problem/90972545>
Comment 3 youenn fablet 2022-03-29 02:52:25 PDT 
Created attachment 456009 [details]
Patch
Comment 4 youenn fablet 2022-03-29 04:19:42 PDT 
Created attachment 456014 [details]
Patch
Comment 5 youenn fablet 2022-03-29 13:07:01 PDT 
Comment on attachment 456014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456014&action=review

> Source/WebCore/Modules/notifications/Notification.cpp:69
> +    notification->showSoon();

Looking at it again, calling showSoon should be removed. Probably worth moving showSoon call out of the Notification::create object.
Comment 6 Brady Eidson 2022-03-29 13:19:12 PDT 
Comment on attachment 456014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456014&action=review

>> Source/WebCore/Modules/notifications/Notification.cpp:69
>> +    notification->showSoon();
> 
> Looking at it again, calling showSoon should be removed. Probably worth moving showSoon call out of the Notification::create object.

Definitely should be: In this case we're not showing it.

Agree with the refactor.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2332
>  
> +

stray newline
Comment 7 youenn fablet 2022-03-30 02:33:19 PDT 
Created attachment 456104 [details]
Patch for landing
Comment 8 youenn fablet 2022-03-30 03:06:46 PDT 
> > Looking at it again, calling showSoon should be removed. Probably worth moving showSoon call out of the Notification::create object.
> 
> Definitely should be: In this case we're not showing it.
> 
> Agree with the refactor.

I needed to do a few more refactoring since now we are not reusing the already created Notification object but a new one that should have a matching identifier.
Comment 9 youenn fablet 2022-03-30 05:21:45 PDT 
Created attachment 456114 [details]
Patch for landing
Comment 10 EWS 2022-03-30 11:41:33 PDT 
Committed r292110 (249029@main): <https://commits.webkit.org/249029@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456114 [details].