Bug 157405

Summary: Don't use DiskCache for media resource loads
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Alex Christensen 2016-05-05 18:58:58 PDT
Don't use DiskCache for media resource loads
Comment 1 Alex Christensen 2016-05-05 19:00:53 PDT
Created attachment 278217 [details]
Patch
Comment 2 WebKit Commit Bot 2016-05-05 19:03:26 PDT
Attachment 278217 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/ResourceRequestBase.h:153:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Commit Bot 2016-05-09 09:53:51 PDT
Comment on attachment 278217 [details]
Patch

Clearing flags on attachment: 278217

Committed r200578: <http://trac.webkit.org/changeset/200578>
Comment 4 WebKit Commit Bot 2016-05-09 09:53:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Carlos Garcia Campos 2016-05-23 03:01:00 PDT
Comment on attachment 278217 [details]
Patch

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

> Source/WebCore/loader/MediaResourceLoader.cpp:67
> +    auto cachingPolicy = options & LoadOption::DisallowCaching ? CachingPolicy::DisallowCaching : CachingPolicy::AllowCaching;

I'm confused. This is not about disk cache, but about memory cache, right? Why is this added as an option? Why not passing CachingPolicy::DisallowCaching unconditionally for media loads? Looking at the code below we disallow caching in the disk cache always for media requests.
Comment 6 Alex Christensen 2016-05-31 23:51:24 PDT
(In reply to comment #5)
> Comment on attachment 278217 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=278217&action=review
> 
> > Source/WebCore/loader/MediaResourceLoader.cpp:67
> > +    auto cachingPolicy = options & LoadOption::DisallowCaching ? CachingPolicy::DisallowCaching : CachingPolicy::AllowCaching;
> 
> I'm confused. This is not about disk cache, but about memory cache, right?
> Why is this added as an option? Why not passing
> CachingPolicy::DisallowCaching unconditionally for media loads? Looking at
> the code below we disallow caching in the disk cache always for media
> requests.

This just translates one place where we store the caching policy to another place where we store the same policy.  These are indeed about the memory cache, and we did it this way to not touch GTK/GStreamer's caching policy.  The disk cache is disabled by the uses of WebCore::ResourceRequest::Requester::Media in NetworkCache.cpp.
We do not want to use WebKit's caches right now because we are using AVFoundation's caches.