Closed Bug 1171288 Opened 4 years ago Closed 4 years ago

Add ability to build with RecyclerView support library

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

35 Branch
defect
Not set

Tracking

(firefox41 fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: Margaret, Assigned: sebastian)

References

(Blocks 3 open bugs)

Details

Attachments

(1 file, 2 obsolete files)

https://developer.android.com/tools/support-library/features.html#v7-recyclerview

As I understand it, there's currently no easy way to do this because this involves resource sharing.

nalexander, do you know of any (hacky) way we could do this without full gradle build support?
Flags: needinfo?(nalexander)
Blocks: 1171289
(In reply to :Margaret Leibovic from comment #0)
> https://developer.android.com/tools/support-library/features.html#v7-
> recyclerview
> 
> As I understand it, there's currently no easy way to do this because this
> involves resource sharing.
> 
> nalexander, do you know of any (hacky) way we could do this without full
> gradle build support?

Hi folks, sorry for the delayed reply.

I see extras/android/support/v7/recyclerview/{libs,res}, so this is shipped like other Android libraries.  (i.e., Google hasn't gone fully Gradle, and only shipped this in m2repo.)

Looking at https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=appcompat&redirect=true basically shows what needs to happen:

1) configure.in foo to find the paths.  We could probably simplify this so we don't need configure.in changes to find libraries anymore, but that's more work upfront.

2) add another package to the generated list in mobile/android/base/Makefile.in.  I feel like some of this is conditional on MOZ_NATIVE_DEVICES, but we would make it less conditional.

3) minor changes to base/moz.build to reference the JAR, etc.  Don't worry about the Eclipse stuff -- it's obsolete and we should remove it rather than update it.

4) include in .gradle files as appropriate.

Adding any one library is not terrible; but moving to a better system for libraries is a decent amount of work that I don't think we should invest in.  Implementing Bug 1119520 for local use, and then automation use, would be a better use of our time.

So: this is all possible: it's basically copy-pasta from appcompat.
Flags: needinfo?(nalexander)
Okay, I want to have it and it seems to be easier than I thought. Thank you for the hints, nalexander.
Assignee: nobody → s.kaspari
Attached patch 1171288-recylerview.patch (obsolete) — Splinter Review
This is my current version of the patch. Even though it seemed to be pretty straight-forward I've been running into two problems:

1) fennec-ids-generator.py breaks with the new input.

> 1:31.93 make[5]: *** [fennec_ids.txt] Error 42
> 1:31.93 make[5]: *** Waiting for unfinished jobs....
> 1:32.93 Picked up JAVA_TOOL_OPTIONS: -javaagent:/usr/share/java/jayatanaag.jar
> 1:33.09 /home/sebastian/Projects/FxTeam/config/recurse.mk:79: recipe for target 'mobile/android/base/libs' failed
> 1:33.09 make[4]: *** [mobile/android/base/libs] Error 2
> 1:33.09 /home/sebastian/Projects/FxTeam/config/recurse.mk:32: recipe for target 'libs' failed
> 1:33.09 make[3]: *** [libs] Error 2
> 1:33.09 /home/sebastian/Projects/FxTeam/config/rules.mk:538: recipe for target 'default' failed
> 1:33.09 make[2]: *** [default] Error 2
> 1:33.09 /home/sebastian/Projects/FxTeam/client.mk:400: recipe for target 'realbuild' failed
> 1:33.09 make[1]: *** [realbuild] Error 2
> 1:33.09 client.mk:171: recipe for target 'build' failed
> 1:33.09 make: *** [build] Error 2
> 1:33.10 443 compiler warnings present.

It seems like fennec-ids-generator.py does not expect comments between id definitions:

> [..]
>        public static int info=0x7f08011c;
>        public static int intro_view=0x7f0800cc;
>        /**  ItemTouchHelper uses this id to save a View's original elevation. 
>         */
>        public static int item_touch_helper_previous_elevation=0x7f080000;
>        public static int keyInput=0x7f080173;
>        public static int label_search_hint=0x7f08013b;
> [..]

I "quick-fixed" this locally by just omitting lines that can't be parsed with the existing code and this results in a successful build. However then (2) happened.


2) The app crashes on my tablet (N9) now. I can't see how this is related to the build changes so far but this file hasn't been touched in ages.

> E/GeckoCrashHandler(16911): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
> E/GeckoCrashHandler(16911): java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.ViewTreeObserver android.view.View.getViewTreeObserver()' on a null object reference
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.TabMenuStripLayout.onPageSelected(TabMenuStripLayout.java:68)
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.TabMenuStrip.onPageSelected(TabMenuStrip.java:83)
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomePager.setCurrentItem(HomePager.java:282)
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomePager.access$600(HomePager.java:35)
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomePager$ConfigLoaderCallbacks.onLoadFinished(HomePager.java:463)
> E/GeckoCrashHandler(16911): 	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.callOnLoadFinished(Unknown Source)
> E/GeckoCrashHandler(16911): 	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.onLoadComplete(Unknown Source)
> E/GeckoCrashHandler(16911): 	at android.support.v4.content.Loader.deliverResult(Unknown Source)
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomeConfigLoader.deliverResult(HomeConfigLoader.java:44)
> E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomeConfigLoader.deliverResult(HomeConfigLoader.java:16)
> E/GeckoCrashHandler(16911): 	at android.support.v4.content.AsyncTaskLoader$LoadTask.onPostExecute(Unknown Source)
> E/GeckoCrashHandler(16911): 	at android.support.v4.content.ModernAsyncTask.access$500(Unknown Source)
> E/GeckoCrashHandler(16911): 	at android.support.v4.content.ModernAsyncTask$InternalHandler.handleMessage(Unknown Source)
> E/GeckoCrashHandler(16911): 	at android.os.Handler.dispatchMessage(Handler.java:102)
> E/GeckoCrashHandler(16911): 	at android.os.Looper.loop(Looper.java:135)
> E/GeckoCrashHandler(16911): 	at android.app.ActivityThread.main(ActivityThread.java:5254)
> E/GeckoCrashHandler(16911): 	at java.lang.reflect.Method.invoke(Native Method)
> E/GeckoCrashHandler(16911): 	at java.lang.reflect.Method.invoke(Method.java:372)
> E/GeckoCrashHandler(16911): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
> E/GeckoCrashHandler(16911): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

The build seems to run on phones though. This will need some more investigation to figure out what's going wrong here. In addition to that I'll try to build a version with some simple RecyclerView somewhere in the UI to see whether the build was actually successful.
Comment on attachment 8622384 [details] [diff] [review]
1171288-recylerview.patch

@nalexander: Looking at this patch: Is the general approach correct? Do you think the tablet crash is actually related to this or is it just some side effect?
Attachment #8622384 - Flags: feedback?(nalexander)
(In reply to :Sebastian Kaspari from comment #3)
> Created attachment 8622384 [details] [diff] [review]
> 1171288-recylerview.patch
> 
> This is my current version of the patch. Even though it seemed to be pretty
> straight-forward I've been running into two problems:
> 
> 1) fennec-ids-generator.py breaks with the new input.
> 
> > 1:31.93 make[5]: *** [fennec_ids.txt] Error 42
> > 1:31.93 make[5]: *** Waiting for unfinished jobs....
> > 1:32.93 Picked up JAVA_TOOL_OPTIONS: -javaagent:/usr/share/java/jayatanaag.jar
> > 1:33.09 /home/sebastian/Projects/FxTeam/config/recurse.mk:79: recipe for target 'mobile/android/base/libs' failed
> > 1:33.09 make[4]: *** [mobile/android/base/libs] Error 2
> > 1:33.09 /home/sebastian/Projects/FxTeam/config/recurse.mk:32: recipe for target 'libs' failed
> > 1:33.09 make[3]: *** [libs] Error 2
> > 1:33.09 /home/sebastian/Projects/FxTeam/config/rules.mk:538: recipe for target 'default' failed
> > 1:33.09 make[2]: *** [default] Error 2
> > 1:33.09 /home/sebastian/Projects/FxTeam/client.mk:400: recipe for target 'realbuild' failed
> > 1:33.09 make[1]: *** [realbuild] Error 2
> > 1:33.09 client.mk:171: recipe for target 'build' failed
> > 1:33.09 make: *** [build] Error 2
> > 1:33.10 443 compiler warnings present.
> 
> It seems like fennec-ids-generator.py does not expect comments between id
> definitions:

ARGH!  We should remove this fennec_ids nonsense once and for all; see https://bugzilla.mozilla.org/show_bug.cgi?id=969921.  I have a local patch; I'll get review for it.  I'd rather remove than patch.

> > [..]
> >        public static int info=0x7f08011c;
> >        public static int intro_view=0x7f0800cc;
> >        /**  ItemTouchHelper uses this id to save a View's original elevation. 
> >         */
> >        public static int item_touch_helper_previous_elevation=0x7f080000;
> >        public static int keyInput=0x7f080173;
> >        public static int label_search_hint=0x7f08013b;
> > [..]
> 
> I "quick-fixed" this locally by just omitting lines that can't be parsed
> with the existing code and this results in a successful build. However then
> (2) happened.
> 
> 
> 2) The app crashes on my tablet (N9) now. I can't see how this is related to
> the build changes so far but this file hasn't been touched in ages.
> 
> > E/GeckoCrashHandler(16911): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
> > E/GeckoCrashHandler(16911): java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.ViewTreeObserver android.view.View.getViewTreeObserver()' on a null object reference
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.TabMenuStripLayout.onPageSelected(TabMenuStripLayout.java:68)
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.TabMenuStrip.onPageSelected(TabMenuStrip.java:83)
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomePager.setCurrentItem(HomePager.java:282)
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomePager.access$600(HomePager.java:35)
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomePager$ConfigLoaderCallbacks.onLoadFinished(HomePager.java:463)
> > E/GeckoCrashHandler(16911): 	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.callOnLoadFinished(Unknown Source)
> > E/GeckoCrashHandler(16911): 	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.onLoadComplete(Unknown Source)
> > E/GeckoCrashHandler(16911): 	at android.support.v4.content.Loader.deliverResult(Unknown Source)
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomeConfigLoader.deliverResult(HomeConfigLoader.java:44)
> > E/GeckoCrashHandler(16911): 	at org.mozilla.gecko.home.HomeConfigLoader.deliverResult(HomeConfigLoader.java:16)
> > E/GeckoCrashHandler(16911): 	at android.support.v4.content.AsyncTaskLoader$LoadTask.onPostExecute(Unknown Source)
> > E/GeckoCrashHandler(16911): 	at android.support.v4.content.ModernAsyncTask.access$500(Unknown Source)
> > E/GeckoCrashHandler(16911): 	at android.support.v4.content.ModernAsyncTask$InternalHandler.handleMessage(Unknown Source)
> > E/GeckoCrashHandler(16911): 	at android.os.Handler.dispatchMessage(Handler.java:102)
> > E/GeckoCrashHandler(16911): 	at android.os.Looper.loop(Looper.java:135)
> > E/GeckoCrashHandler(16911): 	at android.app.ActivityThread.main(ActivityThread.java:5254)
> > E/GeckoCrashHandler(16911): 	at java.lang.reflect.Method.invoke(Native Method)
> > E/GeckoCrashHandler(16911): 	at java.lang.reflect.Method.invoke(Method.java:372)
> > E/GeckoCrashHandler(16911): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
> > E/GeckoCrashHandler(16911): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)
> 
> The build seems to run on phones though. This will need some more
> investigation to figure out what's going wrong here. In addition to that
> I'll try to build a version with some simple RecyclerView somewhere in the
> UI to see whether the build was actually successful.

I see no crash on an older tablet.  We have seen this before https://crash-analysis.mozilla.com/rkaiser/2014-08-01/2014-08-01.fennecandroid.nightly.startup.html so I expect it is not really related.  But we would want some additional test runs, etc.  We could be tickling some existing error.

Anyway, this patch is r+ with nits (and the fennec_ids dependency).
Comment on attachment 8622384 [details] [diff] [review]
1171288-recylerview.patch

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

Please add the new dep to the appropriate build.gradle.  (Look for the support v4 dep.)

Questions:
* have you used the RecyclerView to make sure this is actually working?
* can I get a byte count on classes.dex before and after (with an actual usage)?  I want to understand APK bloat a little.
* does this want a build flag?  Is it supporting a particular feature?  (I think not, but want to understand.)

We will need to either remove fennec_ids stuff or hack around the script.  I'd prefer the former and have a small patch, but we don't /have/ to block this.

::: mobile/android/base/Makefile.in
@@ +65,5 @@
>  
>  JAVA_BOOTCLASSPATH := $(subst $(NULL) ,:,$(strip $(JAVA_BOOTCLASSPATH)))
>  
> +JAVA_CLASSPATH += \
> +    $(ANDROID_RECYCLERVIEW_LIB)

nit: Either follow suit with \ $(NULL) or make this a single line, like:

JAVA_CLASSPATH += $(ANDROID_RECYCLERVIEW_LIB)

@@ +371,5 @@
>  generated/org/mozilla/gecko/R.java: .aapt.deps ;
>  
>  # If native devices are enabled, add Google Play Services, build their resources
>  generated/android/support/v7/appcompat/R.java: .aapt.deps ;
> +generated/android/support/v7/recyclerview/R.java: .aapt.deps ;

nit: alphabetize, so this comes below mediarouter.
Attachment #8622384 - Flags: feedback?(nalexander) → review+
I'm removing fennec_ids.txt stuff in Bug 969925, although it's not a hard dependency for this ticket.
Attached patch 1171288-recylerview_v2.patch (obsolete) — Splinter Review
I attached the updated patch.

(In reply to Nick Alexander :nalexander from comment #6)
> * have you used the RecyclerView to make sure this is actually working?

That's what I did today but unfortunately I've ran into build errors using ./mach build:

> [..]
>  1:26.01 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v4/android-support-v4.jar]
>  1:26.94 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v7/recyclerview/libs/android-support-v7-recyclerview.jar]
>  1:27.25 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/libs/google-play-services.jar]
>  1:31.26 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v7/mediarouter/libs/android-support-v7-mediarouter.jar]
>  1:31.38 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar]
>  1:32.07 Reading library jar [/home/sebastian/.mozbuild/android-sdk-linux/platforms/android-22/android.jar]
>  1:32.19 /home/sebastian/Projects/FxTeam/mobile/android/base/home/PanelRecyclerView.java:4: error: cannot find symbol
>  1:32.19 import android.support.v7.widget.LinearLayoutManager;
>  1:32.20                                 ^
>  1:32.20   symbol:   class LinearLayoutManager
>  1:32.20   location: package android.support.v7.widget
>  1:32.20 /home/sebastian/Projects/FxTeam/mobile/android/base/home/PanelRecyclerView.java:5: error: cannot find symbol
>  1:32.20 import android.support.v7.widget.RecyclerView;
>  1:32.20                                 ^
>  1:32.20   symbol:   class RecyclerView
>  1:32.20   location: package android.support.v7.widget
>  1:32.21 /home/sebastian/Projects/FxTeam/mobile/android/base/home/PanelRecyclerView.java:10: error: cannot find symbol
>  1:32.21 public class PanelRecyclerView extends RecyclerView {
> [..]

From the log output it looks like it's picking up the code ("1:26.01 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v4/android-support-v4.jar]"). However later it does not find all the classes.

@nalexander: Do you have an idea what's going wrong here?

(In reply to Nick Alexander :nalexander from comment #6)
> * can I get a byte count on classes.dex before and after (with an actual
> usage)?  I want to understand APK bloat a little.

Sure! I'll do that as soon as I've a working build.

(In reply to Nick Alexander :nalexander from comment #6)
> * does this want a build flag?  Is it supporting a particular feature?  (I
> think not, but want to understand.)

I want to use RecyclerView for some new home panel APIs (See bug 1157539 and bug 1172136). Martyn is planning to use it for some other UI updates and there is this meta bug 1158303 about replacing some of our ListViews with RecyclerView. In this context there's especially bug 1167942 where we want to use RecyclerView as replacement for ListView to work around a bug. So, yeah, there are plans to use it all over the app - This makes a build flag useless here, right?
Attachment #8622384 - Attachment is obsolete: true
Attachment #8623175 - Flags: feedback?(nalexander)
(In reply to :Sebastian Kaspari from comment #8)
> Created attachment 8623175 [details] [diff] [review]
> 1171288-recylerview_v2.patch
> 
> I attached the updated patch.
> 
> (In reply to Nick Alexander :nalexander from comment #6)
> > * have you used the RecyclerView to make sure this is actually working?
> 
> That's what I did today but unfortunately I've ran into build errors using
> ./mach build:

So close:

diff --git a/mobile/android/base/moz.build b/mobile/android/base/moz.build
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -613,16 +613,17 @@ moz_native_devices_sources = [
     'ChromeCast.java',
     'GeckoMediaPlayer.java',
     'MediaPlayerManager.java',
     'PresentationMediaPlayerManager.java',
 ]
 if CONFIG['MOZ_NATIVE_DEVICES']:
     gbjar.extra_jars += moz_native_devices_jars
     gbjar.sources += moz_native_devices_sources
+gbjar.extra_jars += [CONFIG['ANDROID_RECYCLERVIEW_LIB']]

 gbjar.javac_flags += ['-Xlint:all,-deprecation,-fallthrough', '-J-Xmx512m', '-J-Xms128m']

 # gecko-thirdparty is a good place to put small independent libraries
 gtjar = add_java_jar('gecko-thirdparty')
 gtjar.sources += [ thirdparty_source_dir + f for f in [
     'com/nineoldandroids/animation/Animator.java',
     'com/nineoldandroids/animation/AnimatorInflater.java',

https://pastebin.mozilla.org/8837043

> > [..]
> >  1:26.01 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v4/android-support-v4.jar]
> >  1:26.94 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v7/recyclerview/libs/android-support-v7-recyclerview.jar]
> >  1:27.25 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/google/google_play_services/libproject/google-play-services_lib/libs/google-play-services.jar]
> >  1:31.26 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v7/mediarouter/libs/android-support-v7-mediarouter.jar]
> >  1:31.38 Reading program jar [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar]
> >  1:32.07 Reading library jar [/home/sebastian/.mozbuild/android-sdk-linux/platforms/android-22/android.jar]
> >  1:32.19 /home/sebastian/Projects/FxTeam/mobile/android/base/home/PanelRecyclerView.java:4: error: cannot find symbol
> >  1:32.19 import android.support.v7.widget.LinearLayoutManager;
> >  1:32.20                                 ^
> >  1:32.20   symbol:   class LinearLayoutManager
> >  1:32.20   location: package android.support.v7.widget
> >  1:32.20 /home/sebastian/Projects/FxTeam/mobile/android/base/home/PanelRecyclerView.java:5: error: cannot find symbol
> >  1:32.20 import android.support.v7.widget.RecyclerView;
> >  1:32.20                                 ^
> >  1:32.20   symbol:   class RecyclerView
> >  1:32.20   location: package android.support.v7.widget
> >  1:32.21 /home/sebastian/Projects/FxTeam/mobile/android/base/home/PanelRecyclerView.java:10: error: cannot find symbol
> >  1:32.21 public class PanelRecyclerView extends RecyclerView {
> > [..]
> 
> From the log output it looks like it's picking up the code ("1:26.01 Reading
> program jar
> [/home/sebastian/.mozbuild/android-sdk-linux/extras/android/support/v4/
> android-support-v4.jar]"). However later it does not find all the classes.
> 
> @nalexander: Do you have an idea what's going wrong here?

This is an irritation: java -classpath -classpath ignores the first -classpath.  See the above little addition.

> (In reply to Nick Alexander :nalexander from comment #6)
> > * can I get a byte count on classes.dex before and after (with an actual
> > usage)?  I want to understand APK bloat a little.
> 
> Sure! I'll do that as soon as I've a working build.
> 
> (In reply to Nick Alexander :nalexander from comment #6)
> > * does this want a build flag?  Is it supporting a particular feature?  (I
> > think not, but want to understand.)
> 
> I want to use RecyclerView for some new home panel APIs (See bug 1157539 and
> bug 1172136). Martyn is planning to use it for some other UI updates and
> there is this meta bug 1158303 about replacing some of our ListViews with
> RecyclerView. In this context there's especially bug 1167942 where we want
> to use RecyclerView as replacement for ListView to work around a bug. So,
> yeah, there are plans to use it all over the app - This makes a build flag
> useless here, right?

Agreed.
Thank you once again! Now I can use RecyclerView from mach and gradle builds. I added the new patch for review.

I also pushed the patch to try:

Now I'll check what the addition of the library does to the byte count of classes.dex. The jar itself has a size of 248Kb.
Attachment #8623175 - Attachment is obsolete: true
Attachment #8623175 - Flags: feedback?(nalexander)
Attachment #8623609 - Flags: review?(nalexander)
Oops, I forgot the try link in my last comment:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0fc395e0d01

Looking at classes.dex, it's now roughly 190Kb (194940 bytes) larger:

Before applying patch:
> -rw-r--r--   1 sebastian  staff   5932272 Jun 17 12:09 classes.dex

After applying patch (clobber + rebuild):
> -rw-r--r--   1 sebastian  staff   6127212 Jun 17 14:08 classes.dex
Comment on attachment 8623609 [details] [diff] [review]
1171288-recylerview_v3.patch

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

Nice!  I'm actually pleasantly surprised how short this ended up being.
Attachment #8623609 - Flags: review?(nalexander) → review+
(In reply to :Sebastian Kaspari from comment #11)
> Oops, I forgot the try link in my last comment:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0fc395e0d01
> 
> Looking at classes.dex, it's now roughly 190Kb (194940 bytes) larger:
> 
> Before applying patch:
> > -rw-r--r--   1 sebastian  staff   5932272 Jun 17 12:09 classes.dex
> 
> After applying patch (clobber + rebuild):
> > -rw-r--r--   1 sebastian  staff   6127212 Jun 17 14:08 classes.dex

The amount of APK bloat is rough for this functionality: 42037603 - 41847595 = 190008, 190k.  Basically all the classes.dex.  We don't really have a process for assessing APK bloat, so I suggest that we ship this even with the bloat.  margaret, do you agree?

Try build:

[nalexander@people1.dmz.scl3 ~]$ curl -I http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/s.kaspari@gmail.com-b0fc395e0d01/try-android-api-11/fennec-41.0a1.en-US.android-arm.apk
<snip>
Content-Length: 42037603

Recent fx-team build:

[nalexander@people1.dmz.scl3 ~]$ curl -I http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/fx-team-android-api-11/1434447079/fennec-41.0a1.en-US.android-arm.apk
<snip>
Content-Length: 41847595
Flags: needinfo?(margaret.leibovic)
(In reply to Nick Alexander :nalexander from comment #13)
> The amount of APK bloat is rough for this functionality: 42037603 - 41847595
> = 190008, 190k.  Basically all the classes.dex.  We don't really have a
> process for assessing APK bloat, so I suggest that we ship this even with
> the bloat.  margaret, do you agree?

As briefly discussed in the mobile weekly meeting: We are planning to use RecyclerView for so many features that we'll have to accept the bloat for now. But: The addition of RecyclerView will allow us to get rid of our outdated TwoWayView version. Hopefully this will get us some of our precious bytes back. I'll file a follow-up bug for that now.
Flags: needinfo?(margaret.leibovic)
(In reply to :Sebastian Kaspari from comment #15)
> I'll file a follow-up bug for that now.

This is already covered by meta bug 1158303.
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/cae5d7fee424
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
See Also: → 1180132
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 41 → mozilla41
You need to log in before you can comment on or make changes to this bug.