Bug 176055

Summary: Setting the cache storage engine root path according the session WebsiteDataStore
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, cdumez, cgarcia, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description youenn fablet 2017-08-29 09:42:52 PDT
This will allow cache storage per session/websitedatastore.
Ephemeral sessions will have a specific null path so that no access is done at all.
Comment 1 youenn fablet 2017-08-29 09:54:45 PDT
Created attachment 319255 [details]
Patch
Comment 2 Alex Christensen 2017-08-29 13:23:33 PDT
Comment on attachment 319255 [details]
Patch

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

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:53
> +    auto addResult = globalEngineMap().add(sessionID, nullptr);

This can probably still use ensure.

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:357
> +        channel->read(0, std::numeric_limits<size_t>::max(), m_ioQueue.get(), [callback = WTFMove(callback)](const Data& data, int error) mutable {
> +            RunLoop::main().dispatch([callback = WTFMove(callback), data, error]() mutable {

Will this create a copy of data, or just capture a reference to something that will probably be gone?
Comment 3 youenn fablet 2017-08-29 13:45:15 PDT
Comment on attachment 319255 [details]
Patch

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

>> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:53
>> +    auto addResult = globalEngineMap().add(sessionID, nullptr);
> 
> This can probably still use ensure.

I tried it and it was crashing.
I haven't investigated precisely the reason for this crash.

>> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:357
>> +            RunLoop::main().dispatch([callback = WTFMove(callback), data, error]() mutable {
> 
> Will this create a copy of data, or just capture a reference to something that will probably be gone?

It will capture data by copy which will do like capturing a Ref.
I guess the lambda could take a Data&& instead.
Comment 4 WebKit Commit Bot 2017-08-29 14:39:51 PDT
Comment on attachment 319255 [details]
Patch

Clearing flags on attachment: 319255

Committed r221315: <http://trac.webkit.org/changeset/221315>
Comment 5 WebKit Commit Bot 2017-08-29 14:39:53 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2017-08-29 14:40:39 PDT
<rdar://problem/34142875>