Summary: | [Qt] Crash in gmail on enabling desktop notifications | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | jingdow | ||||||||||||
Component: | New Bugs | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Major | CC: | allan.jensen, jturcotte, nowrep | ||||||||||||
Priority: | P3 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 88186, 103747, 107438 | ||||||||||||||
Attachments: |
|
Description
jingdow
2013-01-11 14:07:12 PST
I've built qtwebkit with "bool NotificationPresenterClientQt::dumpNotification" set to "true" in /Source/WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.cpp and I get in console output: DESKTOP NOTIFICATION PERMISSION REQUESTED: https://mail.google.com Created attachment 182426 [details]
gdb trace log from QtTestBrowser
Another thing I've noticed. In file /Source/WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.cpp in function void NotificationPresenterClientQt::requestPermission(ScriptExecutionContext* context, PassRefPtr<VoidCallback> callback) I've added two printf commands, like this: printf("Before emited signal\n"); emit toPage(context)->featurePermissionRequested(toFrame(context), QWebPage::Notifications); printf("After emited signal\n"); Upon crash, only the first one gets printed ("Before emited signal"). Probably doesn't mean anything, but just in case... Created attachment 183556 [details]
Check the validity of ptr right before handling callback.
Created attachment 183557 [details]
Don't add invalid pointer into to m_callbacks
From w3.org: static void requestPermission(NotificationPermissionCallback callback); There is one parameter to requestPermission method. However, if this method is called without it, the Callback pointer that is passed to NotificationPresenterClientQt will be invalid and finally will lead to this crash. The simple "window.webkitNotifications.requestPermission();" should crash it. I've added two patches, the first checks the validity of callback pointer right before using it, while the second rather prevents adding invalid callback pointer into m_callbacks array. I'd prefer the behavior of the second patch. Both patches fixes the crash and enables desktop notifications on gmail.com Confirmed, patches fix the problem. N(In reply to comment #7) > Confirmed, patches fix the problem. Nice, looks like it might apply to trunk as well. I will test on Monday. Created attachment 183747 [details]
Patch
Committed r140322: <http://trac.webkit.org/changeset/140322> |