Crash under WebResourceLoadStatisticsStore::persistentStoragePath(WTF::String const&)

[Chris Dumez]

Crash under WebResourceLoadStatisticsStore::persistentStoragePath(WTF::String const&):
27 WebKit: WebKit::WebResourceLoadStatisticsStore::persistentStoragePath(WTF::String const&) const <==
        27 WebKit: WebKit::WebResourceLoadStatisticsStore::createDecoderFromDisk(WTF::String const&) const
          27 WebKit: WebKit::WebResourceLoadStatisticsStore::createDecoderFromDisk(WTF::String const&) const
            27 WebKit: WebKit::WebResourceLoadStatisticsStore::refreshFromDisk()
              27 libdispatch.dylib: _dispatch_client_callout
                24 libdispatch.dylib: _dispatch_continuation_pop$VARIANT$mp
                | 24 libdispatch.dylib: _dispatch_source_invoke$VARIANT$mp
                |   24 libdispatch.dylib: _dispatch_queue_serial_drain$VARIANT$mp
                |     24 libdispatch.dylib: _dispatch_queue_invoke$VARIANT$mp
                |       24 libdispatch.dylib: _dispatch_root_queue_drain_deferred_wlh$VARIANT$mp
                |         24 libdispatch.dylib: _dispatch_workloop_worker_thread$VARIANT$mp
                |           24 libsystem_pthread.dylib: _pthread_wqthread
                |             24 libsystem_pthread.dylib: 
                pruning: 3 libdispatch.dylib: _dispatch_continuation_pop$VARIANT$armv81

[Chris Dumez]

<rdar://problem/33093552>

[Chris Dumez]

Created attachment 314727 [details]
Patch

[Chris Dumez]

Created attachment 314729 [details]
Patch

[Brent Fulgham]

Comment on attachment 314729 [details]
Patch

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

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:289
> +        String resourceLog = resourceLogFilePath();

This is a good chance. We originally thought we might want multiple files, so the label made sense, but it's just been added complexity. Good riddance!

[Chris Dumez]

(In reply to Brent Fulgham from comment #4)
> Comment on attachment 314729 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=314729&action=review
> 
> > Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:289
> > +        String resourceLog = resourceLogFilePath();
> 
> This is a good chance. We originally thought we might want multiple files,
> so the label made sense, but it's just been added complexity. Good riddance!

Yes, we can introduce the label back when we actually need it.

[Chris Dumez]

Created attachment 314730 [details]
Patch

[Brent Fulgham]

Comment on attachment 314730 [details]
Patch

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

r=me

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:364
> +    return m_statisticsStoragePath.isolatedCopy();

I have to admit I'm shocked that this was necessary -- I guess it's because we might call this method on the work queue?

[Chris Dumez]

Comment on attachment 314730 [details]
Patch

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

>> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:364
>> +    return m_statisticsStoragePath.isolatedCopy();
> 
> I have to admit I'm shocked that this was necessary -- I guess it's because we might call this method on the work queue?

This is definitely called from the background work queue.

[Chris Dumez]

Created attachment 314738 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 314738 [details]
Patch

Clearing flags on attachment: 314738

Committed r219211: <http://trac.webkit.org/changeset/219211>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.