Kling, I applied the patch from https://bugs.webkit.org/show_bug.cgi?id=41065, but the Qt runtime tests were still crashing. After disabling the 3 tests: getSetDynamicProperty(), getSetDynamicProperty_data(), and getSetStaticProperty() I get 2 unrelated XFail, but no crash.
Can you please check if you had other code changes in your environment that I should apply to mine?
thanks,

Created attachment 62184[details]
Patch
Call NotificationsCenter::disconnectFrame when a frame is detached from its page. We want to cancel any outstanding premision requests at that point.
Since I could not reproduce the crash, I did not add tests at this time, but there is a new test for canceling notifications requests in https://bugs.webkit.org/show_bug.cgi?id=41413 .

+ if (!presenter())
+ return;
m_notificationPresenter->cancelRequestsForPermission(m_scriptExecutionContext);
Is this null check still needed with other changes?
It would probably be better to use m_notificationPresenter consistently - it's not immediately obvious if presenter() is the same thing.

(In reply to comment #7)
> + if (!presenter())
> + return;
> m_notificationPresenter->cancelRequestsForPermission(m_scriptExecutionContext);
>
> Is this null check still needed with other changes?
>
I think it is safe to add this check, especially since I don't have exact steps to reproduce the error.
> It would probably be better to use m_notificationPresenter consistently - it's not immediately obvious if presenter() is the same thing.
I agree, I'll update the patch.

Created attachment 62193[details]
Patch
Added comment about the NULL check, per comment #10.
I am reluctant to add an ASSERT here, because if m_notificationPresenter will be NULL, Chromium will continue to crash and we will continue to get crash reports.
I assume these crash reports are generated from debug builds, not release builds, so the ASSERT will be hit.

> I assume these crash reports are generated from debug builds, not release builds, so the ASSERT will be hit.
This sounds surprising - I never heard that any vendor shipped a debug build of WebKit to users.

(In reply to comment #13)
> > I assume these crash reports are generated from debug builds, not release builds, so the ASSERT will be hit.
>
> This sounds surprising - I never heard that any vendor shipped a debug build of WebKit to users.
I don't know where these crash logs come from. Maybe someone from Chromium project could tell us.
However, if you look at all other functions of NotificationCenter, there is a NULL check in each one of them, so I don't understand why it is a problem to have a NULL check in this new function as well?

I don't really like the name DOMWindow::clearNotificationRequests. The method does more than that, since it also sets the NotificationCenter to 0. Perhaps 'disconnectNotifications()' would be more specific.

(In reply to comment #14)
> (In reply to comment #13)
> > > I assume these crash reports are generated from debug builds, not release builds, so the ASSERT will be hit.
> >
> > This sounds surprising - I never heard that any vendor shipped a debug build of WebKit to users.
>
> I don't know where these crash logs come from. Maybe someone from Chromium project could tell us.
> However, if you look at all other functions of NotificationCenter, there is a NULL check in each one of them, so I don't understand why it is a problem to have a NULL check in this new function as well?
The crash reports are from release builds, not debug builds, so I think that the ASSERT would be fine.

Comment on attachment 62209[details]
Patch
> + // 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.