Bug 77363 - Clear shown notifications when context is no longer active
Summary: Clear shown notifications when context is no longer active
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.7
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-01-30 13:16 PST by Jon Lee
Modified: 2012-02-02 14:17 PST (History)
3 users (show)

See Also:


Attachments
Patch (35.74 KB, patch)
2012-01-30 14:06 PST, Jon Lee
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2012-01-30 13:16:09 PST
When the user navigates away from that page, or closes the tab or window associated with that page, or quits the application altogether, all of the page's associated notifications could be cleared depending on the implementation.

<rdar://problem/10568907>
Comment 1 Jon Lee 2012-01-30 14:06:22 PST
Created attachment 124598 [details]
Patch
Comment 2 Darin Adler 2012-02-01 14:52:37 PST
Comment on attachment 124598 [details]
Patch

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

> Source/WebCore/notifications/NotificationPresenter.h:62
> +    virtual void clearNotifications(ScriptExecutionContext*) = 0;

We could consider providing a default empty implementation. Then we would only need to override this for platforms as they implement it, and the patch would be a lot smaller. Unless we think people will really want to implement this on every platform.

> Source/WebCore/page/DOMWindow.cpp:731
> +    m_notifications->clearNotifications();
>      m_notifications->disconnectFrame();

Could we just add the code to disconnectFrame instead of adding a new clearNotifications function?
Comment 3 Jon Lee 2012-02-02 12:45:35 PST
(In reply to comment #2)
> (From update of attachment 124598 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124598&action=review
> 
> > Source/WebCore/notifications/NotificationPresenter.h:62
> > +    virtual void clearNotifications(ScriptExecutionContext*) = 0;
> 
> We could consider providing a default empty implementation. Then we would only need to override this for platforms as they implement it, and the patch would be a lot smaller. Unless we think people will really want to implement this on every platform.

Done.
> 
> > Source/WebCore/page/DOMWindow.cpp:731
> > +    m_notifications->clearNotifications();
> >      m_notifications->disconnectFrame();
> 
> Could we just add the code to disconnectFrame instead of adding a new clearNotifications function?

Yes, that makes sense.
Thanks!
Comment 4 Jon Lee 2012-02-02 14:17:21 PST
Committed r106592: <http://trac.webkit.org/changeset/106592>