Bug 92099 – [EFL] [WK2] Memory leak in ewk_view_resource_load_client.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Sudarsana Nagineni (babu)]

Valgrind reports the following memory leak in ewk_view_resource_load_client.cpp.

==27984== 648 bytes in 27 blocks are definitely lost in loss record 687 of 782
==27984==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27984==    by 0xA86A1CA: WTF::fastMalloc(unsigned long) (FastMalloc.cpp:268)
==27984==    by 0x549CA88: ewk_web_resource_new(char const*, bool) (FastMalloc.h:265)
==27984==    by 0x549BD22: didInitiateLoadForResource(OpaqueWKPage const*, OpaqueWKFrame const*, unsigned long, OpaqueWKURLRequest const*, bool, void const*) (ewk_view_resource_load_client.cpp:59)
==27984==    by 0x53CAE04: WebKit::WebResourceLoadClient::didInitiateLoadForResource(WebKit::WebPageProxy*, WebKit::WebFrameProxy*, unsigned long, WebCore::ResourceRequest const&, bool) (WebResourceLoadClient.cpp:43)
==27984==    by 0x538D1E6: WebKit::WebPageProxy::didInitiateLoadForResource(unsigned long, unsigned long, WebCore::ResourceRequest const&, bool) (WebPageProxy.cpp:2340)
==27984==    by 0x54B9393: void CoreIPC::callMemberFunction<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long, unsigned long, WebCore::ResourceRequest const&, bool), unsigned long, unsigned long, WebCore::ResourceRequest, bool>(CoreIPC::Arguments4<unsigned long, unsigned long, WebCore::ResourceRequest, bool> const&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long, unsigned long, WebCore::ResourceRequest const&, bool)) (HandleMessage.h:37)
==27984==    by 0x54B5ADE: void CoreIPC::handleMessage<Messages::WebPageProxy::DidInitiateLoadForResource, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long, unsigned long, WebCore::ResourceRequest const&, bool)>(CoreIPC::ArgumentDecoder*, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long, unsigned long, WebCore::ResourceRequest const&, bool)) (HandleMessage.h:302)
==27984==    by 0x54B2A95: WebKit::WebPageProxy::didReceiveWebPageProxyMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) (WebPageProxyMessageReceiver.cpp:322)
==27984==    by 0x5389A56: WebKit::WebPageProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) (WebPageProxy.cpp:1771)
==27984==    by 0x53C0A39: WebKit::WebProcessProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) (WebProcessProxy.cpp:347)
==27984==    by 0x534F08E: WebKit::WebConnectionToWebProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) (WebConnectionToWebProcess.cpp:92)
==27984==    by 0x52F939A: CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder>&) (Connection.cpp:691)
==27984==    by 0x52F9538: CoreIPC::Connection::dispatchOneMessage() (Connection.cpp:717)
==27984==    by 0x530342B: WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>::operator()(CoreIPC::Connection*) (Functional.h:173)
==27984==    by 0x5303231: WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>, void ()(CoreIPC::Connection*)>::operator()() (Functional.h:405)
==27984==    by 0x5454DBD: WTF::Function<void ()()>::operator()() const (Functional.h:613)
==27984==    by 0x9E69861: WebCore::RunLoop::performWork() (RunLoop.cpp:102)
==27984==    by 0xA79D5EE: WebCore::RunLoop::wakeUpEvent(void*, void*, unsigned int) (RunLoopEfl.cpp:100)
==27984==    by 0x415B060: _ecore_pipe_read (ecore_pipe.c:625)
==27984==    by 0x415A130: _ecore_main_loop_iterate_internal (ecore_private.h:343)
==27984==    by 0x415A676: ecore_main_loop_begin (ecore_main.c:931)
==27984==    by 0x40200B: main (main.c:218)
==27984==

[Sudarsana Nagineni (babu)]

Created attachment 154028 [details]
Patch

[Chris Dumez]

Comment on attachment 154028 [details]
Patch

LGTM. Thanks for fixing this.

[Gyuyoung Kim]

Comment on attachment 154028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154028&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:297
> +    for ( ; it != end; ++it)

If possible, why don't you modify this one as below ?

for (LoadingResourcesMap::iterator it = loadingResourcesMap.begin(); it != end; ++it)

In addition, we can save a line.

[Chris Dumez]

Comment on attachment 154028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154028&action=review

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:297
>> +    for ( ; it != end; ++it)
> 
> If possible, why don't you modify this one as below ?
> 
> for (LoadingResourcesMap::iterator it = loadingResourcesMap.begin(); it != end; ++it)
> 
> In addition, we can save a line.

This is really a matter of preference. There is no coding style rule that says we should do it either way and AFAIK there is no performance impact.

[Gyuyoung Kim]

(In reply to comment #4)
> This is really a matter of preference. There is no coding style rule that says we should do it either way and AFAIK there is no performance impact.

This is just my suggestion because it seems to me above code is more used in WebKit inside. Of course, there is no performance gain.

[Sudarsana Nagineni (babu)]

(In reply to comment #5)
> This is just my suggestion because it seems to me above code is more used in WebKit inside. Of course, there is no performance gain.

Can I leave this as it is since there is no any gain in changing it?

[Gyuyoung Kim]

(In reply to comment #6)
> (In reply to comment #5)
> > This is just my suggestion because it seems to me above code is more used in WebKit inside. Of course, there is no performance gain.
> 
> Can I leave this as it is since there is no any gain in changing it?

Ok, I don't wanna disturb this patch is landed. LGTM as well.

[Sudarsana Nagineni (babu)]

Created attachment 154581 [details]
Patch

Rebased

[Kentaro Hara]

Comment on attachment 154581 [details]
Patch

Looks OK

[WebKit Review Bot]

Comment on attachment 154581 [details]
Patch

Clearing flags on attachment: 154581

Committed r123727: <http://trac.webkit.org/changeset/123727>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.