Bug 141154 – [EFL] REGRESSION (r178685): ASSERTION FAILED: !parameters.mediaKeyStorageDirectory.isEmpty()

• Component: New Bugs
• Status: RESOLVED FIXED

[Paweł Forysiuk]

Revsion 178685 introduced a crash with assert  !parameters.mediaKeyStorageDirectory.isEmpty() in  Source/WebKit2/WebProcess/MediaCache/WebMediaStorageManager.cpp

Merge API::ProcessPoolConfiguration and _WKProcessPoolConfiguration
​https://bugs.webkit.org/show_bug.cgi?id=140601


It can be reproduced every time by running some test with WebKitTestRunner, eg.

./Tools/Scripts/run-webkit-tests --efl --debug LayoutTests/fast/forms/plaintext-mode-1.html

It can be reproduced every time for EFL port, GTK port does not seem to trigger this issue.


STDERR: ERR<2949>:ecore_evas modules/ecore_evas/engines/x/ecore_evas_x.c:4419 ecore_evas_gl_x11_options_new_internal() evas_engine_info_set() init engine 'opengl_x11' failed.
STDERR: ERR<2949>:evas_main lib/evas/canvas/evas_gl.c:138 evas_gl_new() Evas GL engine not available.
STDERR: ERR<2949>:efreet_cache lib/efreet/efreet_cache.c:1134 on_send_register() org.freedesktop.DBus.Error.ServiceUnknown The name org.enlightenment.Efreet was not provided by any .service files
STDERR: ASSERTION FAILED: !parameters.mediaKeyStorageDirectory.isEmpty()
STDERR: ../../Source/WebKit2/WebProcess/MediaCache/WebMediaKeyStorageManager.cpp(40) : virtual void WebKit::WebMediaKeyStorageManager::initialize(const WebKit::WebProcessCreationParameters&)
STDERR: 1   0x7f85a73f62df WTFCrash
STDERR: 2   0x7f85ac52a5c8 WebKit::WebMediaKeyStorageManager::initialize(WebKit::WebProcessCreationParameters const&)
STDERR: 3   0x7f85ac30f0e7 WebKit::WebProcess::initializeWebProcess(WebKit::WebProcessCreationParameters&&)
STDERR: 4   0x7f85ac5974cb void IPC::callMemberFunctionImpl<WebKit::WebProcess, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&), std::tuple<WebKit::WebProcessCreationParameters>, 0ul>(WebKit::WebProcess*, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&), std::tuple<WebKit::WebProcessCreationParameters>&&, std::index_sequence<0ul>)
STDERR: 5   0x7f85ac596b54 void IPC::callMemberFunction<WebKit::WebProcess, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&), std::tuple<WebKit::WebProcessCreationParameters>, std::make_index_sequence<1ul> >(std::tuple<WebKit::WebProcessCreationParameters>&&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&))
STDERR: 6   0x7f85ac594b8c void IPC::handleMessage<Messages::WebProcess::InitializeWebProcess, WebKit::WebProcess, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&)>(IPC::MessageDecoder&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&))
STDERR: 7   0x7f85ac593867 WebKit::WebProcess::didReceiveWebProcessMessage(IPC::Connection&, IPC::MessageDecoder&)
STDERR: 8   0x7f85ac310246 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
STDERR: 9   0x7f85ac11081e IPC::Connection::dispatchMessage(IPC::MessageDecoder&)
STDERR: 10  0x7f85ac1108ea IPC::Connection::dispatchMessage(std::unique_ptr<IPC::MessageDecoder, std::default_delete<IPC::MessageDecoder> >)
STDERR: 11  0x7f85ac110aab IPC::Connection::dispatchOneMessage()
STDERR: 12  0x7f85ac12110d WTF::FunctionWrapper<void (IPC::Connection::*)()>::operator()(IPC::Connection*)
STDERR: 13  0x7f85ac120f58 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (IPC::Connection::*)()>, void (IPC::Connection*)>::operator()()
STDERR: 14  0x7f85ac11aa03 WTF::Function<void ()>::operator()() const
STDERR: 15  0x7f85ac11732d std::_Function_handler<void (), WTF::Function<void ()> >::_M_invoke(std::_Any_data const&)
STDERR: 16  0x7f85ac0f9598 std::function<void ()>::operator()() const
STDERR: 17  0x7f85ade0b14f WTF::RunLoop::performWork()
STDERR: 18  0x7f85ade0d356 WTF::RunLoop::wakeUpEvent(void*, void*, unsigned int)
STDERR: 19  0x7f85a521f88f
STDERR: 20  0x7f85a522005a
STDERR: 21  0x7f85a521ee61
STDERR: 22  0x7f85a521f087 ecore_main_loop_begin
STDERR: 23  0x7f85ade0d2e7 WTF::RunLoop::run()
STDERR: 24  0x7f85ac5378c6 int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**)
STDERR: 25  0x7f85ac537506 WebProcessMainUnix
STDERR: 26  0x4008d1 main
STDERR: 27  0x7f85a7de1ec5 __libc_start_main
STDERR: 28  0x4007d9
STDERR: 1   0x7fda52569335
STDERR: 2   0x7fda4669cd40
STDERR: 3   0x43d8e2 WTF::RefPtr<WTF::StringImpl>::operator!() const
STDERR: 4   0x43c70a WTF::String::isNull() const
STDERR: 5   0x7fda521eba88 WTF::operator!(WTF::String const&)
STDERR: 6   0x7fda4bfbf013 WebKit::toCopiedURLAPI(WTF::String const&)
STDERR: 7   0x7fda4c30e834 WebKit::WebViewClient::webProcessCrashed(WebKit::WebView*, WTF::String const&)
STDERR: 8   0x7fda4c30c970 WebKit::WebView::processDidExit()
STDERR: 9   0x7fda4c06ddc7 WebKit::WebPageProxy::resetStateAfterProcessExited()
STDERR: 10  0x7fda4c06d8e1 WebKit::WebPageProxy::processDidCrash()
STDERR: 11  0x7fda4c0a9d15 WebKit::WebProcessProxy::didClose(IPC::Connection&)
STDERR: 12  0x7fda4bf32e76
STDERR: 13  0x7fda4bf3430a
STDERR: 14  0x7fda524f749c std::function<void ()>::operator()() const
STDERR: 15  0x7fda4dc2e14f WTF::RunLoop::performWork()
STDERR: 16  0x7fda4dc30356 WTF::RunLoop::wakeUpEvent(void*, void*, unsigned int)
STDERR: 17  0x7fda47a1188f
STDERR: 18  0x7fda47a1205a
STDERR: 19  0x7fda47a10e61
STDERR: 20  0x7fda47a11087 ecore_main_loop_begin
STDERR: 21  0x451d31 WTR::TestController::platformRunUntil(bool&, double)
STDERR: 22  0x438ba0 WTR::TestController::runUntil(bool&, double)
STDERR: 23  0x437cc5 WTR::TestController::resetStateToConsistentValues()
STDERR: 24  0x438aba WTR::TestController::run()
STDERR: 25  0x435527 WTR::TestController::TestController(int, char const**)
STDERR: 26  0x451f3a main
STDERR: 27  0x7fda46687ec5 __libc_start_main
STDERR: 28  0x431559
STDERR: LEAK: 1 WebPageProxy
STDERR: LEAK: 2 WebProcessPool

[Paweł Forysiuk]

Created attachment 245930 [details]
Patch

[Csaba Osztrogonác]

Could you explain why is it a regression of r178685 and why
do you think if it is the proper fix?

It's not clear for me why is it EFL specific. I haven't found 
any EFL specific thing near mediaKeyStorageDirectory.

[Paweł Forysiuk]

Well the patch uploaded was just a quick workaround to be honest.


I don't know if it is just EFL specific but GTK port 
does not seem to be affected.


(In reply to comment #2)
> Could you explain why is it a regression of r178685 and why

It started to occur after that commit, i bisected it and reverting that change
would prevent the assertion.

> do you think if it is the proper fix?
> 

No. That was a quick workaround. I will attach a different patch, seems like a more proper solution but I'm still unsure.

> It's not clear for me why is it EFL specific. I haven't found 
> any EFL specific thing near mediaKeyStorageDirectory.

Don't know if it is just EFL specific but it does not occur on GTK port,
they use slighly different codepaths when setting things up. Seems like GTK
port somehow creates those folders when initialising Web process whlie EFL does not it seems. If I miss something obvious please tell me.

[Paweł Forysiuk]

Created attachment 246656 [details]
Updated patch with more generic code

[WebKit Commit Bot]

Attachment 246656 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Paweł Forysiuk]

Created attachment 246657 [details]
Fixed up previous patch

[Paweł Forysiuk]

Created attachment 246658 [details]
patch

[Grzegorz Czajkowski]

(In reply to comment #3)
> Well the patch uploaded was just a quick workaround to be honest.
> 
> 
> I don't know if it is just EFL specific but GTK port 
> does not seem to be affected.
> 

Probably because of they always create legacy options when context is constructed. See their webkitWebContextConstructed(GObject* object) in "Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp"

> 
> (In reply to comment #2)
> > Could you explain why is it a regression of r178685 and why
> 
> It started to occur after that commit, i bisected it and reverting that
> change
> would prevent the assertion.
> 
> > do you think if it is the proper fix?
> > 
> 
> No. That was a quick workaround. I will attach a different patch, seems like
> a more proper solution but I'm still unsure.
> 
> > It's not clear for me why is it EFL specific. I haven't found 
> > any EFL specific thing near mediaKeyStorageDirectory.
> 
> Don't know if it is just EFL specific but it does not occur on GTK port,
> they use slighly different codepaths when setting things up. Seems like GTK
> port somehow creates those folders when initialising Web process whlie EFL
> does not it seems. If I miss something obvious please tell me.

I think it's good idea to implement WebsiteDataStore::websiteDataDirectoryFileSystemRepresentation for EFL and have it WebKitTestRunner specific only. It looks like Mac already does, see Source/WebKit2/UIProcess/API/Cocoa/APIWebsiteDataStoreCocoa.mm

After that, we will have separate paths for testing and potential applications.

[Grzegorz Czajkowski]

Comment on attachment 246658 [details]
patch

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

> Source/WebKit2/ChangeLog:7
> +

What to be nice to describe what is the purpose of this change.

> Source/WebKit2/ChangeLog:10
> +        * PlatformGTK.cmake: Ditto.

According to your description, this line should go above.

> Source/WebKit2/PlatformEfl.cmake:83
> +    UIProcess/API/efl/WebsiteDataStoreEFL.cpp

Try to keep alphabetical order.

> Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Apple ?

> Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:13
> + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''

Please use BSD license.

> Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:27
> +#include "APIWebsiteDataStore.h"

An empty line is required here.

> Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:36
> +    const String& path = String::fromUTF8(efreet_data_home_get()) + "/" +  directoryName;

Can we use more WebKitTestRunner specific path? How about taking it from DUMPRENDERTREE_TEMP? Ossy and Gyuyoung, what's your take on it?

> Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:48
> +    configuration.localStorageDirectory = websiteDataDirectoryFileSystemRepresentation("LocalStorage");

Is this needed? Seems strange that we partially implement it. Either ensure all storage paths here or remove local storage path.

[Grzegorz Czajkowski]

Comment on attachment 246658 [details]
patch

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

>> Source/WebKit2/UIProcess/API/efl/WebsiteDataStoreEFL.cpp:36
>> +    const String& path = String::fromUTF8(efreet_data_home_get()) + "/" +  directoryName;
> 
> Can we use more WebKitTestRunner specific path? How about taking it from DUMPRENDERTREE_TEMP? Ossy and Gyuyoung, what's your take on it?

Please ignore this comment, WebsiteDataStore::websiteDataDirectoryFileSystemRepresentation is not WebKitTestRunner specific.

[Grzegorz Czajkowski]

(In reply to comment #8)
> (In reply to comment #3)
> > Well the patch uploaded was just a quick workaround to be honest.
> > 
> > 
> > I don't know if it is just EFL specific but GTK port 
> > does not seem to be affected.
> > 
> 
> Probably because of they always create legacy options when context is
> constructed. See their webkitWebContextConstructed(GObject* object) in
> "Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp"
> 
> > 
> > (In reply to comment #2)
> > > Could you explain why is it a regression of r178685 and why
> > 
> > It started to occur after that commit, i bisected it and reverting that
> > change
> > would prevent the assertion.
> > 
> > > do you think if it is the proper fix?
> > > 
> > 
> > No. That was a quick workaround. I will attach a different patch, seems like
> > a more proper solution but I'm still unsure.
> > 
> > > It's not clear for me why is it EFL specific. I haven't found 
> > > any EFL specific thing near mediaKeyStorageDirectory.
> > 
> > Don't know if it is just EFL specific but it does not occur on GTK port,
> > they use slighly different codepaths when setting things up. Seems like GTK
> > port somehow creates those folders when initialising Web process whlie EFL
> > does not it seems. If I miss something obvious please tell me.
> 
> I think it's good idea to implement
> WebsiteDataStore::websiteDataDirectoryFileSystemRepresentation for EFL and
> have it WebKitTestRunner specific only. It looks like Mac already does, see
> Source/WebKit2/UIProcess/API/Cocoa/APIWebsiteDataStoreCocoa.mm
> 
> After that, we will have separate paths for testing and potential
> applications.

Please ignore this comment, WebsiteDataStore::websiteDataDirectoryFileSystemRepresentation is not WebKitTestRunner specific.

[Paweł Forysiuk]

Created attachment 247853 [details]
Patch

[Grzegorz Czajkowski]

IMHO, looks sensibly to set MediaKeys as well as other DataBase paths in WebKitTestRunner code. Since it affects Mac port as well I'm adding Apple developers. 
We can not run-webkit-tests in debug build because of it :(

[Chris Dumez]

This is a non-EFL specific WebKitTestRunner change, maybe Alexey can take a look?

[Alexey Proskuryakov]

+Eric and Jer.

[Jer Noble]

Comment on attachment 247853 [details]
Patch

LGTM. r+.

[Grzegorz Czajkowski]

Comment on attachment 247853 [details]
Patch

Thank you for fixing it!

[WebKit Commit Bot]

Comment on attachment 247853 [details]
Patch

Clearing flags on attachment: 247853

Committed r181071: <http://trac.webkit.org/changeset/181071>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.