Allow retrieving Cache Storage records for a given URL only

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

For efficiency, allow retrieving Cache Storage records for a given URL only.

[youenn fablet]

Created attachment 319551 [details]
Patch

[Build Bot]

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


ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:159:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:197:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 18 files


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

[youenn fablet]

Created attachment 319596 [details]
Patch

[youenn fablet]

Created attachment 319605 [details]
Patch

[Build Bot]

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


ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:159:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:197:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 18 files


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

[youenn fablet]

Created attachment 319610 [details]
Patch

[Build Bot]

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


ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:159:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:197:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 18 files


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

[youenn fablet]

WPE bots error is unrelated.

[Alex Christensen]

Comment on attachment 319610 [details]
Patch

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

> Source/WebCore/Modules/cache/Cache.cpp:388
> +    retrieveURL.setQuery({ });

Do we want to remove the fragment here, too?

> Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:104
> +    if (keyURL.hasQuery())
> +        keyURL.setQuery({ });
> +    keyURL.removeFragmentIdentifier();

Maybe we should make a method to do both of these at the same time.  That would avoid an unnecessary string allocation and copy.

> Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:116
> +    Vector<Record> newRecords;
> +    newRecords.reserveInitialCapacity(1);
> +    newRecords.uncheckedAppend(WTFMove(record));
> +    return m_records.set(key, WTFMove(newRecords)).iterator->value.last();

{ WTFMove(record) }

[youenn fablet]

Created attachment 319905 [details]
Patch for landing

[Build Bot]

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


ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:159:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:197:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 18 files


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

[youenn fablet]

Thanks for the review.

(In reply to Alex Christensen from comment #9)
> Comment on attachment 319610 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319610&action=review
> 
> > Source/WebCore/Modules/cache/Cache.cpp:388
> > +    retrieveURL.setQuery({ });
> 
> Do we want to remove the fragment here, too?

OK

> > Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:104
> > +    if (keyURL.hasQuery())
> > +        keyURL.setQuery({ });
> > +    keyURL.removeFragmentIdentifier();
> 
> Maybe we should make a method to do both of these at the same time.  That
> would avoid an unnecessary string allocation and copy.

Will do in a follow-up
 
> > Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp:116
> > +    Vector<Record> newRecords;
> > +    newRecords.reserveInitialCapacity(1);
> > +    newRecords.uncheckedAppend(WTFMove(record));
> > +    return m_records.set(key, WTFMove(newRecords)).iterator->value.last();
> 
> { WTFMove(record) }

That is not working.
Probably need a tweak in Vector.h.
I'll try to fix that in a follow-up.

[WebKit Commit Bot]

Comment on attachment 319905 [details]
Patch for landing

Clearing flags on attachment: 319905

Committed r221624: <http://trac.webkit.org/changeset/221624>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693839>