Disable casting code for constrained builds

RESOLVED FIXED in Firefox 36

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: rnewman, Assigned: nalexander)

Tracking

(Blocks 1 bug)

Trunk
Firefox 36
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

No description provided.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
"Casting" might be too vague. Since I assume you are chasing APK size, I bet you are specific to Chromecast. Roku and other non-Chromecast casting should add much to the APK.
Is the difference the "NATIVE" part, where native means Chromecast?
Yeah, the thing to disable here is MOZ_NATIVE_DEVICES, which should knock-on and disable Google Play services.  That's where the APK bloat is.

The presenting issue is that rnewman wants to make MOZ_NATIVE_DEVICES (set in mobile/android/confvars.sh) depend on MOZ_ANDROID_RESOURCE_CONSTRAINED.  This appears to be an ordering issue: configure.in evaluates confvars.sh before the code in build/autoconf/android.m4 is run.  I will try to find a solution for this ordering -- surely this scenario arises somewhere in the tree.

I also see that android.m4 needs to AC_DEFINE_UNQUOTED both MOZ_ANDROID_RESOURCE_CONSTRAINED and MOZ_ANDROID_EXCLUDE_FONTS.
(In reply to Nick Alexander :nalexander from comment #3)
> Yeah, the thing to disable here is MOZ_NATIVE_DEVICES, which should knock-on
> and disable Google Play services.  That's where the APK bloat is.
> 
> The presenting issue is that rnewman wants to make MOZ_NATIVE_DEVICES (set
> in mobile/android/confvars.sh) depend on MOZ_ANDROID_RESOURCE_CONSTRAINED. 
> This appears to be an ordering issue: configure.in evaluates confvars.sh
> before the code in build/autoconf/android.m4 is run.  I will try to find a
> solution for this ordering -- surely this scenario arises somewhere in the
> tree.

I cannot find an example like this, and I don't know enough configure to understand if there's an easy fix, so I'm escalating to glandium.  glandium, I can think of the following ways to make MOZ_NATIVE_DEVICES depend on MOZ_ANDROID_RESOURCE_CONSTRAINED:

* move the dependency logic into android.m4;
* make configure.in include the local m4 files (like android.m4) before confvars.sh;
* move the relevant android.m4 logic into configure.in.

Can you suggest a path (or an existing example)?
Flags: needinfo?(mh+mozilla)
I think the right thing to do is to move those things out of android.m4. android.m4 is really for low-level stuff, not for generic "i want this feature in fennec" stuff.
Flags: needinfo?(mh+mozilla)
This sounds like a bug for Nick.
Assignee: rnewman → nalexander
Blocks: fatfennec
A small data-point:

Without --enable-android-resource-constrained (i.e., with MOZ_NATIVE_DEVICES):
  -rw-r--r--   1 nalexander  staff  6728060 Nov  3 15:32 classes-proguard.dex
  -rw-r--r--   1 nalexander  staff  7396560 Nov  3 15:32 classes.dex
and:
  -rw-r--r--     1 nalexander  staff  36145120 Nov  3 15:34 fennec-36.0a1.en-US.android-arm.apk

With --enable-android-resource-constrained (i.e., without MOZ_NATIVE_DEVICES):
  -rw-r--r--   1 nalexander  staff  3513136 Nov  3 15:10 classes-proguard.dex
  -rw-r--r--   1 nalexander  staff  4185628 Nov  3 15:10 classes.dex
and:
  -rw-r--r--     1 nalexander  staff  31658352 Nov  3 15:37 fennec-36.0a1.en-US.android-arm.apk

That's a 3meg reduction in the dex and a 4.5meg reduction in the APK.  I see the resource constrained APK does not include some resources, so it's not an apples-to-apples comparison.
This saves dexing and shipping the Google Play Services and other Google
libraries, which add resources and about 3megs of code.

Due to ordering issues, the relevant flags and toggles were moved to
configure.in and exposed early enough to be used by confvars.sh.
Attachment #8516322 - Flags: review?(mh+mozilla)
rnewman: test this?  I swizzled ac_add_options --enable-android-resource-constrained in my mozconfig to test locally but I didn't actually clobber build to verify.
Flags: needinfo?(rnewman)
Built locally; MOZ_MEDIA_PLAYER is false.

Testing on device now.
(In reply to Richard Newman [:rnewman] from comment #10)
> Built locally; MOZ_MEDIA_PLAYER is false.
> 
> Testing on device now.

For the record, I get immediate crashes that look like resource errors.  Not 100% that it's due to this patch, but it seems very likely.
(In reply to Nick Alexander :nalexander from comment #11)
> (In reply to Richard Newman [:rnewman] from comment #10)
> > Built locally; MOZ_MEDIA_PLAYER is false.
> > 
> > Testing on device now.
> 
> For the record, I get immediate crashes that look like resource errors.  Not
> 100% that it's due to this patch, but it seems very likely.

Yeah, just flipped the resource constrained pref off and rebuilt, and now I don't get the resource error.  I expect this is a legitimate issue in the tree that will need to be fixed for this to work on TBPL, but I'll leave that for a future ticket.
If you were testing on tablet, bug 1093394?
Comment on attachment 8516322 [details] [diff] [review]
Disable MOZ_NATIVE_DEVICES when MOZ_ANDROID_RESOURCE_CONSTRAINED is set. r=glandium

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

::: mobile/android/confvars.sh
@@ +66,5 @@
>  # Enable second screen and casting support for external devices.
>  MOZ_DEVICES=1
>  
>  # Enable second screen using native Android libraries
> +if test ! "$MOZ_ANDROID_RESOURCE_CONSTRAINED"; then

test -z
Attachment #8516322 - Flags: review?(mh+mozilla) → review+
(In reply to Richard Newman [:rnewman] from comment #13)
> If you were testing on tablet, bug 1093394?

Nah, phone.  This manifests as a crash at [1].  I see this for "generic resource corruption".

[1] http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/gfx/LayerRenderer.java?from=LayerRenderer.java&case=true#141
https://hg.mozilla.org/mozilla-central/rev/72f0d9f6d5f1
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Filed Bug 1093708 for the crash, which is unrelated to this bug.

Thanks for getting this done, Nick!
Flags: needinfo?(rnewman)
You need to log in before you can comment on or make changes to this bug.