Closed Bug 1119803 Opened 5 years ago Closed 5 years ago
Uninitialised value use in Stop
test case = dom/media/test/test_playback.html Looks like MediaDecoderStateMachine::mIsVideoPrerolling is used without being initialised. I had a bit of a look around but the class is vast and I can't guess how the logic works. I can say that mIsVideoPrerolling -- and, for that matter, mIsAudioPrerolling -- are not initialised in MediaDecoderStateMachine::MediaDecoderStateMachine. Maybe they should be?
This stops V complaining. Does it seem the right approach? Note that I did not also initialise mIsAudioPrerolling since V didn't complain about that. But not doing so rather bugs me since it seems as if the logic for those two fields is the same and so either both of them or neither of them should be initialised in MediaDecoderStateMachine::MediaDecoderStateMachine.
Comment on attachment 8547456 [details] [diff] [review] The obvious fix Review of attachment 8547456 [details] [diff] [review]: ----------------------------------------------------------------- Yeah please initialize both this and mIsVideoPrerolling to false. r=me with that.
This is basically a harmless bug I think - the worst impact would be one non-deterministic invocation of ScheduleStateMachine. But we should probably get it on the uplift radar anyway.
(In reply to Bobby Holley (Busy with media, don't ask for DOM/JS/XPConnect things) from comment #3) > Yeah please initialize both this and mIsVideoPrerolling to false. r=me with IIUC "this" already refers to mIsVideoPrerolling. Do you mean mIsAudioPrerolling?
Yes, we should definitely uplift this. I can nominate after it lands on m-c.
Comment on attachment 8547456 [details] [diff] [review] The obvious fix Approval Request Comment [Feature/regressing bug #]: MSE [User impact if declined]: Less consistent testing, hidden bugs from uninitialized variable use. [Describe test coverage new/current, TBPL]: Landed on m-i. [Risks and why]: Risk is minimal. Small and obvious change. [String/UUID change made/needed]: None
Assignee: nobody → jseward
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.