Bug 127794 – Add support for sessions to MemoryCache

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Martin Hock]

Currently, the memory cache ignores sessions.  Different sessions should not see each other's cache.

[Martin Hock]

Created attachment 222488 [details]
patch

[Vicki Pfau]

Comment on attachment 222488 [details]
patch

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

> Source/WebCore/platform/network/cf/ResourceRequest.h:171
> +        const String& setPartitionSession() const { return m_partitionSession = (m_cachePartition.isNull() ? emptyString() : m_cachePartition) + '|' + String::number(m_sessionID); }

Given that this takes no arguments and returns a value, it probably shouldn't be called set*. I'm a bit confused as to why you have it both do an assignment and return the value in one operation, however.

[Build Bot]

Comment on attachment 222488 [details]
patch

Attachment 222488 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6622893324632064

New failing tests:
editing/unsupported-content/list-type-before.html
editing/unsupported-content/table-type-before.html
editing/unsupported-content/list-delete-003.html
editing/unsupported-content/list-delete-001.html
editing/unsupported-content/list-type-after.html
editing/unsupported-content/table-delete-002.html
editing/unsupported-content/table-type-after.html

[Build Bot]

Created attachment 222521 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Martin Hock]

Created attachment 223739 [details]
patch

[Martin Hock]

Created attachment 223766 [details]
Remove extraneous change to WebCache.mm

[Martin Hock]

Created attachment 224535 [details]
Create another MemoryCache level; make WebCore sessionID-aware

[Martin Hock]

Created attachment 224770 [details]
refactor

[Martin Hock]

Created attachment 225304 [details]
split out SessionID changes (129141)

[Martin Hock]

Created attachment 225338 [details]
fix WebCache.mm

[Martin Hock]

Created attachment 225432 [details]
restore MemoryCache::resourceForURL(const URL&)

[Martin Hock]

Created attachment 225435 [details]
build fix

[Martin Hock]

Created attachment 225691 [details]
rebase

[WebKit Commit Bot]

Comment on attachment 225691 [details]
rebase

Clearing flags on attachment: 225691

Committed r165013: <http://trac.webkit.org/changeset/165013>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Comment on attachment 225691 [details]
rebase

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

> Source/WebCore/loader/cache/MemoryCache.cpp:867
> +void MemoryCache::removeRequestFromSessionCaches(ScriptExecutionContext*, const ResourceRequest& request)
> +{
> +    for (auto& resources : memoryCache()->m_sessionResources) {
> +        if (CachedResource* resource = memoryCache()->resourceForRequestImpl(request, *resources.value))
> +        memoryCache()->remove(resource);
> +    }

Unlike all other functions around, this new function is not thread safe, which caused assertion failures on bots.

See <http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r165015%20(3510)/fast/files/workers/worker-apply-blob-url-to-xhr-crash-log.txt>.

> Source/WebCore/loader/cache/MemoryCache.cpp:897
> +    for (auto &resources : m_sessionResources) {

Misplaced &

[WebKit Commit Bot]

Re-opened since this is blocked by bug 129646

[Martin Hock]

Created attachment 225806 [details]
support multithreading in removeRequestFromSessionCaches

[Martin Hock]

Created attachment 225807 [details]
misplaced &

[Martin Hock]

(In reply to comment #16)
> (From update of attachment 225691 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225691&action=review
> 
> > Source/WebCore/loader/cache/MemoryCache.cpp:867
> > +void MemoryCache::removeRequestFromSessionCaches(ScriptExecutionContext*, const ResourceRequest& request)
> > +{
> > +    for (auto& resources : memoryCache()->m_sessionResources) {
> > +        if (CachedResource* resource = memoryCache()->resourceForRequestImpl(request, *resources.value))
> > +        memoryCache()->remove(resource);
> > +    }
> 
> Unlike all other functions around, this new function is not thread safe, which caused assertion failures on bots.
> 
> See <http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r165015%20(3510)/fast/files/workers/worker-apply-blob-url-to-xhr-crash-log.txt>.

This should be fixed - I've run all the tests and I don't think any of the minor flakiness I observe is due to my changes.

> > Source/WebCore/loader/cache/MemoryCache.cpp:897
> > +    for (auto &resources : m_sessionResources) {
> 
> Misplaced &

Fixed in second patch, thanks.

[WebKit Commit Bot]

Comment on attachment 225807 [details]
misplaced &

Clearing flags on attachment: 225807

Committed r165117: <http://trac.webkit.org/changeset/165117>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.