Bug 203460

Summary: REGRESSION(2.27.2): [GTK] Incognito mode is broken under flatpak
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bugs-noreply, cdumez, cgarcia, clopez, mcatanzaro, pnormand, youennf
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
flapjak manifest
none
Patch
none
Patch
youennf: review+
Patch for landing none

Description Michael Catanzaro 2019-10-26 10:00:03 PDT
Since 2.27.2, Epiphany's incognito mode no longer works. No page content is rendered.
Comment 1 Carlos Garcia Campos 2019-10-29 04:11:13 PDT
I can't reproduce this.
Comment 2 Michael Catanzaro 2019-10-29 07:08:14 PDT Comment hidden (obsolete)
Comment 3 Michael Catanzaro 2019-11-06 07:25:15 PST
Note this is a blocker for Tech Preview, we will stay with 2.26 indefinitely until this is fixed.
Comment 4 Carlos Garcia Campos 2019-11-07 00:40:20 PST
We can't fix something we can't reproduce unless we get more info. I don't see why this would be a blocker if it only happened to you.
Comment 5 Michael Catanzaro 2019-11-07 06:47:28 PST
I assume it probably would affect all Tech Preview users, not just me. Have you followed my instruction to downgrade the runtime to reproduce it?

If you're using Tech Preview with runtime commit eac891c0f64c367302e877cd7cc1a79acc5d995f47951c01a1eb563c6616f972 and you can't reproduce, that would be news to me. In that case, I'm not sure how we should proceed, because I no longer have time for debugging WebKit.

If you're not using Tech Preview with runtime commit eac891c0f64c367302e877cd7cc1a79acc5d995f47951c01a1eb563c6616f972, then I suggest doing so, because that should probably allow you to reproduce.
Comment 6 Carlos Garcia Campos 2019-11-07 06:51:04 PST
I'm not using Tech Preview at all, I use jhbuild.
Comment 7 Michael Catanzaro 2019-11-07 08:36:20 PST
(In reply to Michael Catanzaro from comment #2)
> It's 100% reproducible using Tech Preview if you manually downgrade the
> runtime to get a runtime built with 2.27.2:
> 
> $ flatpak update
> --commit=eac891c0f64c367302e877cd7cc1a79acc5d995f47951c01a1eb563c6616f972
> org.gnome.Platform//master

Looks like I copy/pasted the wrong commit, sorry. That runtime actually has 2.26.1. This works to get 2.27.2 and reproduces the issue:

$ flatpak update --commit=829ed46e7c9913831c022592cc19bda29d8ce800f73aacd5abbe74de06de3a8d org.gnome.Platform//master
Comment 8 Michael Catanzaro 2019-11-07 08:49:13 PST
BTW there is no relevant debug output on the command line.

This works fine:

$ flatpak run org.gnome.Epiphany.Devel//master -p

But this does not work:

$ flatpak run org.gnome.Epiphany.Devel//master -i

Problem is (a) no web content is rendered, and (b) if a new tab is created, it will continue loading forever.
Comment 9 Michael Catanzaro 2019-11-08 07:29:03 PST
OK the problem is the network process crashes in a loop, but only in incognito mode. There's no rendering problem, just the page fails to load. We don't show any indication of page load state when there's only one tab, which is why I incorrectly assumed a rendering problem.

Backtrace is interesting. The crash is occurring deep inside WTF::parseES5DateFromNullTerminatedCharacters.

Program terminated with signal SIGSEGV, Segmentation fault.
#0  invalid_name (name=0x0) at ../sysdeps/posix/opendir.c:89
89	  if (__glibc_unlikely (invalid_name (name)))

(gdb) bt full
#0  0x00007f0f6aeb6214 in invalid_name (name=0x0) at ../sysdeps/posix/opendir.c:89
#1  0x00007f0f6aeb6214 in __opendir (name=0x0) at ../sysdeps/posix/opendir.c:89
#2  0x00007f0f6b910213 in WTF::Optional<std::tuple<WTF::String, WTF::String> >::operator*() & (this=0x7f0f210fda90)
    at DerivedSources/ForwardingHeaders/wtf/Optional.h:528
        arguments = 
                    {<WTF::Optional_base<std::tuple<WTF::String, WTF::String> >> = {init_ = 208, storage_ = {dummy_ = 212 '\324', value_ = std::tuple containing = {[1] = {static MaxLength = 2147483647, m_impl = {static isRefPtr = <error reading variable: Missing ELF symbol "WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::isRefPtr".>, m_ptr = 0x7f0f643d3010}}, [2] = {static MaxLength = 2147483647, m_impl = {static isRefPtr = <error reading variable: Missing ELF symbol "WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::isRefPtr".>, m_ptr = 0x7f0f6b9194d4 <IPC::handleMessage<Messages::DrawingAreaProxy::DidUpdateBackingStoreState, WebKit::DrawingAreaProxy, void (WebKit::DrawingAreaProxy::*)(unsigned long, WebKit::UpdateInfo const&, WebKit::LayerTreeContext const&)>(IPC::Decoder&, WebKit::DrawingAreaProxy*, void (WebKit::DrawingAreaProxy::*)(unsigned long, WebKit::UpdateInfo const&, WebKit::LayerTreeContext const&))+12>}}}}}, <No data fields>}
        completionHandler = 
              {m_function = {m_callableWrapper = std::unique_ptr<class WTF::Detail::CallableWrapperBase<void, bool>> = {get() = 0x7f0f643d200c}}}
#3  0x00007f0f6b910213 in IPC::handleMessageSynchronous<Messages::PluginControllerProxy::HandleEditingCommand, WebKit::PluginControllerProxy, void (WebKit::PluginControllerProxy::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void (bool)>&&)>(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder, std::default_delete<IPC::Encoder> >&, WebKit::PluginControllerProxy*, void (WebKit::PluginControllerProxy::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void (bool)>&&))
    (connection=..., decoder=..., replyEncoder=std::unique_ptr<class IPC::Encoder> = {...}, object=0x0, function=
    (void (WebKit::PluginControllerProxy::*)(class WebKit::PluginControllerProxy * const, const class WTF::String &, const class WTF::String &, class WTF::CompletionHandler<void(bool)> &&)) 0x7f0f643d2000, this adjustment 139704083128744) at ../Source/WebKit/Platform/IPC/HandleMessage.h:148
        arguments = 
                    {<WTF::Optional_base<std::tuple<WTF::String, WTF::String> >> = {init_ = 208, storage_ = {dummy_ = 212 '\324', value_ = std::tuple containing = {[1] = {static MaxLength = 2147483647, m_impl = {static isRefPtr = <error reading variable: Missing ELF symbol "WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::isRefPtr".>, m_ptr = 0x7f0f643d3010}}, [2] = {static MaxLength = 2147483647, m_impl = {static isRefPtr = <error reading variable: Missing ELF symbol "WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::isRefPtr".>, m_ptr = 0x7f0f6b9194d4 <IPC::handleMessage<Messages::DrawingAreaProxy::DidUpdateBackingStoreState, WebKit::DrawingAreaProxy, void (WebKit::DrawingAreaProxy::*)(unsigned long, WebKit::UpdateInfo const&, WebKit::LayerTreeContext const&)>(IPC::Decoder&, WebKit::DrawingAreaProxy*, void (WebKit::DrawingAreaProxy::*)(unsigned long, WebKit::UpdateInfo const&, WebKit::LayerTreeContext const&))+12>}}}}}, <No data fields>}
        completionHandler = 
              {m_function = {m_callableWrapper = std::unique_ptr<class WTF::Detail::CallableWrapperBase<void, bool>> = {get() = 0x7f0f643d200c}}}
#4  0x00007f0f69ec54be in g_main_dispatch
    (context=0x7f0f698557d0 <WTF::StringImpl::replace(char16_t, unsigned char const*, unsigned int)+512>)
    at ../glib/gmain.c:3174
        dispatch = 0x555701dd6110
        prev_source = <optimized out>
        was_in_call = 0
        user_data = 0x7f0f643d2000
        callback = 0x7f0f698557b0 <WTF::StringImpl::replace(char16_t, unsigned char const*, unsigned int)+480>
        cb_funcs = 0x7f0f698557bd <WTF::StringImpl::replace(char16_t, unsigned char const*, unsigned int)+493>
        cb_data = 0x7f0f69f9a280 <g_source_callback_funcs>
        need_destroy = <optimized out>
        source = 0x555701dd61a0
        current = 0x555701dd61a0
        i = 0
        __func__ = "g_main_dispatch"
#5  0x00007f0f69ec54be in g_main_context_dispatch (context=0x7f0f698557d0 <WTF::StringImpl::replace(char16_t, unsigned char const*, unsigned int)+512>) at ../glib/gmain.c:3850
#6  0x00007f0f69ec5870 in g_main_context_iterate (context=0x555701dd6050, block=-1, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3894
        max_priority = 2147483647
        timeout = -1
        some_ready = <optimized out>
        nfds = <optimized out>
        allocated_nfds = <optimized out>
        fds = 0x0
#7  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701dd4be0) at ../glib/gmain.c:4115
        gais_temp = 1
        __func__ = "g_main_loop_run"
#8  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
        noUpper = <optimized out>
        ored = <optimized out>
        length = <optimized out>
        data16 = 0x555701dd5ae0 u"嬀ǝ啗"
        newImpl = <optimized out>
        status = 32527
        realLength = <optimized out>
#9  0x00007f0f643f81b0 in  ()
#10 0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
        longResult = 139704083120536
        postParsePosition = 0x7f0f210fe700 ""
        daysPerMonth = {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}
        year = 0
        month = 1
        day = 1
        hours = 0
        minutes = 0
        seconds = 0
        timeZoneSeconds = 0
        currentPosition = <optimized out>
        dateSeconds = <optimized out>
#11 0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
        postParsePosition = 0x7f0f210fe700 ""
        daysPerMonth = {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}
        year = 0
        month = 1
        day = 1
        hours = 0
        minutes = 0
        seconds = 0
        timeZoneSeconds = 0
        currentPosition = <optimized out>
        dateSeconds = <optimized out>
#12 0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
        daysPerMonth = {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}
        year = 0
        month = 1
        day = 1
        hours = 0
        minutes = 0
        seconds = 0
        timeZoneSeconds = 0
        currentPosition = <optimized out>
        dateSeconds = <optimized out>
#13 0x00007f0f210fe700 in  ()
#14 0x75d8cbe0ef0b9cfa in  ()
#15 0x75d846ca9f899cfa in  ()
#16 0x0000000000000000 in  ()

That looked weird, and it doesn't look like the main thread either, so I decided to get a backtrace of all threads. Looks l ike many of them are all struck inside the same function, WTF::parseES5DateFromNullTerminatedCharacters:

Thread 10 (Thread 0x7f0f2327c700 (LWP 212)):
#0  0x00007f0f6aee38ef in __GI___poll (fds=0x555701d927e0, nfds=1, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007f0f69ec57de in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x555701d927e0, timeout=-1, context=0x555701d929f0) at ../glib/gmain.c:4217
#2  0x00007f0f69ec57de in g_main_context_iterate (context=0x555701d929f0, block=<optimized out>, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3918
#3  0x00007f0f69ec5913 in g_main_context_iteration (context=0x555701d927e0, may_block=1) at ../glib/gmain.c:3980
#4  0x00007f0f69ec5961 in glib_worker_main (data=0x555701d927e0) at ../glib/gmain.c:5867
#5  0x0000555701d92c00 in  ()
#6  0x00007f0f69eeef41 in g_thread_proxy (data=0x7f0f69f9aaa8 <g.unix_signal_lock_lock>) at ../glib/gthread.c:798
#7  0x00007f0f679ae5e2 in start_thread (arg=<optimized out>) at pthread_create.c:479
#8  0x00007f0f6aeee413 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 9 (Thread 0x7f0f242fd700 (LWP 210)):
#0  0x00007f0f6aee38ef in __GI___poll (fds=0x555701d76fc0, nfds=1, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007f0f69ec57de in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x555701d76fc0, timeout=-1, context=0x555701d764a0) at ../glib/gmain.c:4217
#2  0x00007f0f69ec57de in g_main_context_iterate (context=0x555701d764a0, block=<optimized out>, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3918
#3  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701d76d90) at ../glib/gmain.c:4115
#4  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
#5  0x00007f0f643f8018 in  ()
#6  0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
#7  0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
#8  0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
#9  0x00007f0f242fd700 in  ()
#10 0x75d8c1a0cf0b9cfa in  ()
#11 0x75d846ca9f899cfa in  ()
#12 0x0000000000000000 in  ()

Thread 8 (Thread 0x7f0f23afc700 (LWP 211)):
#0  0x00007f0f6aee38ef in __GI___poll (fds=0x555701d796d0, nfds=1, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007f0f69ec57de in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x555701d796d0, timeout=-1, context=0x555701d78dc0) at ../glib/gmain.c:4217
#2  0x00007f0f69ec57de in g_main_context_iterate (context=0x555701d78dc0, block=<optimized out>, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3918
#3  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701d76fe0) at ../glib/gmain.c:4115
#4  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
#5  0x00007f0f643f8078 in  ()
#6  0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
#7  0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
#8  0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
#9  0x00007f0f23afc700 in  ()
#10 0x75d8cea02f0b9cfa in  ()
#11 0x75d846ca9f899cfa in  ()
#12 0x0000000000000000 in  ()

Thread 7 (Thread 0x7f0f64c5b700 (LWP 209)):
#0  0x00007f0f679b503a in futex_abstimed_wait_cancelable (private=0, abstime=0x7f0f64c5abf0, clockid=<optimized out>, expected=0, futex_word=0x7f0f69b7d408) at ../sysdeps/unix/sysv/linux/futex-internal.h:208
#1  0x00007f0f679b503a in __pthread_cond_wait_common (abstime=0x7f0f64c5abf0, clockid=<optimized out>, mutex=0x555701d75410, cond=0x7f0f69b7d3e0) at pthread_cond_wait.c:520
#2  0x00007f0f679b503a in __pthread_cond_timedwait (cond=0x7f0f69b7d3e0, mutex=0x555701d75410, abstime=0x7f0f64c5abf0) at pthread_cond_wait.c:656
#3  0x00007f0f698668e9 in WTF::Unicode::calculateStringHashAndLengthFromUTF8MaskingTop8Bits(char const*, char const*, unsigned int&, unsigned int&) (data=0x7f0f69b7d408 "", dataEnd=<optimized out>, dataLength=@0x7f0f69b7d448: 0, utf16Length=@0x7f0f679b503a: 4026547528) at ../Source/WTF/wtf/unicode/UTF8Conversion.cpp:120
#4  0x0000000000000064 in  ()
#5  0x000000005dc587cb in  ()
#6  0x00000000151312f2 in  ()
#7  0x0000000000000000 in  ()

Thread 6 (Thread 0x7f0f2227a700 (LWP 214)):
#0  0x00007f0f6aee38ef in __GI___poll (fds=0x555701d80ef0, nfds=2, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007f0f69ec57de in g_main_context_poll (priority=2147483647, n_fds=2, fds=0x555701d80ef0, timeout=-1, context=0x555701d804e0) at ../glib/gmain.c:4217
#2  0x00007f0f69ec57de in g_main_context_iterate (context=0x555701d804e0, block=<optimized out>, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3918
#3  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701d803f0) at ../glib/gmain.c:4115
#4  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
#5  0x00007f0f643f80c0 in  ()
#6  0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
#7  0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
#8  0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
#9  0x00007f0f2227a700 in  ()
#10 0x75d8cdb06f0b9cfa in  ()
#11 0x75d846ca9f899cfa in  ()
#12 0x0000000000000000 in  ()

Thread 5 (Thread 0x7f0f208fd700 (LWP 217)):
#0  0x00007f0f6aee38ef in __GI___poll (fds=0x555701d801a0, nfds=1, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007f0f69ec57de in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x555701d801a0, timeout=-1, context=0x555701dd8680) at ../glib/gmain.c:4217
#2  0x00007f0f69ec57de in g_main_context_iterate (context=0x555701dd8680, block=<optimized out>, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3918
#3  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701dd8770) at ../glib/gmain.c:4115
#4  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
#5  0x00007f0f643f81e0 in  ()
#6  0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
#7  0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
#8  0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
#9  0x00007f0f208fd700 in  ()
#10 0x75d8c8e0cf0b9cfa in  ()
#11 0x75d846ca9f899cfa in  ()
#12 0x0000000000000000 in  ()

Thread 4 (Thread 0x7f0f218ff700 (LWP 215)):
#0  0x00007f0f6aee38ef in __GI___poll (fds=0x555701dd4b90, nfds=1, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007f0f69ec57de in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x555701dd4b90, timeout=-1, context=0x555701dd41f0) at ../glib/gmain.c:4217
#2  0x00007f0f69ec57de in g_main_context_iterate (context=0x555701dd41f0, block=<optimized out>, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3918
#3  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701dd4ab0) at ../glib/gmain.c:4115
#4  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
#5  0x00007f0f643f8180 in  ()
#6  0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
#7  0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
#8  0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
#9  0x00007f0f218ff700 in  ()
#10 0x75d8cae08f0b9cfa in  ()
#11 0x75d846ca9f899cfa in  ()
#12 0x0000000000000000 in  ()

Thread 3 (Thread 0x7f0f64c6c9c0 (LWP 207)):
#0  0x00007f0f69ed5fe0 in memcpy (__len=140722217361808, __src=0x555701d55800, __dest=0x0) at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34
#1  0x00007f0f69ed5fe0 in quark_new (string=0x0) at ../glib/gquark.c:296
#2  0x00007f0f69ed5fe0 in quark_from_string (duplicate=1, string=<optimized out>) at ../glib/gquark.c:189
#3  0x00007f0f69ed5fe0 in quark_from_string (duplicate=1, string=<optimized out>) at ../glib/gquark.c:180
#4  0x00007f0f69ed5fe0 in quark_intern_string_locked (duplicate=1, string=<optimized out>) at ../glib/gquark.c:324
#5  0x00007f0f69ed5fe0 in g_intern_string (string=<optimized out>) at ../glib/gquark.c:350
#6  0x0000000000000003 in  ()
#7  0x201cfa3dcc650600 in  ()
#8  0x00007ffc71c72190 in  ()
#9  0x00007f0f23299ae5 in gvfs_dbus_mount_tracker_proxy_get_type () at common/gvfsdbus.c:4771
#10 0x00007f0f23299f48 in gvfs_dbus_mount_tracker_proxy_new_for_bus_sync (bus_type=bus_type@entry=G_BUS_TYPE_SESSION, flags=(G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | unknown: 589924672), flags@entry=(G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS), name=name@entry=0x7f0f232ed264 "org.gtk.vfs.Daemon", object_path=object_path@entry=0x7f0f232ed24a "/org/gtk/vfs/mounttracker", cancellable=0x7f0f6a1d8e38, cancellable@entry=0x0, error=0x0, error@entry=0x7ffc71c72190) at common/gvfsdbus.c:5083
#11 0x00007f0f232e0d26 in create_mount_tracker_proxy (error=error@entry=0x0) at ../client/gdaemonvfs.c:571
#12 0x00007f0f232e1519 in fill_mountable_info (vfs=0x555701d9e2e0 [GDaemonVfs]) at ../client/gdaemonvfs.c:602
#13 0x00007f0f232e1519 in g_daemon_vfs_init (vfs=0x555701d9e2e0 [GDaemonVfs]) at ../client/gdaemonvfs.c:306
#14 0x00007f0f69fd531d in g_type_create_instance (type=<optimized out>) at ../gobject/gtype.c:1866
#15 0x00007f0f69fb73c5 in g_object_new_internal (class=class@entry=0x555701ddb4e0, params=params@entry=0x0, n_params=n_params@entry=0) at ../gobject/gobject.c:1827
#16 0x00007f0f69fb8ac5 in g_object_new_with_properties (object_type=0x555701d88f00 [GDaemonVfs/NITOR], n_properties=0, names=names@entry=0x0, values=values@entry=0x0) at ../gobject/gobject.c:1995
#17 0x00007f0f69fb9671 in g_object_new (object_type=<optimized out>, first_property_name=<optimized out>) at ../gobject/gobject.c:1667
#18 0x00007f0f6a07e5e4 in g_io_extension_get_name (extension=0x7f0f232ac582) at ../gio/giomodule.c:1562
#19 0x00007ffc71c72730 in  ()
#20 0x0000000000000002 in  ()
#21 0x00007f0f6a15538b in  () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#22 0x201cfa3dcc650600 in  ()
#23 0x00007f0f6a0c03f0 in g_volume_monitor_adopt_orphan_mount (mount=0x7f0f232ac582) at ../gio/gunionvolumemonitor.c:617
#24 0x0000555701d7ba60 in  ()
#25 0x00007f0f6a07eaee in _g_io_module_get_default (extension_point=0x555701d75aa0 "\020\221\330\001WU", envvar=0x7f0f6a15538b "o-nfs-file-monitor", verify_func=0x555701d75aa0) at ../gio/giomodule.c:950
#26 0x00007f0f6a0685b2 in g_file_monitor_file (file=0x7f0f643f8268, flags=(unknown: 1681875072), cancellable=0x7f0f643f6080, error=0x7ffc71c72738) at ../gio/gfile.c:5487
#27 0x0000000000000000 in  ()

Thread 2 (Thread 0x7f0f22a7b700 (LWP 213)):
#0  0x00007f0f69f0c9a5 in variant_type_string_scan_internal (string=<optimized out>, limit=0x555701d81150 "a{sv}", endptr=0x555701ddb642, depth=0x2, depth_limit=6) at ../glib/gvarianttype.c:220
#1  0x00007f0f6a10fb58 in parse_value_from_blob (buf=0x555701ddb642, type=0x7f0f22a7a9a0, just_align=581413032, indent=1777341490, error=0x555701da52d0) at ../gio/gdbusmessage.c:1517
#2  0x00007f0f6a10fb1f in parse_value_from_blob (buf=0x7f0f22a7a9a0, type=0x7f0f22a7a8a8, just_align=31307329, indent=4, error=0x7f0f22a7aad0) at ../gio/gdbusmessage.c:1630
#3  0x00007f0f6a111cc2 in validate_headers (message=0x7f0f22a7aad0, error=0x555701ddb3c0) at ../gio/gdbusmessage.c:1279
#4  0x0000000000000005 in  ()
#5  0x0000000000000098 in  ()
#6  0x0000000000000098 in  ()
#7  0x0000000000000098 in  ()
#8  0x0000555701d812a0 in  ()
#9  0x0000000000000001 in  ()
#10 0x0000000000000001 in  ()
#11 0x0000555701da6e00 in  ()
#12 0x0000000000000004 in  ()
#13 0x0000000000000004 in  ()
#14 0x0000000000000000 in  ()

Thread 1 (Thread 0x7f0f210fe700 (LWP 216)):
#0  0x00007f0f6aeb6214 in invalid_name (name=0x0) at ../sysdeps/posix/opendir.c:89
#1  0x00007f0f6aeb6214 in __opendir (name=0x0) at ../sysdeps/posix/opendir.c:89
#2  0x00007f0f6b910213 in WTF::Optional<std::tuple<WTF::String, WTF::String> >::operator*() & (this=0x7f0f210fda90) at DerivedSources/ForwardingHeaders/wtf/Optional.h:528
#3  0x00007f0f6b910213 in IPC::handleMessageSynchronous<Messages::PluginControllerProxy::HandleEditingCommand, WebKit::PluginControllerProxy, void (WebKit::PluginControllerProxy::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void (bool)>&&)>(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder, std::default_delete<IPC::Encoder> >&, WebKit::PluginControllerProxy*, void (WebKit::PluginControllerProxy::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void (bool)>&&)) (connection=..., decoder=..., replyEncoder=std::unique_ptr<class IPC::Encoder> = {...}, object=0x0, function=(void (WebKit::PluginControllerProxy::*)(class WebKit::PluginControllerProxy * const, const class WTF::String &, const class WTF::String &, class WTF::CompletionHandler<void(bool)> &&)) 0x7f0f643d2000, this adjustment 139704083128744) at ../Source/WebKit/Platform/IPC/HandleMessage.h:148
#4  0x00007f0f69ec54be in g_main_dispatch (context=0x7f0f698557d0 <WTF::StringImpl::replace(char16_t, unsigned char const*, unsigned int)+512>) at ../glib/gmain.c:3174
#5  0x00007f0f69ec54be in g_main_context_dispatch (context=0x7f0f698557d0 <WTF::StringImpl::replace(char16_t, unsigned char const*, unsigned int)+512>) at ../glib/gmain.c:3850
#6  0x00007f0f69ec5870 in g_main_context_iterate (context=0x555701dd6050, block=-1, dispatch=1, self=<optimized out>) at ../glib/gmain.c:3894
#7  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701dd4be0) at ../glib/gmain.c:4115
#8  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale() (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
#9  0x00007f0f643f81b0 in  ()
#10 0x00007f0f6980a188 in WTF::parseInt (result=<synthetic pointer>, base=10, stopPosition=0x7f0f69856250 <WTF::StringImpl::convertToLowercaseWithoutLocale()+160>, string=0x0) at ../Source/WTF/wtf/DateMath.cpp:632
#11 0x00007f0f6980a188 in WTF::parseES5DatePortion (day=<synthetic pointer>: <optimized out>, month=<synthetic pointer>: <optimized out>, year=<synthetic pointer>: <optimized out>, currentPosition=0x0) at ../Source/WTF/wtf/DateMath.cpp:657
#12 0x00007f0f6980a188 in WTF::parseES5DateFromNullTerminatedCharacters(char const*) (dateString=0x0) at ../Source/WTF/wtf/DateMath.cpp:803
#13 0x00007f0f210fe700 in  ()
#14 0x75d8cbe0ef0b9cfa in  ()
#15 0x75d846ca9f899cfa in  ()
#16 0x0000000000000000 in  ()
Comment 10 Michael Catanzaro 2019-11-08 07:34:48 PST
(BTW this is a backtrace of the network process.)

This part here is insane:

(In reply to Michael Catanzaro from comment #9)
> #7  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701dd4be0) at
> ../glib/gmain.c:4115
>         gais_temp = 1
>         __func__ = "g_main_loop_run"
> #8  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale()
> (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
>         noUpper = <optimized out>
>         ored = <optimized out>
>         length = <optimized out>
>         data16 = 0x555701dd5ae0 u"嬀ǝ啗"
>         newImpl = <optimized out>
>         status = 32527
>         realLength = <optimized out>

I don't understand how WTF::StringImpl::convertToLowercaseWithoutLocale could possibly result in a call to g_main_loop_run(). Maybe the backtrace is somehow corrupted? That's just too weird for a compiler optimization? :/ Incredibly, four other threads are doing the same thing.
Comment 11 Carlos Alberto Lopez Perez 2019-11-08 09:15:51 PST
(In reply to Michael Catanzaro from comment #9)
> OK the problem is the network process crashes in a loop, but only in
> incognito mode. There's no rendering problem, just the page fails to load.
> We don't show any indication of page load state when there's only one tab,
> which is why I incorrectly assumed a rendering problem.
> 
> Backtrace is interesting. The crash is occurring deep inside
> WTF::parseES5DateFromNullTerminatedCharacters.
> 

I'm just guessing here just based on the name of the function that crashes ...
can it be triggered by different/wrong time-zone or time-settings inside flatpak?
Comment 12 Carlos Alberto Lopez Perez 2019-11-08 09:21:27 PST
(In reply to Michael Catanzaro from comment #10)
> (BTW this is a backtrace of the network process.)
> 
> This part here is insane:
> 
> (In reply to Michael Catanzaro from comment #9)
> > #7  0x00007f0f69ec5b63 in g_main_loop_run (loop=0x555701dd4be0) at
> > ../glib/gmain.c:4115
> >         gais_temp = 1
> >         __func__ = "g_main_loop_run"
> > #8  0x00007f0f69856250 in WTF::StringImpl::convertToLowercaseWithoutLocale()
> > (this=<optimized out>) at ../Source/WTF/wtf/text/StringImpl.cpp:353
> >         noUpper = <optimized out>
> >         ored = <optimized out>
> >         length = <optimized out>
> >         data16 = 0x555701dd5ae0 u"嬀ǝ啗"
> >         newImpl = <optimized out>
> >         status = 32527
> >         realLength = <optimized out>
> 
> I don't understand how WTF::StringImpl::convertToLowercaseWithoutLocale
> could possibly result in a call to g_main_loop_run(). Maybe the backtrace is
> somehow corrupted? That's just too weird for a compiler optimization? :/
> Incredibly, four other threads are doing the same thing.

Indeed, that's very very strange... something really weird is happening there.
Comment 13 Michael Catanzaro 2019-11-08 09:26:28 PST
(In reply to Michael Catanzaro from comment #9)
> #3  0x00007f0f6b910213 in
> IPC::handleMessageSynchronous<Messages::PluginControllerProxy::
> HandleEditingCommand, 

I not sure I trust this backtrace. I took it before I realized that I probably need to manually downgrade my org.gnome.Sdk.Debug extension to make sure that was in sync with the downgraded org.gnome.Platform runtime. But that didn't work: it just causes all WebKit frames to disappear entirely from the backtrace as if no debuginfo for WebKit is available:

(gdb) bt
#0  0x00007f7061c12214 in invalid_name (name=0x0) at ../sysdeps/posix/opendir.c:89
#1  0x00007f7061c12214 in __opendir (name=0x0) at ../sysdeps/posix/opendir.c:89
#2  0x00007f706266c213 in  () at /usr/lib/x86_64-linux-gnu/libwebkit2gtk-4.0.so.37
#3  0x00007f70626754d4 in  () at /usr/lib/x86_64-linux-gnu/libwebkit2gtk-4.0.so.37
#4  0x00007f7060564b85 in WTF::RunLoop::performWork() () at /usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.0.so.18
#5  0x00007f70605b17bd in  () at /usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.0.so.18
#6  0x00007f7060c214be in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#7  0x00007f7060c21870 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x00007f7060c21b63 in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#9  0x00007f70605b2250 in WTF::RunLoop::run() () at /usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.0.so.18
#10 0x00007f7060566188 in WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) ()
    at /usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.0.so.18
#11 0x00007f70605b25cd in  () at /usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.0.so.18
#12 0x00007f705e70a5e2 in start_thread (arg=<optimized out>) at pthread_create.c:479
#13 0x00007f7061c4a413 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

The non-WebKit frames are the same, though, so I think we can probably trust those.

(In reply to Carlos Alberto Lopez Perez from comment #11)
> I'm just guessing here just based on the name of the function that crashes
> ...
> can it be triggered by different/wrong time-zone or time-settings inside
> flatpak?

Not likely. Inside the sandbox, /etc/localtime is a symlink to /run/host/monitor/localtime, which presumably matches the host's. Never had a problem with timezone before. And date works properly:

[📦 org.gnome.Epiphany.Devel ~]$ date
Fri 08 Nov 2019 11:20:43 AM CST
Comment 14 Michael Catanzaro 2019-11-08 09:30:38 PST
There's only one direct call to opendir() in NetworkProcess, in NetworkCacheFileSystem.cpp. But it hasn't changed in six months.
Comment 15 Carlos Garcia Campos 2019-11-11 01:56:27 PST
The WebKit symbol names in the backtrace are all wrong, it's mixing symbols from different web processes, so there must be something wrong in your system.
Comment 16 Michael Catanzaro 2019-11-11 05:48:32 PST
As I explained:

(In reply to Michael Catanzaro from comment #13) 
> I not sure I trust this backtrace. I took it before I realized that I
> probably need to manually downgrade my org.gnome.Sdk.Debug extension to make
> sure that was in sync with the downgraded org.gnome.Platform runtime. But
> that didn't work: it just causes all WebKit frames to disappear entirely
> from the backtrace as if no debuginfo for WebKit is available:

To get a good backtrace, one would need to rebuild the Epiphany flatpak using either a modified runtime (e.g. with flapjak), or (easier) with bundled WebKit using GNOME Builder (-DCMAKE_INSTALL_PREFIX=/app will probably suffice, might also need to use -DLIB_INSTALL_DIR=/app/lib), and hope to reproduce the problem that way. This is just what you need to do when a problem can only be reproduced under flatpak. To solve this bug, someone else will have to learn how to do this, since I do not intend to spend this much effort debugging WebKit anymore.

If that fails, we can upgrade the GNOME runtime to 2.27.2 briefly for debugging, and then immediately downgrade it back to 2.26, but since this will briefly break users I won't do that unless attempts to rebuild locally have failed.
Comment 17 Philippe Normand 2019-11-12 03:05:19 PST
0x00007ffff49d5214 in opendir () from /usr/lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
#0  0x00007ffff49d5214 in opendir () at /usr/lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff5437333 in _ZN6WebKit12NetworkCache17traverseDirectoryERKN3WTF6StringERKNS1_8FunctionIFvS4_NS0_18DirectoryEntryTypeEEEE (path="(null)", function=...)
    at DerivedSources/ForwardingHeaders/wtf/text/CString.h:67
#2  0x00007ffff543f844 in WebKit::NetworkCache::Storage::<lambda()>::operator() (__closure=<optimized out>) at /usr/include/c++/9.2.0/bits/unique_ptr.h:278
#3  0x00007ffff543f844 in WTF::Detail::CallableWrapper<WebKit::NetworkCache::Storage::deleteOldVersions()::<lambda()>, void>::call(void) (this=0x7fffedefa190)
    at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#4  0x00007ffff33111d5 in _ZNK3WTF8FunctionIFvvEEclEv (this=<synthetic pointer>) at Source/WTF/wtf/Lock.h:84
#5  0x00007ffff33111d5 in _ZN3WTF7RunLoop11performWorkEv (this=0x7fffeded3000) at Source/WTF/wtf/RunLoop.cpp:123
#6  0x00007ffff337142d in WTF::RunLoop::<lambda(gpointer)>::operator() (__closure=0x0, userData=<optimized out>) at Source/WTF/wtf/glib/RunLoopGLib.cpp:68
#7  0x00007ffff337142d in WTF::RunLoop::<lambda(gpointer)>::_FUN(gpointer) () at Source/WTF/wtf/glib/RunLoopGLib.cpp:70
#8  0x00007ffff39e34de in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#9  0x00007ffff39e3890 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007ffff39e3b83 in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007ffff3371f60 in _ZN3WTF7RunLoop3runEv () at Source/WTF/wtf/glib/RunLoopGLib.cpp:96
#12 0x00007ffff3312c28 in _ZNK3WTF8FunctionIFvvEEclEv (this=<synthetic pointer>) at Source/WTF/wtf/Function.h:76
#13 0x00007ffff3312c28 in _ZN3WTF6Thread10entryPointEPNS0_16NewThreadContextE (newThreadContext=0x7fffedef7120) at Source/WTF/wtf/Threading.cpp:148
#14 0x00007ffff33722ed in WTF::wtfThreadEntryPoint(void*) (context=<optimized out>) at Source/WTF/wtf/posix/ThreadingPOSIX.cpp:200
#15 0x00007ffff147e5e2 in  () at /usr/lib/x86_64-linux-gnu/libpthread.so.0
#16 0x00007ffff4a0d413 in clone () at /usr/lib/x86_64-linux-gnu/libc.so.6
Comment 18 Philippe Normand 2019-11-12 03:08:06 PST
Created attachment 383345 [details]
flapjak manifest

flatpak-builder --force-clean --user build-dir/ org.gnome.Epiphany.yaml
flatpak-builder --run build-dir/ org.gnome.Epiphany.yaml bash
$ gdb -args epiphany -i
(gdb) set follow-fork-mode child
(gdb) r
Comment 19 Carlos Garcia Campos 2019-11-12 04:43:15 PST
Interesting, the network cache shouldn't be used at all in incognito mode. The directory path should be empty, causing Storage::open() to fail in FileSystem::makeAllDirectories() and return early.
Comment 20 Philippe Normand 2019-11-14 11:09:19 PST
Well, makeVersionedDirectoryPath() returns a valid path (ex: "/Version 14"), so makeAllDirectories() seems to return true...
Comment 21 Carlos Garcia Campos 2019-11-18 01:02:03 PST
(In reply to Philippe Normand from comment #20)
> Well, makeVersionedDirectoryPath() returns a valid path (ex: "/Version 14"),
> so makeAllDirectories() seems to return true...

I don't understand why, it should fail due to permissions to write to /
Comment 22 Carlos Garcia Campos 2019-11-18 01:13:37 PST
Created attachment 383735 [details]
Patch

I hope this fixes the issue
Comment 23 Carlos Garcia Campos 2019-11-18 01:14:09 PST
Thank you very much Phil for the help debugging this mess.
Comment 24 youenn fablet 2019-11-18 01:22:38 PST
Comment on attachment 383735 [details]
Patch

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

> Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:68
> +        return nullptr;

I would think this check is not needed.
Cache::open should only be called if sessionID.isEphemeral() is not true, hence cachePath not being null.
Is it possible that GTK port is using 'ephemeral' sessions with sessionID.isEphemeral() being false?
Comment 25 Carlos Garcia Campos 2019-11-18 04:07:35 PST
(In reply to youenn fablet from comment #24)
> Comment on attachment 383735 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=383735&action=review
> 
> > Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:68
> > +        return nullptr;
> 
> I would think this check is not needed.
> Cache::open should only be called if sessionID.isEphemeral() is not true,
> hence cachePath not being null.
> Is it possible that GTK port is using 'ephemeral' sessions with
> sessionID.isEphemeral() being false?

The network process always creates a default non ephemeral session in NetworkProcess::initializeNetworkProcess().
Comment 26 Michael Catanzaro 2019-11-18 06:22:20 PST
soup ports have never supported creation of non-default sessions (since our API does not, and probably will not ever, expose this). Our code persistently assumes that defaultSessionID() may be used anywhere and that non-default session IDs are used only by Cocoa-specific code. This probably used to be true when SessionID was introduced but now we can see it's become a problem. Changing that should be possible, but likely not trivial as it requires auditing everywhere that uses defaultSessionID() and many places are going to require changes.
Comment 27 Michael Catanzaro 2019-11-18 06:24:45 PST
(In reply to Carlos Garcia Campos from comment #25)
> The network process always creates a default non ephemeral session in
> NetworkProcess::initializeNetworkProcess().

Anyway, changing this would be a good first step, but will almost surely break other features in ephemeral sessions, because the session ID will unexpectedly no longer match defaultSessionID().
Comment 28 Carlos Garcia Campos 2019-11-22 01:31:21 PST
Ping reviewers, please.
Comment 29 Michael Catanzaro 2019-11-22 09:00:18 PST
It needs owner approval from Youenn.

FWIW: I think it's a great workaround in that it seems perfectly reasonable to add an early return here. I almost requested a FIXME comment, but I don't think it's really needed since it just seems generally reasonable to do.

But surely it's much more important to fix our use of SessionID. It's terrifying that currently SessionID::isEphemeral will always return false in ephemeral sessions. If we commit this solution now, there should be a new bug report with a plan to fix that.
Comment 30 Michael Catanzaro 2019-11-22 09:05:23 PST
Actually I think I'm wrong. I misunderstood Carlos's comment. WebKitWebsiteDataManager.cpp creates ephemeral WebsiteDataStore using WebsiteDataStore::createNonPersistent, which uses generateEphemeralSessionID. So I guess it's fine. We could still have various problems caused by overuse defaultSessionID(), of course.

(In reply to Carlos Garcia Campos from comment #25)
> The network process always creates a default non ephemeral session in
> NetworkProcess::initializeNetworkProcess().

I understand it's always created, but just not going to be used in ephemeral mode. So then this change makes sense to me as-is.
Comment 31 youenn fablet 2019-11-22 10:12:49 PST
Comment on attachment 383735 [details]
Patch

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

>>> Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:68
>>> +        return nullptr;
>> 
>> I would think this check is not needed.
>> Cache::open should only be called if sessionID.isEphemeral() is not true, hence cachePath not being null.
>> Is it possible that GTK port is using 'ephemeral' sessions with sessionID.isEphemeral() being false?
> 
> The network process always creates a default non ephemeral session in NetworkProcess::initializeNetworkProcess().

Let's make it GTK specific for now and add an ASSERT(!cachePath.isNull()) for other ports.
If other ports are like GTK, let's make the ASSERT COCOA specific.
Comment 32 Carlos Garcia Campos 2019-11-25 01:45:12 PST
Created attachment 384281 [details]
Patch
Comment 33 Michael Catanzaro 2019-11-25 07:23:09 PST
Comment on attachment 384281 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        REGRESSION(2.27.2): [GTK] Incognito mode is broken under flatpak

Does it really have anything to do with flatpak?

> Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:75
> +#if PLATFORM(GTK) || PLATFORM(WPE)
> +    // This can only happen when the main WebsiteDataStore is ephemeral. Since the network process always creates a default
> +    // non-ephemeral session, this is called with the cache path of the WebsiteDataStore that is nullptr (we don't want any
> +    // disk write in ephemeral contexts).
> +    if (cachePath.isNull())
> +        return nullptr;
> +#else
> +    ASSERT(!cachePath.isNull());
> +#endif

FWIW this comment seems like a good justification to avoid the #ifdefs and use the early return on all ports. But Youenn's decision.

LGTM.
Comment 34 youenn fablet 2019-11-25 07:40:07 PST
I change my mind a bit, sorry...

It might indeed be ok to have a non ephemeral session (say for IDB) but still want to have an ephemeral HTTP cache.
In that case, let's add the check at NetworkCache::Cache::open call site instead of inside.
That will make things clearer and will also remove the RELEASE_LOG_ERROR which in that case makes no sense.
Comment 35 youenn fablet 2019-11-25 07:41:00 PST
Comment on attachment 384281 [details]
Patch

r=me with the if check moved to NetworkSession::NetworkSession.
Comment 36 youenn fablet 2019-11-25 07:41:26 PST
(In reply to youenn fablet from comment #35)
> Comment on attachment 384281 [details]
> Patch
> 
> r=me with the if check moved to NetworkSession::NetworkSession.

To be clear, the if check without #ifdef.
Comment 37 Carlos Garcia Campos 2019-11-26 00:29:48 PST
(In reply to Michael Catanzaro from comment #33)
> Comment on attachment 384281 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=384281&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        REGRESSION(2.27.2): [GTK] Incognito mode is broken under flatpak
> 
> Does it really have anything to do with flatpak?

It can only be reproducible under flatpak. It seems that g_mkdir_with_parents("/Version 14", S_IRWXU) doesn't return -1 under flatpak for some reason. I agree it's much better the explicit early return than waiting until makeAllDirectories fails, though.

> > Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:75
> > +#if PLATFORM(GTK) || PLATFORM(WPE)
> > +    // This can only happen when the main WebsiteDataStore is ephemeral. Since the network process always creates a default
> > +    // non-ephemeral session, this is called with the cache path of the WebsiteDataStore that is nullptr (we don't want any
> > +    // disk write in ephemeral contexts).
> > +    if (cachePath.isNull())
> > +        return nullptr;
> > +#else
> > +    ASSERT(!cachePath.isNull());
> > +#endif
> 
> FWIW this comment seems like a good justification to avoid the #ifdefs and
> use the early return on all ports. But Youenn's decision.
> 
> LGTM.
Comment 38 Carlos Garcia Campos 2019-11-26 01:06:29 PST
Created attachment 384336 [details]
Patch for landing
Comment 39 Carlos Garcia Campos 2019-11-26 02:28:16 PST
Committed r252880: <https://trac.webkit.org/changeset/252880>