[webkit-reviews] review denied: [Bug 42534] Crash in Notification::disconnectFrame() triggered by Frame::lifeSupportTimerFired() : [Attachment 62209] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 21 10:47:32 PDT 2010


Darin Adler <darin at apple.com> has denied Yael <yael.aharon at nokia.com>'s request
for review:
Bug 42534: Crash in Notification::disconnectFrame() triggered by
Frame::lifeSupportTimerFired()
https://bugs.webkit.org/show_bug.cgi?id=42534

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +    // Due to the misterious bug
http://code.google.com/p/chromium/issues/detail?id=49323.

Spelling error here: "misterious".

> +void DOMWindow::disconnectNotifications()

This function should be named after when it's called, the event the DOMWindow
is learning about, rather than what it does. I think pageDestroyed is a fine
name for it. It should not be notifications-specific. The body can be, but the
function itself should not.

> +#if ENABLE(NOTIFICATIONS)
> +    if (m_domWindow)
> +	   m_domWindow->disconnectNotifications();
> +#endif

Then this could be:

    if (m_domWindow)
	m_domWindow->pageDestroyed();

Please make those changes.


More information about the webkit-reviews mailing list