Add better player tab crash handling
ClosedPublic

Authored by broulik on Apr 6 2020, 8:21 AM.

Details

Summary

There is unfortunately no dedicated signal for this (the process API is for dev builds only), so we can only do:

  • When we detect a tab becoming inaudible, check if it's gone now, and signal player gone. This will catch the case of a tab crashing while playing
  • When sending a command, check if it failed, and signal player gone. Instead of doing nothing, the player will disappear. Not really better UX right there but at least the user will go "huh?" rather than "why is it not working?"

There's also no error codes on the error object, just sentences...

BUG: 419699

Test Plan
  • Started playing a tab in chrome, killed the process, had the player gone after a few seconds. This heuristic does not work in Firefox where instead I get a navigation to about:blank and back to the original page...
  • Started playing a tab in chrome, paused it, killed the process. Nothing changed. I then clicked Play on media controller, and the player was gone. This also works in Firefox where the error message is identical.

Diff Detail

Repository
R856 Plasma Browser Integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Apr 6 2020, 8:21 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 6 2020, 8:21 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Apr 6 2020, 8:21 AM
fvogt added a comment.Apr 6 2020, 8:44 AM

If there is a false positive in the detection, how would those be handled? AFAICT the players would never appear in mpris again?

If there is a false positive in the detection, how would those be handled? AFAICT the players would never appear in mpris again?

I believe whenever a player starts playing again, it is propagated through MPRIS again. The playerGone handling is no different from the player being removed from DOM and being added back.

fvogt added a comment.Apr 6 2020, 9:47 AM

If there is a false positive in the detection, how would those be handled? AFAICT the players would never appear in mpris again?

I believe whenever a player starts playing again, it is propagated through MPRIS again. The playerGone handling is no different from the player being removed from DOM and being added back.

Is there an "adding back" though? If I'm not completely wrong, for such a "ghost" tab, currentPlayer() would return {} and nothing happens anymore.

fvogt accepted this revision.Apr 6 2020, 10:08 AM

For false positives the player would get added again by the playing event, which is not ideal, but as it doesn't require a reload it's IMO close enough.

This revision is now accepted and ready to land.Apr 6 2020, 10:08 AM

Waiting for @cblack since they were who mentioned this issue

broulik edited the summary of this revision. (Show Details)Apr 6 2020, 10:09 AM
ngraham accepted this revision.Apr 6 2020, 2:51 PM
ngraham added a subscriber: ngraham.

I have the same issue with Firefox and can confirm that this fixes it!

cblack accepted this revision.Apr 7 2020, 5:27 PM
This revision was automatically updated to reflect the committed changes.