Bug 55267 - Add API to enumerate/delete files downloaded for <audio> and <video>
Summary: Add API to enumerate/delete files downloaded for <audio> and <video>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-25 14:54 PST by Eric Carlson
Modified: 2011-03-07 10:16 PST (History)
2 users (show)

See Also:


Attachments
Proposed patch (4.80 KB, patch)
2011-02-25 15:45 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch (15.76 KB, patch)
2011-03-06 20:21 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch, with missing comma. (15.76 KB, patch)
2011-03-06 20:45 PST, Eric Carlson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2011-02-25 14:54:05 PST
Because some media engines download files for <audio> and <video> directly, we need API so a browser can:

1. Get a list of the origins of all files in the media cache.
2. Delete all files in the media cache.
3. Delete all files in the media cache for a given origin.
Comment 1 Eric Carlson 2011-02-25 14:54:20 PST
<rdar://problem/9049280>
Comment 2 Eric Carlson 2011-02-25 15:45:16 PST
Created attachment 83892 [details]
Proposed patch
Comment 3 Darin Adler 2011-02-25 15:46:33 PST
Comment on attachment 83892 [details]
Proposed patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:2549
> +void HTMLMediaElement::clearMediaCacheForSite(const String site)

Should be const String& rather than just const String.

> Source/WebCore/html/HTMLMediaElement.h:182
> +    void clearMediaCacheForSite(const String);

Here too.

> Source/WebCore/platform/graphics/MediaPlayer.cpp:731
> +void MediaPlayer::clearMediaCacheForSite(const String site)

Here too.

> Source/WebCore/platform/graphics/MediaPlayer.h:293
> +    void clearMediaCacheForSite(const String);

Should be const String& rather than just const String.

> Source/WebCore/platform/graphics/MediaPlayerPrivate.h:146
> +    void clearMediaCacheForSite(const String) { }

Should be const String& rather than just const String.
Comment 4 Eric Carlson 2011-02-25 15:58:19 PST
Oops, landed this when saw the r+ email - which arrived before the one suggesting changes. Will follow up with another check in shortly.
Comment 6 Eric Carlson 2011-03-06 20:08:04 PST
This patch wasn't quite right as it requires an instance of HTMLMediaElement and only works with one media engine. Instead, the methods on HTMLMediaElement should be static, and it should work with all installed media engines.
Comment 7 Eric Carlson 2011-03-06 20:21:36 PST
Created attachment 84916 [details]
Updated patch
Comment 8 WebKit Review Bot 2011-03-06 20:27:25 PST
Attachment 84916 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8101418
Comment 9 Eric Carlson 2011-03-06 20:43:08 PST
(In reply to comment #8)
> Attachment 84916 [details] did not build on chromium:
> Build output: http://queues.webkit.org/results/8101418

Oops, forgot a comma :-(
Comment 10 Eric Carlson 2011-03-06 20:45:49 PST
Created attachment 84918 [details]
Updated patch, with missing comma.
Comment 11 Darin Adler 2011-03-07 09:52:13 PST
Comment on attachment 84918 [details]
Updated patch, with missing comma.

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

> Source/WebCore/platform/graphics/MediaPlayer.cpp:743
> +    if (engines.isEmpty())
> +        return;
> +
> +    unsigned count = engines.size();
> +    for (unsigned ndx = 0; ndx < count; ndx++) {
> +        if (engines[ndx]->getSitesInMediaCache)
> +            engines[ndx]->getSitesInMediaCache(sites);
> +    }

No need for a special case for an empty vector.

Should use size_t instead of unsigned.

I would name the local variable “size” to echo the name of the function.

We normally use “i” rather than “ndx” for this sort of thing.

Same comments for the other two functions below.
Comment 12 Eric Carlson 2011-03-07 10:16:55 PST
http://trac.webkit.org/changeset/80473