Last Comment Bug 778105 - nsOggReader::GetBuffered treats PageSyncResult as nsresult
: nsOggReader::GetBuffered treats PageSyncResult as nsresult
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla17
Assigned To: Nobody; OK to take it and work on it
: Maire Reavy [:mreavy] Please needinfo me
Depends on:
Blocks: 584615 nsresult-enum
  Show dependency treegraph
Reported: 2012-07-27 05:29 PDT by Aryeh Gregor (:ayg) (next working March 28-April 26)
Modified: 2012-08-01 02:52 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (1009 bytes, patch)
2012-07-31 16:08 PDT, Chris Pearce (:cpearce)
cajbir.bugzilla: review+
Details | Diff | Splinter Review

Description User image Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-07-27 05:29:11 PDT
There's a "return PAGE_SYNC_ERROR;" in there, added by bug 584615.  This is wrong -- PAGE_SYNC_ERROR is equal to 1, which isn't a valid nsresult, and will be interpreted as successful.
Comment 1 User image Chris Pearce (:cpearce) 2012-07-29 13:34:19 PDT
Thanks for filing this bug. Thankfully is return value doesn't propagate all the way up to JS.

We hit this case when we detect a page from a stream we've not encountered before (like a new link in a chain or a live stream).

I think we can just return NS_OK in this case.
Comment 2 User image Chris Pearce (:cpearce) 2012-07-31 16:08:00 PDT
Created attachment 647736 [details] [diff] [review]
Comment 3 User image cajbir (:cajbir) 2012-07-31 16:27:17 PDT
Comment on attachment 647736 [details] [diff] [review]

Review of attachment 647736 [details] [diff] [review]:

::: content/media/ogg/nsOggReader.cpp
@@ +1621,5 @@
>          continue;
>        }
>        else {
>          // Page is for a stream we don't know about (possibly a chained
>          // ogg), return an error.

Might want to change the comment since we're no longer returning an error.
Comment 4 User image Chris Pearce (:cpearce) 2012-07-31 18:25:34 PDT
With comment changed:
Comment 5 User image Ed Morley [:emorley] 2012-08-01 02:52:05 PDT

Note You need to log in before you can comment on or make changes to this bug.