Summary: | [EFL][WK2] Create a ewk view object with new context for API tests | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jinwoo Song <jinwoo7.song> | ||||||||||||
Component: | WebKit EFL | Assignee: | Jinwoo Song <jinwoo7.song> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cdumez, gyuyoung.kim, kenneth, lucas.de.marchi, rakuco, ryuan.choi, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 104110, 104113 | ||||||||||||||
Bug Blocks: | 103229 | ||||||||||||||
Attachments: |
|
Description
Jinwoo Song
2012-11-29 17:31:37 PST
Created attachment 176888 [details]
Patch
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. 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 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. (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) (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? (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. How about creating ewk_context and ewk_view for every test cases? (In reply to comment #8) > How about creating ewk_context and ewk_view for every test cases? That looks like a better solution, thanks. Created attachment 177082 [details]
Patch
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 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. 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!!! 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) 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? (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. 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). Created attachment 177752 [details]
Patch
Unrefed for newly created context
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 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. :) Created attachment 178012 [details]
Patch
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 on attachment 178012 [details]
Patch
I think the comments are not really needed but LGTM.
(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. 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 on attachment 178729 [details] Patch Clearing flags on attachment: 178729 Committed r137278: <http://trac.webkit.org/changeset/137278> All reviewed patches have been landed. Closing bug. |