Bug 103692 - [EFL][WK2] Create a ewk view object with new context for API tests
Summary: [EFL][WK2] Create a ewk view object with new context for API tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on: 104110 104113
Blocks: 103229
  Show dependency treegraph
 
Reported: 2012-11-29 17:31 PST by Jinwoo Song
Modified: 2012-12-11 03:09 PST (History)
7 users (show)

See Also:


Attachments
Patch (6.94 KB, patch)
2012-11-29 19:22 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (2.09 KB, patch)
2012-11-30 22:02 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (2.37 KB, patch)
2012-12-05 08:06 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (2.36 KB, patch)
2012-12-06 07:44 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (2.12 KB, patch)
2012-12-10 23:38 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2012-11-29 17:31:37 PST
Bug 103229 fixes the bug which PageGroupID increased even though the EwkView was created with default context.
If the Bug 103229 is resolved, the EwkViews with same context will have a same setting preferences.
For example, in Minibrowser, if there are two windows(EwkViews) A and B, and if the Javascript is disabled in window A,
the Javascript will be disabled in B, too.

Currently, in ewk_setting API test, Javascript is disabled while testing ewk_settings_javascript_enabled API, 
and then next test cases are tested using HTML files with Javascript. So it eventually causes the test case fail.

So IMO, it would be proper to restore the default setting values after testing the setting APIs not to influence
the following test cases.
Comment 1 Jinwoo Song 2012-11-29 19:22:16 PST
Created attachment 176888 [details]
Patch
Comment 2 Gyuyoung Kim 2012-11-29 23:35:19 PST
Comment on attachment 176888 [details]
Patch

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

> Source/WebKit2/ChangeLog:6
> +        Restore the default setting values after testing the setting APIs not to influence

This patch description is almost same as bug title. Please write this more detailed.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_settings.cpp:68
> +    ASSERT_TRUE(ewk_settings_javascript_enabled_set(settings, EINA_FALSE));

We decided to use standard boolean in test case as well. I think we need to change this whenever we touch API test cases.
Comment 3 Chris Dumez 2012-11-29 23:39:38 PST
Does this mean that the settings are not view-specific? The view is being destroyed between each API test so we should not have any side effect on following tests if the settings were view-specific.

If they are not view-specific, it is a bit confusing because in the EWK API, we get the Ewk_Settings object from the view.
Comment 4 Jinwoo Song 2012-11-30 00:05:11 PST
Comment on attachment 176888 [details]
Patch

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

>> Source/WebKit2/ChangeLog:6
>> +        Restore the default setting values after testing the setting APIs not to influence
> 
> This patch description is almost same as bug title. Please write this more detailed.

Okay, I'll describe more context.

>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_settings.cpp:68
>> +    ASSERT_TRUE(ewk_settings_javascript_enabled_set(settings, EINA_FALSE));
> 
> We decided to use standard boolean in test case as well. I think we need to change this whenever we touch API test cases.

I'll fix it.
Comment 5 Jinwoo Song 2012-11-30 00:19:33 PST
(In reply to comment #3)
> Does this mean that the settings are not view-specific? The view is being destroyed between each API test so we should not have any side effect on following tests if the settings were view-specific.
> 
> If they are not view-specific, it is a bit confusing because in the EWK API, we get the Ewk_Settings object from the view.

Actually, the settings are pageGroup-specific not view-specific. But in current implementation, without Bug 103229, the ewk views with default context are created by increasing the pageGroup's unique ID. So there was no problem in the ewk_setting API test.

My patch in Bug 103229 fixes the ewk views with default context to be created with the same pageGroup ID. By this change, the newly created ewk view has the default EwkContext instance and has the same pageGroup as previous ewk_view. It means that the new ewk_view get influenced by the setting values of the previous ewk_view.(assuming default context)
Comment 6 Chris Dumez 2012-11-30 00:22:20 PST
(In reply to comment #5)
> (In reply to comment #3)
> > Does this mean that the settings are not view-specific? The view is being destroyed between each API test so we should not have any side effect on following tests if the settings were view-specific.
> > 
> > If they are not view-specific, it is a bit confusing because in the EWK API, we get the Ewk_Settings object from the view.
> 
> Actually, the settings are pageGroup-specific not view-specific. But in current implementation, without Bug 103229, the ewk views with default context are created by increasing the pageGroup's unique ID. So there was no problem in the ewk_setting API test.
> 
> My patch in Bug 103229 fixes the ewk views with default context to be created with the same pageGroup ID. By this change, the newly created ewk view has the default EwkContext instance and has the same pageGroup as previous ewk_view. It means that the new ewk_view get influenced by the setting values of the previous ewk_view.(assuming default context)

Well, I'm not sure this is what we want. I find it clean that the settings of one view were not propagated implicitly to the next one. Why do we want to change the behavior?
Comment 7 Jinwoo Song 2012-11-30 00:27:46 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > Does this mean that the settings are not view-specific? The view is being destroyed between each API test so we should not have any side effect on following tests if the settings were view-specific.
> > > 
> > > If they are not view-specific, it is a bit confusing because in the EWK API, we get the Ewk_Settings object from the view.
> > 
> > Actually, the settings are pageGroup-specific not view-specific. But in current implementation, without Bug 103229, the ewk views with default context are created by increasing the pageGroup's unique ID. So there was no problem in the ewk_setting API test.
> > 
> > My patch in Bug 103229 fixes the ewk views with default context to be created with the same pageGroup ID. By this change, the newly created ewk view has the default EwkContext instance and has the same pageGroup as previous ewk_view. It means that the new ewk_view get influenced by the setting values of the previous ewk_view.(assuming default context)
> 
> Well, I'm not sure this is what we want. I find it clean that the settings of one view were not propagated implicitly to the next one. Why do we want to change the behavior?

Then could you review the bug 103229 first if it is a right way? There is a bug that the window name is not recognized rightly as the pageGroup is created twice. I think bug 103229 should be discussed before this one.
Comment 8 Ryuan Choi 2012-11-30 00:47:16 PST
How about creating ewk_context and ewk_view for every test cases?
Comment 9 Jinwoo Song 2012-11-30 09:44:07 PST
(In reply to comment #8)
> How about creating ewk_context and ewk_view for every test cases?
That looks like a better solution, thanks.
Comment 10 Jinwoo Song 2012-11-30 22:02:52 PST
Created attachment 177082 [details]
Patch
Comment 11 Ryuan Choi 2012-11-30 22:10:58 PST
Comment on attachment 177082 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:62
> +    m_webView = ewk_view_smart_add(evas, smart, ewk_context_new());

Don't we need unref for newly created context?
Comment 12 Chris Dumez 2012-11-30 23:52:19 PST
Comment on attachment 177082 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:62
>> +    m_webView = ewk_view_smart_add(evas, smart, ewk_context_new());
> 
> Don't we need unref for newly created context?

Ryuan is right.
Comment 13 Jinwoo Song 2012-12-04 18:27:39 PST
There is a crash in the favicon database API test after using new context and releasing the context.

jinwoo@jinwoo-linux:~/webkit_opensource/webkit-efl/WebKit/WebKitBuild/Release/bin$ ./test_ewk2_favicon_database 
[==========] Running 2 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 2 tests from EWK2UnitTestBase
[ RUN      ] EWK2UnitTestBase.ewk_favicon_database_url_get
[       OK ] EWK2UnitTestBase.ewk_favicon_database_url_get (716 ms)
[ RUN      ] EWK2UnitTestBase.ewk_favicon_database_async_icon_get
EINA: Data at address 0x0 is invalid. Replacing with zero page.
mmap: Operation not permitted
ERR<20249>:eina_mmap eina_mmap.c:109 _eina_mmap_safe_sigbus() Failed to mmap() /dev/zero in place of page. SIGBUS!!!
Comment 14 Jinwoo Song 2012-12-05 00:58:43 PST
Crash log from debug build.

ASSERTION FAILED: !result
/home/jinwoo/webkit_opensource/webkit-efl/WebKit/Source/WTF/wtf/ThreadingPthreads.cpp(345) : void WTF::Mutex::lock()
1   0x7ffa60c82839 WTF::Mutex::lock()
2   0x7ffa6089f4a4 WTF::Locker<WTF::Mutex>::Locker(WTF::Mutex&)
3   0x7ffa5ff23d47 void WTF::addIterator<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >(WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> > const*, WTF::HashTableConstIterator<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >*)
4   0x7ffa5ff25f5a WTF::HashTableConstIterator<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >::HashTableConstIterator(WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> > const*, WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> const*, WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> const*)
5   0x7ffa5ff2525b WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >::makeConstIterator(WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>*) const
6   0x7ffa5ff23cb9 WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >::begin() const
7   0x7ffa5ff22be1 WTF::HashMap<void (*)(char const*, void*), IconChangeCallbackData, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >::begin() const
8   0x7ffa5ff21a0b EwkFaviconDatabase::didChangeIconForPageURL(OpaqueWKIconDatabase const*, OpaqueWKURL const*, void const*)
9   0x7ffa5fd52711 WebKit::WebIconDatabaseClient::didChangeIconForPageURL(WebKit::WebIconDatabase*, WebKit::WebURL*)
10  0x7ffa5fd4fae5 WebKit::WebIconDatabase::didChangeIconForPageURL(WTF::String const&)
11  0x7ffa5aa60111 WebCore::IconDatabase::setIconDataForIconURL(WTF::PassRefPtr<WebCore::SharedBuffer>, WTF::String const&)
12  0x7ffa5fd4f43d WebKit::WebIconDatabase::setIconDataForIconURL(CoreIPC::DataReference const&, WTF::String const&)
13  0x7ffa5ff6ce6f void CoreIPC::callMemberFunction<WebKit::WebIconDatabase, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&), CoreIPC::DataReference, WTF::String>(CoreIPC::Arguments2<CoreIPC::DataReference, WTF::String> const&, WebKit::WebIconDatabase*, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&))
14  0x7ffa5ff6c9a7 void CoreIPC::handleMessage<Messages::WebIconDatabase::SetIconDataForIconURL, WebKit::WebIconDatabase, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&)>(CoreIPC::MessageDecoder&, WebKit::WebIconDatabase*, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&))
15  0x7ffa5ff6c1f1 WebKit::WebIconDatabase::didReceiveWebIconDatabaseMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
16  0x7ffa5fd4fde8 WebKit::WebIconDatabase::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
17  0x7ffa5fc9e6c2 CoreIPC::MessageReceiverMap::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
18  0x7ffa5fd2e25d WebKit::WebContext::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
19  0x7ffa5fdab9ba WebKit::WebProcessProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
20  0x7ffa5fc936f4 CoreIPC::Connection::dispatchMessage(CoreIPC::MessageID, CoreIPC::MessageDecoder&)
21  0x7ffa5fc93860 CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::MessageDecoder>&)
22  0x7ffa5fc93aab CoreIPC::Connection::dispatchOneMessage()
23  0x7ffa5fc9dc4c WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>::operator()(CoreIPC::Connection*)
24  0x7ffa5fc9da52 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>, void (CoreIPC::Connection*)>::operator()()
25  0x7ffa60c86496 WTF::Function<void ()>::operator()() const
26  0x7ffa5ab79622 WebCore::RunLoop::performWork()
27  0x7ffa5b5a5a17 WebCore::RunLoop::wakeUpEvent(void*, void*, unsigned int)
28  0x7ffa61307751
29  0x7ffa613066a1
30  0x7ffa61306b45 ecore_main_loop_iterate
31  0x40e49f EWK2UnitTest::EWK2UnitTestBase::waitUntilLoadFinished(double)
Comment 15 Jinwoo Song 2012-12-05 01:42:49 PST
It seems that there is a mutex lock problem in the favicon database when multiple web process are running. The issue itself is an another bug.
Also, I found one more bug that the web process does not killed even though the page is closed. (Actually, there is no logic to handle this yet.)

So I suggest to resolve this bug with the first patch. It means API test case creates a ewk view with default context and restore the default setting values. 

Ryuan, how do you think about this?
Comment 16 Chris Dumez 2012-12-05 03:02:43 PST
(In reply to comment #14)
> Crash log from debug build.
> 
> ASSERTION FAILED: !result
> /home/jinwoo/webkit_opensource/webkit-efl/WebKit/Source/WTF/wtf/ThreadingPthreads.cpp(345) : void WTF::Mutex::lock()
> 1   0x7ffa60c82839 WTF::Mutex::lock()
> 2   0x7ffa6089f4a4 WTF::Locker<WTF::Mutex>::Locker(WTF::Mutex&)
> 3   0x7ffa5ff23d47 void WTF::addIterator<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >(WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> > const*, WTF::HashTableConstIterator<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >*)
> 4   0x7ffa5ff25f5a WTF::HashTableConstIterator<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >::HashTableConstIterator(WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> > const*, WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> const*, WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> const*)
> 5   0x7ffa5ff2525b WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >::makeConstIterator(WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>*) const
> 6   0x7ffa5ff23cb9 WTF::HashTable<void (*)(char const*, void*), WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<void (*)(char const*, void*), IconChangeCallbackData> >, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashMapValueTraits<WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >, WTF::HashTraits<void (*)(char const*, void*)> >::begin() const
> 7   0x7ffa5ff22be1 WTF::HashMap<void (*)(char const*, void*), IconChangeCallbackData, WTF::PtrHash<void (*)(char const*, void*)>, WTF::HashTraits<void (*)(char const*, void*)>, WTF::HashTraits<IconChangeCallbackData> >::begin() const
> 8   0x7ffa5ff21a0b EwkFaviconDatabase::didChangeIconForPageURL(OpaqueWKIconDatabase const*, OpaqueWKURL const*, void const*)
> 9   0x7ffa5fd52711 WebKit::WebIconDatabaseClient::didChangeIconForPageURL(WebKit::WebIconDatabase*, WebKit::WebURL*)
> 10  0x7ffa5fd4fae5 WebKit::WebIconDatabase::didChangeIconForPageURL(WTF::String const&)
> 11  0x7ffa5aa60111 WebCore::IconDatabase::setIconDataForIconURL(WTF::PassRefPtr<WebCore::SharedBuffer>, WTF::String const&)
> 12  0x7ffa5fd4f43d WebKit::WebIconDatabase::setIconDataForIconURL(CoreIPC::DataReference const&, WTF::String const&)
> 13  0x7ffa5ff6ce6f void CoreIPC::callMemberFunction<WebKit::WebIconDatabase, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&), CoreIPC::DataReference, WTF::String>(CoreIPC::Arguments2<CoreIPC::DataReference, WTF::String> const&, WebKit::WebIconDatabase*, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&))
> 14  0x7ffa5ff6c9a7 void CoreIPC::handleMessage<Messages::WebIconDatabase::SetIconDataForIconURL, WebKit::WebIconDatabase, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&)>(CoreIPC::MessageDecoder&, WebKit::WebIconDatabase*, void (WebKit::WebIconDatabase::*)(CoreIPC::DataReference const&, WTF::String const&))
> 15  0x7ffa5ff6c1f1 WebKit::WebIconDatabase::didReceiveWebIconDatabaseMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
> 16  0x7ffa5fd4fde8 WebKit::WebIconDatabase::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
> 17  0x7ffa5fc9e6c2 CoreIPC::MessageReceiverMap::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
> 18  0x7ffa5fd2e25d WebKit::WebContext::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
> 19  0x7ffa5fdab9ba WebKit::WebProcessProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&)
> 20  0x7ffa5fc936f4 CoreIPC::Connection::dispatchMessage(CoreIPC::MessageID, CoreIPC::MessageDecoder&)
> 21  0x7ffa5fc93860 CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::MessageDecoder>&)
> 22  0x7ffa5fc93aab CoreIPC::Connection::dispatchOneMessage()
> 23  0x7ffa5fc9dc4c WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>::operator()(CoreIPC::Connection*)
> 24  0x7ffa5fc9da52 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>, void (CoreIPC::Connection*)>::operator()()
> 25  0x7ffa60c86496 WTF::Function<void ()>::operator()() const
> 26  0x7ffa5ab79622 WebCore::RunLoop::performWork()
> 27  0x7ffa5b5a5a17 WebCore::RunLoop::wakeUpEvent(void*, void*, unsigned int)
> 28  0x7ffa61307751
> 29  0x7ffa613066a1
> 30  0x7ffa61306b45 ecore_main_loop_iterate
> 31  0x40e49f EWK2UnitTest::EWK2UnitTestBase::waitUntilLoadFinished(double)

I implemented the Ewk_Favicon_Database so I can take a look at this. I have an idea on how to fix it, I just need to test it.
Comment 17 Chris Dumez 2012-12-05 03:47:29 PST
FYI, I'm addressing the EwkFavicon crash issue in Bug 103229. Note however that this patch is causing some other API tests to crash so please wait for those to be fixed before you land this present patch (I can look into the other crashing issues).
Comment 18 Jinwoo Song 2012-12-05 08:06:55 PST
Created attachment 177752 [details]
Patch

Unrefed for newly created context
Comment 19 Chris Dumez 2012-12-06 05:20:16 PST
Comment on attachment 177752 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:62
> +    m_webView = ewk_view_smart_add(evas, smart, ewk_context_new());

Honestly, I think the following would be cleaner:
Ewk_Context* newContext = ewk_context_new();
m_webView = ewk_view_smart_add(evas, smart, newContext); // Ewk_View will ref it and keep a pointer to it.
ewk_object_unref(newContext); // We unref it straight away since we don't keep a pointer to it.

> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:72
> +    ewk_object_unref(ewk_view_context_get(m_webView));

This is not how it is supposed to be done, please see comment above.
Comment 20 Jinwoo Song 2012-12-06 07:38:52 PST
Comment on attachment 177752 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:62
>> +    m_webView = ewk_view_smart_add(evas, smart, ewk_context_new());
> 
> Honestly, I think the following would be cleaner:
> Ewk_Context* newContext = ewk_context_new();
> m_webView = ewk_view_smart_add(evas, smart, newContext); // Ewk_View will ref it and keep a pointer to it.
> ewk_object_unref(newContext); // We unref it straight away since we don't keep a pointer to it.

Thanks for advice and I'll fix the patch. :)
Comment 21 Jinwoo Song 2012-12-06 07:44:28 PST
Created attachment 178012 [details]
Patch
Comment 22 Gyuyoung Kim 2012-12-10 17:54:53 PST
Comment on attachment 178012 [details]
Patch

I also agree Ryuan's comment. Ryuan and Christophe, do you guys wanna take look this patch further ?
Comment 23 Chris Dumez 2012-12-10 21:43:38 PST
Comment on attachment 178012 [details]
Patch

I think the comments are not really needed but LGTM.
Comment 24 Gyuyoung Kim 2012-12-10 22:43:32 PST
(In reply to comment #23)
> (From update of attachment 178012 [details])
> I think the comments are not really needed but LGTM.

I think so. Jinwoo, please land this after removing unneeded comment and checking if there is no API test fail and layout test.
Comment 25 Jinwoo Song 2012-12-10 23:38:02 PST
Created attachment 178729 [details]
Patch

Removed redundant comments. There is no API test regressions and this patch does not influence the layout test as this only modified the API test file.
Comment 26 WebKit Review Bot 2012-12-11 03:09:10 PST
Comment on attachment 178729 [details]
Patch

Clearing flags on attachment: 178729

Committed r137278: <http://trac.webkit.org/changeset/137278>
Comment 27 WebKit Review Bot 2012-12-11 03:09:16 PST
All reviewed patches have been landed.  Closing bug.