Created attachment 52930 [details] fix to check for null pointer Qt-Symbian applications which use Qtwebkit and load Netscape plugins currently crashes on exit. The crash has been narrowed down to: void PluginView::platformDestroy() { QWebPageClient* client = m_parentFrame->view()->hostWindow()->platformPageClient(); if (qobject_cast<QGraphicsWebView*>(client->pluginParent())) // Crashes here delete static_cast<PluginContainerSymbian*>(platformPluginWidget())->proxy(); else delete platformPluginWidget(); } The crash is caused by a NULL client*. qobject_cast causes an exception as it requires a valid QObject pointer to access underlying Qtmetadata.
Created attachment 52932 [details] Alternative patch The plugin view in symbian can only be of a type PluginContainerSymbian. The proxy object is initialized to NULL or a QGraphicsProxyWidget*. There is no harm deleting NULLs, so there is no need to do the qobject_cast. If the platform widget pointer is not null, it should be safe to delete both the proxy and the widget itself.
Attachment 52932 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/plugins/symbian/PluginViewSymbian.cpp:417: Missing space before ( in if( [whitespace/parens] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 52932 [details] Alternative patch > + if( PlatformPluginWidget() ) { This is not going to compile because of the uppercase P.
Committed r57333: <http://trac.webkit.org/changeset/57333>
(In reply to comment #0) > Created an attachment (id=52930) [details] > fix to check for null pointer Cherry-picked r51006 into qtwebkit-4.6 with commit 14feb62c96ffe2c37e3e2fdac4e370fdbc76ef62
Revision r57333 cherry-picked into qtwebkit-2.0 with commit e3b79791d73b624eceebbfbbfd6d6752a532b390
(In reply to comment #3) > (From update of attachment 52932 [details]) > > > + if( PlatformPluginWidget() ) { > > This is not going to compile because of the uppercase P. Thanks! Good catch. Funny enough RVCT still compiles and works on the device. I think we are leaking the container now with the landed patch. Both the proxy and the container are new'ed but the if fix will only delete the proxy. This leaks the container. Will submit an updated fix once i test out a window'ed plugin to make sure it is working.
Created attachment 53005 [details] Patch Updated alternative patch. It will now properly delete the proxy and the container objects.
Tested the code with window'ed and windowless plugins. For window'ed plugins the current fix will leak the container. Updated the patch with Simon's comment for review.
Comment on attachment 53005 [details] Patch Fixes memory leak.
changing on behalf of David
Created attachment 53191 [details] SVN up'ed to latest webkit to fix auto check errors
Comment on attachment 52930 [details] fix to check for null pointer Clearing review as this attachment has been landed
As a side note: Due to the use of QGraphicsProxyWidget this is going to be horribly slow on Symbian and generally mobile platforms. Hopefully the flash plugin on the other side will default to windowless painting, otherwise this isn't going to be actually usable.
Comment on attachment 53191 [details] SVN up'ed to latest webkit to fix auto check errors Marking it to cq+ for landing.
Comment on attachment 53191 [details] SVN up'ed to latest webkit to fix auto check errors Clearing flags on attachment: 53191 Committed r58035: <http://trac.webkit.org/changeset/58035>
All reviewed patches have been landed. Closing bug.
Revision r58035 cherry-picked into qtwebkit-2.0 with commit 7513c8718ac700b5fa7f58cb459f76fadd526df3
Reopening for a potential double-deallocation case when a page is destroyed which had a plugin loaded. It seems that the container is deleted twice, once with delete container->proxy(); and then by calling delete container. Abhinav promised to have a patch.
Fix committed to trunk as http://trac.webkit.org/changeset/62159. We should seriously consider this patch for the 4.6 and 2.0 branches as well.
*** Bug 41366 has been marked as a duplicate of this bug. ***
<cherry-pick-for-backport: r62159>
Revision r62159 cherry-picked into qtwebkit-2.0 with commit 5bddb0b27458a4ac655ca9527393b41a729d3717