Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: wesj, Assigned: wesj)

Tracking

unspecified
Firefox 34
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox32 wontfix, firefox33 fixed, firefox34 fixed)

Details

Attachments

(1 attachment)

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.
Posted patch PatchSplinter Review
Attachment #8464068 - Flags: review?(blassey.bugs)
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?
Attachment #8464068 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/f9b7cc4629ab
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
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.
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]
You need to log in before you can comment on or make changes to this bug.