Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 312756 Refactor TextTrackLoader to use RawResource and remove TextTrackResource
Starred by 1 user Reported by phil...@opera.com, Oct 29 2013 Back to list
Status: Fixed
Owner:
Email to this user bounced
Closed: Oct 2013
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment
TextTrackResource doesn't do anything useful, so just use RawResource instead.
 
Project Member Comment 1 by bugdroid1@chromium.org, Oct 29 2013
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=160848

------------------------------------------------------------------------
r160848 | philipj@opera.com | 2013-10-29T18:34:48.521051Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/TextTrackResource.cpp?r1=160848&r2=160847&pathrev=160848
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/TextTrackLoader.h?r1=160848&r2=160847&pathrev=160848
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/TextTrackResource.h?r1=160848&r2=160847&pathrev=160848
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/ResourceClient.h?r1=160848&r2=160847&pathrev=160848
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/TextTrackLoader.cpp?r1=160848&r2=160847&pathrev=160848

Let TextTrackResource be a RawResource

This allows deprecatedDidReceiveResource to be removed completely, by
using the exising appendData callback instead.

The removed FIXME was a little bit confused, in that it didn't seem to
realize that incremental parsing is needed for WebVTT.

Also add OVERRIDE where possible.

BUG= 312756 

Review URL: https://codereview.chromium.org/50123006
------------------------------------------------------------------------
Comment 2 by phil...@opera.com, Oct 30 2013
Summary: Refactor TextTrackLoader to use RawResource and remove TextTrackResource (was: Remove TextTrackResource (just use RawResource))
Project Member Comment 3 by bugdroid1@chromium.org, Oct 30 2013
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=160953

------------------------------------------------------------------------
r160953 | philipj@opera.com | 2013-10-30T17:41:17.703825Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/TextTrackLoader.h?r1=160953&r2=160952&pathrev=160953
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/TextTrackLoader.cpp?r1=160953&r2=160952&pathrev=160953

Rename TextTrackLoader::m_cachedCueData to m_resource

It's only used to add and remove a client and the code doesn't know
or care whether it's cached or not, just call it m_resource like
in DocumentThreadableLoader.

BUG= 312756 

Review URL: https://codereview.chromium.org/52463002
------------------------------------------------------------------------
Comment 4 by phil...@opera.com, Oct 30 2013
bugdroid seems to be sick, these reviews are also landed now:

https://codereview.chromium.org/52533003 (Refactor TextTrackLoader to properly use the dataReceived callback)
https://codereview.chromium.org/52563002 (Remove TextTrackResource)
Comment 5 by phil...@opera.com, Oct 30 2013
Status: Fixed
Project Member Comment 6 by bugdroid1@chromium.org, Oct 31 2013
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=160965

------------------------------------------------------------------------
r160965 | philipj@opera.com | 2013-10-30T20:11:44.940546Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/media/track/opera/track/webvtt/parsing/001-expected.txt?r1=160965&r2=160964&pathrev=160965
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/TextTrackLoader.cpp?r1=160965&r2=160964&pathrev=160965
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/TextTrackLoader.h?r1=160965&r2=160964&pathrev=160965

Refactor TextTrackLoader to properly use the dataReceived callback

We get the data and length in the callback, so just use it directly.

This accidentally fixes a parsing test, by changing the segmentation
of the input received by the WebVTT parser. A separate bug was filed
to fix the problem: http://crbug.com/313222

BUG= 312756 

Review URL: https://codereview.chromium.org/52533003
------------------------------------------------------------------------
Comment 8 by phil...@opera.com, Oct 31 2013
Labels: Cr-Blink-Text-Track
Comment 9 by sshru...@google.com, May 23 2016
Components: -Blink>Text>Track Blink>Media>Track
Renamed Blink>Text>Track to Blink>Media>Track. Moving these bugs to the new component.
Sign in to add a comment