Seeking to the end of the stream causes problems




9 years ago
9 years ago


(Reporter: roc, Assigned: roc)


Mac OS X
Bug Flags:
blocking1.9.2 +
in-testsuite +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)



(4 attachments)

1) Sometimes we do an HTTP seek to the end of the content. This is unnecessary, we know there is no content to read there.

2) This can trigger an HTTP response 416 "Requested Range Not Satisfiable", which we treat as data and put into the media cache at the end of the stream.

3) That can confuse nsWaveDecoder, making it think there's data to play after the current mPlaybackPosition, which causes test failures because we might be in the ended state but readyState is HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA.
Created attachment 401780 [details] [diff] [review]
Part 1: Fix Resume to not seek to EOF

If someone calls Resume and we know the current offset is at the end of the stream, don't recreate the channel and seek to the offset. It's a waste of an HTTP transaction because unless the server lied to us, there's no data to read there. We will also probably get a 416 response which will trigger some other problems.
Attachment #401780 - Flags: review?(kinetik)
Created attachment 401781 [details] [diff] [review]
Part 2: don't seek to end of stream in nsMediaCache::Update

Another case where we might seek to EOF is in nsMediaCache::Update. We'd rather suspend than carry out an actual seek operation to the end. But if we're already reading from the end, we may as well carry on without suspending.
Attachment #401781 - Flags: review?(chris.double)
Attachment #401780 - Flags: review?(kinetik) → review+
Created attachment 401782 [details] [diff] [review]
Part 3: handle HTTP errors

We should never allow an error page to be treated as media data. Non-416 errors should be treated as network errors. 416s can still happen if the server supports seeking, doesn't send us a Content-Type, and we get really unlucky trying to seek to the end of the data we've got and finding that we had just reached the end of the available data.
Attachment #401782 - Flags: review?(kinetik)
Attachment #401782 - Flags: review?(kinetik) → review+


9 years ago
Attachment #401781 - Flags: review?(chris.double) → review+
Created attachment 401784 [details] [diff] [review]
Part 4: Fix Wave decoder

The Wave decoder should not say we have more data to play if there is data in the file beyond the current position but the current position is at the end of the sample data.
Attachment #401784 - Flags: review?(kinetik)
Attachment #401784 - Flags: review?(kinetik) → review+
Part 1 checked in to fix test issues:
Whiteboard: [needs landing][needs 192 landing]
These issues are pretty bad since they cause test failures and lead to us placing completely bogus data in the media cache.
Flags: blocking1.9.2+
Priority: -- → P2
Part 1 checked in on 1.9.2:
Whiteboard: [needs landing][needs 192 landing] → [needs landing]
Part 3 caused some errors in tests on the try-servers. The problem turns out to be when a request gets canceled before we reach OnStartRequest. Then in OnStartRequest nsIHttpChannel::GetRequestSucceeded returns false which triggers a call to NetworkError. So I adjusted part 3 to ignore cases where we have a lower-level error reported in GetStatus. Those errors are handled in OnStopRequest, which calls NotifyDownloadEnded, which doesn't call NetworkError for canceled requests.
Checked in parts 2, 3 and 4:
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 192 landing]
Whiteboard: [needs 192 landing]
You need to log in before you can comment on or make changes to this bug.