Bug 103692 – [EFL][WK2] Create a ewk view object with new context for API tests

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Jinwoo Song]

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.

[Jinwoo Song]

Created attachment 176888 [details]
Patch

[Gyuyoung Kim]

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.

[Chris Dumez]

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.

[Jinwoo Song]

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.

[Jinwoo Song]

(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)

[Chris Dumez]

(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?

[Jinwoo Song]

(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.

[Ryuan Choi]

How about creating ewk_context and ewk_view for every test cases?

[Jinwoo Song]

(In reply to comment #8)
> How about creating ewk_context and ewk_view for every test cases?
That looks like a better solution, thanks.

[Jinwoo Song]

Created attachment 177082 [details]
Patch

[Ryuan Choi]

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?

[Chris Dumez]

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.

[Jinwoo Song]

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!!!

[Jinwoo Song]

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)

[Jinwoo Song]

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?

[Chris Dumez]

(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.

[Chris Dumez]

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).

[Jinwoo Song]

Created attachment 177752 [details]
Patch

Unrefed for newly created context

[Chris Dumez]

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.

[Jinwoo Song]

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. :)

[Jinwoo Song]

Created attachment 178012 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 178012 [details]
Patch

I also agree Ryuan's comment. Ryuan and Christophe, do you guys wanna take look this patch further ?

[Chris Dumez]

Comment on attachment 178012 [details]
Patch

I think the comments are not really needed but LGTM.

[Gyuyoung Kim]

(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.

[Jinwoo Song]

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.

[WebKit Review Bot]

Comment on attachment 178729 [details]
Patch

Clearing flags on attachment: 178729

Committed r137278: <http://trac.webkit.org/changeset/137278>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.