1045693 - Cleanup Chromecast code

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

There are a couple issues with enabling Chromecast that were fixed in the patch for bug 1033560, and should land even if it doesn't.

1.) We should make init a JNITarget so that its not removed by proguard
2.) MediaPlayerManager is initialized twice in GeckoApp. Stop doing that.
3.) I added a check to make sure Google Play Services was installed or throw. We should keep that to safely run on non Google devices.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8464068 [details] [diff] [review]
Patch

Review of attachment 8464068 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/BrowserApp.java
@@ -566,5 @@
> -            } catch(Exception ex) {
> -                // Ignore failures
> -                Log.i(LOGTAG, "No native casting support", ex);
> -            }
> -        }

so happy to see this reflection go

::: mobile/android/base/MediaPlayerManager.java
@@ +118,5 @@
>                  try {
>                      disps.put(disp.toJSON());
>                  } catch(Exception ex) {
>                      // This may happen if the device isn't a real Chromecast,
> +                    // for example Firefly casting devices.@

typo?

::: mobile/android/confvars.sh
@@ +73,5 @@
>  # Enable second screen and casting support for external devices.
>  MOZ_DEVICES=1
>  
>  # Enable second screen using native Android libraries
> +MOZ_NATIVE_DEVICES=1

are we ready to enable on the builders?

Comment 3

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/f9b7cc4629ab

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f9b7cc4629ab

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 8464068 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: New feature
[User impact if declined]: No casting videos to chromecast devices
[Describe test coverage new/current, TBPL]: There isn't much :(
[Risks and why]: This is pretty low risk. Fixing some build errors in the Chromecast code that aren't visible until its enabled.
[String/UUID change made/needed]: None.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Wesley, are you planning to write tests for this feature? thanks

Comment 7

[Wesley Johnston (:wesj)]

We have some basic tests for casting. Testing the Chromecast code itself might be hard. We can write some basic Unit tests now though. Maybe we can mock something out and do this. I'll look into it.

Comment 8

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/74e3bdff5561