Remove static MediaButtonReceiver in AndroidManifest for v3.2

[martinmidtsund]

The first commit removes the static MediaButtonReceiver which is causing a lot of RemoteServiceExceptions, and a group of other related exceptions, most of them mentioned in this issue. This change made all of the related errors go away in our production environment, and has been running there for almost a month now. @elliotdickison has also reported the same.

See https://developer.android.com/guide/topics/media-apps/mediabuttons#summary where they say you need the static MediaButtonReceiver for pre-lollipop, but later versions don't need it.

As seen here a service can also receive media button actions: A service can receive a key event by including an intent filter that handles ACTION_MEDIA_BUTTON which is already registered in the manifest, and it seems to work just fine. We have verified with a connected bluetooth headset that skip, play and pause is still working when originating from the headset. It would be great if you could test the PR and verify it with other devices as well.

The last commit exporting the MusicService and setting foregroundServiceType is not tested in production yet, but should be set as I gather from this page

Sorry for the wall of text. Please advise if anything should be changed.

[mmmoussa]

We have been using this change, but with android:exported left unchanged (ie. false), in production with a large number of users. I can confirm that it has significantly reduced our crash rate by resolving #1676 and #1874. However, we are still encountering #1666 and #1812.

[martinmidtsund]

@mmmoussa Great to hear! My guess, without having been able to test it yet, is that the last commit hopefully alleviates some of the notAllowed errors.

[martinmidtsund]

As you can see here if we removed the exported-attribute the default would be true as long as we an intent filter present, without the intent filter the default would be false.

[mmmoussa]

We have been running with all of the changes in this PR (including exported as true) and I can confirm that #1666 and #1812 are not fixed, though this is still worth merging as it fixes other issues as mentioned previously.

[bradfloodx]

Yep same here. Deployed this change to production, still getting crash reports on new versions for #1666 and #1812 :(

[martinmidtsund]

@mmmoussa @bradleyflood Thanks for testing!

Perhaps @jspizziri or @dcvz could take a look? I really think this should go into the main-branch as well as this 3.2 branch, but it's a good place to start. :)

[dcvz]

@mmmoussa @bradleyflood Thanks for testing!

Perhaps @jspizziri or @dcvz could take a look? I really think this should go into the main-branch as well as this 3.2 branch, but it's a good place to start. :)

This one is on our radar! We’ll likely cherry pick after merge and make one more release of 3.2 — we’re just waiting until one of us gets a chance to test it with a few devices.

[dcvz]

Alright merging this in @martinmidtsund and cherry picking to 4.0.0 nightlies. We'll keep an eye out for any unintended consequences.