[webkit-reviews] review granted: [Bug 77363] Clear shown notifications when context is no longer active : [Attachment 124598] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jon Lee <jonlee at apple.com>'s request
for review:
Bug 77363: Clear shown notifications when context is no longer active
https://bugs.webkit.org/show_bug.cgi?id=77363

Attachment 124598: Patch
https://bugs.webkit.org/attachment.cgi?id=124598&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
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?