Closed Bug 1311245 Opened 3 years ago Closed 3 years ago

Modify and clean up some codes for media control

Categories

(Firefox for Android :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

Details

Attachments

(4 files)

This bug is fork from bug1282897 comment18, in order to fix the WebRTC test fail (test_getUserMedia_playAudioTwice.html).
Because of the fail of "test_getUserMedia_playAudioTwice.html", I found that some codes in media control can be modified in order to make it more robust.
Blocks: 1290836
Summary: Fix WebRTC test fail "test_getUserMedia_playAudioTwice.html" on Autophone → Modify and clean up some codes for media control
Comment on attachment 8803248 [details]
Bug 1311245 - part1 : don't need to dispatch event to gecko again when receving tab event 'MEDIA_PLAYING_CHANGE' and 'CLOSED'.

https://reviewboard.mozilla.org/r/87490/#review87556
Attachment #8803248 - Flags: review?(s.kaspari) → review+
Comment on attachment 8803249 [details]
Bug 1311245 - part2 : make sure the mActionState would be modified everytime the UI interface changed.

https://reviewboard.mozilla.org/r/87492/#review87558

::: mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java:334
(Diff revision 2)
> +            case ACTION_PAUSE:
> +                mActionState = ACTION_RESUME;
> +            case ACTION_RESUME:
> +                mActionState = ACTION_PAUSE;
> +            case ACTION_STOP:
> +                mActionState = ACTION_STOP;

I think you are missing break; statements here?
(In reply to Sebastian Kaspari (:sebastian) from comment #11)
> Comment on attachment 8803249 [details]
> Bug 1311245 - part2 : make sure the mActionState would be modified everytime
> the UI interface changed.
> 
> https://reviewboard.mozilla.org/r/87492/#review87558
> 
> :::
> mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java:334
> (Diff revision 2)
> > +            case ACTION_PAUSE:
> > +                mActionState = ACTION_RESUME;
> > +            case ACTION_RESUME:
> > +                mActionState = ACTION_PAUSE;
> > +            case ACTION_STOP:
> > +                mActionState = ACTION_STOP;
> 
> I think you are missing break; statements here?

Ah, sorry, I would add "break;" for them.
Comment on attachment 8803249 [details]
Bug 1311245 - part2 : make sure the mActionState would be modified everytime the UI interface changed.

https://reviewboard.mozilla.org/r/87492/#review88004
Attachment #8803249 - Flags: review?(s.kaspari) → review+
Comment on attachment 8803250 [details]
Bug 1311245 - part3 : mActionState should only store resume/pause/stop.

https://reviewboard.mozilla.org/r/87494/#review88006
Attachment #8803250 - Flags: review?(s.kaspari) → review+
Comment on attachment 8803251 [details]
Bug 1311245 - part4 : remove redundant state.

https://reviewboard.mozilla.org/r/87496/#review88008
Attachment #8803251 - Flags: review?(s.kaspari) → review+
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f36768e0407
part1 : don't need to dispatch event to gecko again when receving tab event 'MEDIA_PLAYING_CHANGE' and 'CLOSED'. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/b2066343565f
part2 : make sure the mActionState would be modified everytime the UI interface changed. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/e86ad7e50645
part3 : mActionState should only store resume/pause/stop. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/f06568be1513
part4 : remove redundant state. r=sebastian
You need to log in before you can comment on or make changes to this bug.