Bug 70134 - Cues should be loaded by the cached resource loader
Summary: Cues should be loaded by the cached resource loader
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: InRadar
Depends on:
Blocks: 43668
  Show dependency treegraph
 
Reported: 2011-10-14 12:44 PDT by Eric Carlson
Modified: 2019-05-02 16:23 PDT (History)
9 users (show)

See Also:


Attachments
Part 1: Update CachedResourceLoader to load cues (17.79 KB, patch)
2011-10-14 12:46 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Part 2: Implement the cue loader (11.40 KB, patch)
2011-10-14 12:47 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch with ChangeLog (19.72 KB, patch)
2011-10-15 23:06 PDT, Eric Carlson
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Add new file to other build systems. (21.43 KB, patch)
2011-10-16 09:43 PDT, 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-10-14 12:44:50 PDT
Cues should be cached like other web resources so they should be loaded by a CachedResourceLoader
Comment 1 Radar WebKit Bug Importer 2011-10-14 12:45:03 PDT
<rdar://problem/10288601>
Comment 2 Eric Carlson 2011-10-14 12:46:42 PDT
Created attachment 111056 [details]
Part 1: Update CachedResourceLoader to load cues
Comment 3 Eric Carlson 2011-10-14 12:47:25 PDT
Created attachment 111057 [details]
Part 2: Implement the cue loader
Comment 4 WebKit Review Bot 2011-10-14 13:38:59 PDT
Comment on attachment 111057 [details]
Part 2: Implement the cue loader

Attachment 111057 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10060651
Comment 5 Eric Carlson 2011-10-14 13:45:18 PDT
(In reply to comment #4)
> (From update of attachment 111057 [details])
> Attachment 111057 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/10060651

Part 2 depends on Part 1.
Comment 6 Collabora GTK+ EWS bot 2011-10-14 23:22:51 PDT
Comment on attachment 111057 [details]
Part 2: Implement the cue loader

Attachment 111057 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10074211
Comment 7 Eric Carlson 2011-10-15 10:47:50 PDT
(In reply to comment #6)
> (From update of attachment 111057 [details])
> Attachment 111057 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/10074211

See comment #5.
Comment 8 Eric Carlson 2011-10-15 13:48:59 PDT
Comment on attachment 111057 [details]
Part 2: Implement the cue loader

Clearing flags, this won't compile without the other patch so I will create a new bug for this once the other changes land.
Comment 9 Eric Carlson 2011-10-15 23:06:47 PDT
Created attachment 111176 [details]
Patch with ChangeLog
Comment 10 WebKit Review Bot 2011-10-15 23:42:44 PDT
Comment on attachment 111176 [details]
Patch with ChangeLog

Attachment 111176 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10074465
Comment 11 Collabora GTK+ EWS bot 2011-10-16 00:12:15 PDT
Comment on attachment 111176 [details]
Patch with ChangeLog

Attachment 111176 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10073548
Comment 12 Eric Carlson 2011-10-16 09:43:26 PDT
Created attachment 111181 [details]
Add new file to other build systems.
Comment 13 Darin Adler 2011-10-17 12:01:37 PDT
Comment on attachment 111181 [details]
Add new file to other build systems.

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

> Source/WebCore/loader/cache/CachedCues.cpp:53
> +    setEncodedSize(m_data.get() ? m_data->size() : 0);

Should not need a "get" here.

> Source/WebCore/loader/cache/CachedCues.cpp:56
> +    while (CachedResourceClient *client = walker.next())

The "*" needs to be next to the type here.

> Source/WebCore/loader/cache/CachedCues.h:43
> +    virtual void data(PassRefPtr<SharedBuffer> data, bool allDataReceived);

Can this be private?
Comment 14 Eric Carlson 2011-10-17 12:23:07 PDT
http://trac.webkit.org/changeset/97637
Comment 15 Eric Carlson 2011-10-17 12:23:42 PDT
(In reply to comment #13)
> (From update of attachment 111181 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111181&action=review
> 
> > Source/WebCore/loader/cache/CachedCues.cpp:53
> > +    setEncodedSize(m_data.get() ? m_data->size() : 0);
> 
> Should not need a "get" here.
> 
> > Source/WebCore/loader/cache/CachedCues.cpp:56
> > +    while (CachedResourceClient *client = walker.next())
> 
> The "*" needs to be next to the type here.
> 
> > Source/WebCore/loader/cache/CachedCues.h:43
> > +    virtual void data(PassRefPtr<SharedBuffer> data, bool allDataReceived);
> 
> Can this be private?

I missed these comments, but will do the suggested cleanup in another patch.