Bug 171487

Summary: Remove support for legacy Notifications
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jonlee, ryanhaddad
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 171714    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
jonlee: review+
Patch none

Sam Weinig
Reported 2017-04-30 15:28:50 PDT
Remove support for legacy Notifications
Attachments
Patch (313.77 KB, patch)
2017-04-30 18:02 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (313.77 KB, patch)
2017-04-30 18:28 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (244.65 KB, patch)
2017-04-30 19:13 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (245.70 KB, patch)
2017-05-01 12:05 PDT, Sam Weinig 		jonlee: review+
DetailsFormatted DiffDiff
Patch (310.84 KB, patch)
2017-05-09 17:17 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2017-04-30 18:02:33 PDT
Created attachment 308703 [details] Patch
Sam Weinig
Comment 2 2017-04-30 18:04:02 PDT
I discussed this briefly with Jon Lee in the passed, and he was not against the idea on the condition that gmail's use of local notifications still worked. I checked, and gmail's use of local notifications works just fine without legacy notifications.
Sam Weinig
Comment 3 2017-04-30 18:28:48 PDT
Created attachment 308704 [details] Patch
Sam Weinig
Comment 4 2017-04-30 18:29:40 PDT
First patch didn't apply, but I am thinking the tools may be broken again, and not working for removed files :(. Maybe this second upload will prove me wrong.
Sam Weinig
Comment 5 2017-04-30 18:34:04 PDT
(In reply to Sam Weinig from comment #4) > First patch didn't apply, but I am thinking the tools may be broken again, > and not working for removed files :(. Maybe this second upload will prove me > wrong. Bummer.
Sam Weinig
Comment 6 2017-04-30 18:36:06 PDT
Filed https://bugs.webkit.org/show_bug.cgi?id=171488 to track the patch applying issue.
Sam Weinig
Comment 7 2017-04-30 19:13:47 PDT
Created attachment 308706 [details] Patch
Sam Weinig
Comment 8 2017-04-30 19:14:31 PDT
Attempting workout by not removing any directories, only the files within. Will need second pass to remove empty directories.
Sam Weinig
Comment 9 2017-05-01 12:05:39 PDT
Created attachment 308743 [details] Patch
Sam Weinig
Comment 10 2017-05-04 13:56:03 PDT
Committed r216206: <http://trac.webkit.org/changeset/216206>
WebKit Commit Bot
Comment 11 2017-05-04 22:33:32 PDT
Re-opened since this is blocked by bug 171714
Ryan Haddad
Comment 12 2017-05-05 09:22:19 PDT
(In reply to WebKit Commit Bot from comment #11) > Re-opened since this is blocked by bug 171714 The rollout does appear to have stopped the LayoutTest crashes seen on the bots. Crashlog and details in: https://bugs.webkit.org/show_bug.cgi?id=171705
Sam Weinig
Comment 13 2017-05-09 17:17:07 PDT
Created attachment 309564 [details] Patch
Sam Weinig
Comment 14 2017-05-10 17:29:39 PDT
Committed r216641: <http://trac.webkit.org/changeset/216641>
Sam Weinig
Comment 15 2017-05-10 17:31:07 PDT
Re-landed with fix to use ActiveDOMObject::stop, rather than ContextDestructionObserver::contextDestroyed as the override where we unregister the notification.
Note You need to log in before you can comment on or make changes to this bug.