Closed Bug 1045693 Opened 9 years ago Closed 9 years ago

Cleanup Chromecast code


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox32 wontfix, firefox33 fixed, firefox34 fixed)

Firefox 34
Tracking Status
firefox32 --- wontfix
firefox33 --- fixed
firefox34 --- fixed


(Reporter: wesj, Assigned: wesj)



(1 file)

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.
Attached patch PatchSplinter Review
Attachment #8464068 - Flags: review?(blassey.bugs)
Comment on attachment 8464068 [details] [diff] [review]

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

::: mobile/android/base/
@@ -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/
@@ +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.@


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

are we ready to enable on the builders?
Attachment #8464068 - Flags: review?(blassey.bugs) → review+
Assignee: nobody → wjohnston
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Comment on attachment 8464068 [details] [diff] [review]

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.
Attachment #8464068 - Flags: approval-mozilla-aurora?
Whiteboard: [dont-auto-uplift]
Wesley, are you planning to write tests for this feature? thanks
Flags: needinfo?(wjohnston)
Attachment #8464068 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Flags: needinfo?(wjohnston)
Whiteboard: [dont-auto-uplift]
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.