In UNIX port(Gtk/QT/Brackberry), m_npWindow.ws_info = new NPSetWindowCallbackStruct; is called in PluginView::platformStart(). each platform does not delete this object(m_npWindow.ws_info), and it is done in PluginView:stop(). However, If m_isWindowed is false (it means windowless mode), it's not deleted. m_npWindow.ws_info should be deleted even if plugin run as windowless mode to remove memory leak. Every plugin works well after this patch without any crash.
Created attachment 135281 [details] [UNIX Plugins] delete ws_info, remove memory leak patch. it's simple. i just removed a condition in the IF statement.
Created attachment 137267 [details] [UNIX Plugins] delete ws_info, remove memory leak rebase and change casting code to C++ style.
Comment on attachment 135281 [details] [UNIX Plugins] delete ws_info, remove memory leak Mark this as obsolete because there's a second patch for review.
Comment on attachment 137267 [details] [UNIX Plugins] delete ws_info, remove memory leak View in context: https://bugs.webkit.org/attachment.cgi?id=137267&action=review Looks good in general, but I think you can remove the if() altogether and there are a few language nitpicks :) > Source/WebCore/ChangeLog:3 > + [Plugins] delete ws_info regardless window mode. "regardless window mode" -> "regardless of the window mode" > Source/WebCore/ChangeLog:10 > + Additionally, change casting synta to C++ style. synta -> syntax > Source/WebCore/plugins/PluginView.cpp:370 > + if (m_npWindow.ws_info) > + delete static_cast<NPSetWindowCallbackStruct *>(m_npWindow.ws_info); I don't think the if() is needed at all. delete 0 is perfectly valid C++.
Created attachment 137298 [details] [UNIX Plugins] delete ws_info, remove memory leak change the title. revise wrong word. remove unneeded if().
Comment on attachment 137298 [details] [UNIX Plugins] delete ws_info, remove memory leak mark this as obsolete
Comment on attachment 137298 [details] [UNIX Plugins] delete ws_info, remove memory leak sorry for my mistake
Comment on attachment 137267 [details] [UNIX Plugins] delete ws_info, remove memory leak >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 1863a58..2b59ef9 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,17 @@ >+2012-04-15 Bang Kwang min <justine.bang@samsung.com> >+ >+ [Plugins] delete ws_info regardless of window mode. >+ https://bugs.webkit.org/show_bug.cgi?id=83004 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ m_npWindow.ws_info should be deleted even if plugin run as windowless mode to remove memory leak. >+ Every plugin works well after this patch without any crash. >+ Additionally, change casting syntax to C++ style. >+ >+ * plugins/PluginView.cpp: >+ (WebCore::PluginView::stop): >+ > 2012-04-15 James Robinson <jamesr@chromium.org> > > [chromium] LayerRendererChromium shouldn't know anything about CCLayerImpl >diff --git a/Source/WebCore/plugins/PluginView.cpp b/Source/WebCore/plugins/PluginView.cpp >index 749f609..389a8fe 100644 >--- a/Source/WebCore/plugins/PluginView.cpp >+++ b/Source/WebCore/plugins/PluginView.cpp >@@ -366,8 +366,8 @@ void PluginView::stop() > } > > #ifdef XP_UNIX >- if (m_isWindowed && m_npWindow.ws_info) >- delete (NPSetWindowCallbackStruct *)m_npWindow.ws_info; >+ if (m_npWindow.ws_info) >+ delete static_cast<NPSetWindowCallbackStruct *>(m_npWindow.ws_info); > m_npWindow.ws_info = 0; > #endif >
Comment on attachment 137298 [details] [UNIX Plugins] delete ws_info, remove memory leak as i think, delete 0 is the problem so, please review previous patch. i modified the title and comment.
Created attachment 137307 [details] [UNIX Plugins] delete ws_info, remove memory leak rebase. I'm sorry i'm not accustomed to manage the attachment. - if remove if(), it does not pass qt port test.
(In reply to comment #10) > Created an attachment (id=137307) [details] > [UNIX Plugins] delete ws_info, remove memory leak > > rebase. > > I'm sorry i'm not accustomed to manage the attachment. No worries, it seems you got the hang of it :) > - > if remove if(), it does not pass qt port test. Can you elaborate a bit on that?
(In reply to comment #9) > (From update of attachment 137298 [details]) > as i think, delete 0 is the problem See also http://www.parashift.com/c++-faq-lite/freestore-mgmt.html#faq-16.8
(In reply to comment #12) > (In reply to comment #9) > > (From update of attachment 137298 [details] [details]) > > as i think, delete 0 is the problem > > See also http://www.parashift.com/c++-faq-lite/freestore-mgmt.html#faq-16.8 I agree with you, and i has uploaded the patch, but it failed to pass test. please see attachment id 137298. you can see qt, qt-wk2 and efl are pink color. To know why it failed, i'm trying to build QT.
Created attachment 137433 [details] delete without null checking. delete without null checking. you can say that.
*** Bug 84021 has been marked as a duplicate of this bug. ***
Comment on attachment 137433 [details] delete without null checking. View in context: https://bugs.webkit.org/attachment.cgi?id=137433&action=review > Source/WebCore/plugins/PluginView.cpp:369 > + delete static_cast<NPSetWindowCallbackStruct *>(m_npWindow.ws_info); Small coding style glitch, space after *. I'll fix it when landing.
Committed r114358: <http://trac.webkit.org/changeset/114358>