719560 - Can't publish split native and xul builds under the same product on android market

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Chris AtLee [:catlee]]

I've uploaded both split builds from blassey to the android market under a test product, and am unable to activate both APKs simultaneously. The error is:

"Error: APK version 2012011708 supports all the same devices as other APKs with higher versions. It would not be served. Please deactivate an APK."

version 2012011708 refers to the split xul build.

Comment 1

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

Attachment: patch

making builds for catlee to test now

Comment 2

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

Attachment: patch

Comment 3

[Matt Brubeck (:mbrubeck)]

Comment on attachment 590240 [details] [diff] [review]
patch

The android:resizeable attribute is deprecated and was apparently only used in Android 1.6.  I think we can remove it.  (The docs say "You should not use it.")

r=mbrubeck with that change.

Comment 4

[Matt Brubeck (:mbrubeck)]

Note: *If* we end up *not* shipping the native version alongside the XUL version (say, for Firefox 11), we should make sure this change is backed out of that XUL version for that release.

Comment 5

[Matt Brubeck (:mbrubeck)]

Nominating for tracking-firefox11 because this patch depends on our go/no-go decision to ship native Fennec.  (See comment 4.)

Comment 6

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

Attachment: patch

this patch will keep trunk installing on all devices while building aurora, beta and release to allow for the split release

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 590977 [details] [diff] [review]
patch

Based on testing in bug 721551, we actually don't need to set xlargeScreens="false" for native Fennec.  So we could leave the native Fennec manifest and configure settings unchanged, and check in only the XUL Fennec changes.

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Flyer is downloading the Native version with the split APK.

Comment 9

[Alex Keybl [:akeybl]]

(In reply to Naoki Hirata :nhirata from comment #8)
> Flyer is downloading the Native version with the split APK.

That's expected, right? Doesn't it fall under the same criteria as the Kindle Fire and get the phone UI?

Do we have a document full of test results that we can reference and verify?

Comment 10

[Martijn Wargers (dead)]

I've made a document here: https://wiki.mozilla.org/User:MartijnWargers/splitbuild_testing/

Comment 11

[Matt Brubeck (:mbrubeck)]

Comment on attachment 590977 [details] [diff] [review]
patch

Updated review comments based on new information:

>+++ b/embedding/android/Makefile.in

> ifeq (,$(ANDROID_VERSION_CODE))
>+# decrement the version code by 1 so native fennec will win any compatability ties
>+ANDROID_VERSION_CODE=$(shell echo `$(PYTHON) $(topsrcdir)/toolkit/xre/make-platformini.py --print-buildid | cut -c1-10` - 1 | bc)
>+endif

This should be flipped; the XUL version needs to be higher than the native version.  See bug 721551.

>+++ b/mobile/android/base/AndroidManifest.xml.in

>+#ifdef MOZ_PHONES_ONLY
>+    <supports-screens android:smallScreens="true"
>+                      android:normalScreens="true"
>+                      android:largeScreens="true"
>+                      android:xlargeScreens="false" />
>+#endif

In bug 721551 we found this had no effect on the Market; tablet-only apps are not offered to phones, but phone-only apps are still offered to tablets.  We can remove all the native Fennec parts of this patch, and check in only the XUL fennec changes.

r=mbrubeck with those changes.

Comment 12

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/934ce089223c

Comment 13

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

backed this out because the xul builds were not installing on the tegras and thus all tests were failing for xul builds

Comment 14

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

Attachment: patch to not restrict based on OS version

this went ran green on try. The essential change here from the original patch is to not restrict based on OS version. In addition this patch fixes the Makefile logic that was broken in the original patch.

Comment 15

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

try run: https://tbpl.mozilla.org/?tree=Try&rev=e37c6dc2bcab

Comment 16

[Matt Brubeck (:mbrubeck)]

Comment on attachment 593653 [details] [diff] [review]
patch to not restrict based on OS version

>+++ b/embedding/android/Makefile.in
> 
>+# decrement the version code by 1 so native fennec will win any compatability ties
>+ANDROID_VERSION_CODE=$(shell echo `$(PYTHON) $(topsrcdir)/toolkit/xre/make-platformini.py --print-buildid | cut -c1-10` + 1 | bc)

Nit: The comment needs to be updated (it's now reversed).

Comment 17

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/00d5bbcc98c8

Comment 18

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

Comment on attachment 593653 [details] [diff] [review]
patch to not restrict based on OS version

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:

Comment 19

[Alex Keybl [:akeybl]]

Comment on attachment 593653 [details] [diff] [review]
patch to not restrict based on OS version

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/00d5bbcc98c8

Comment 21

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/ae27914c62ac
https://hg.mozilla.org/releases/mozilla-beta/rev/33e29c66740d

Comment 22

[Matt Brubeck (:mbrubeck)]

This broke XUL Fennec browser-chrome tests and reftests on Aurora and Beta (but not on mozilla-central, probably because it is only enabled in the Aurora/Beta mozconfigs).

Is there a way we can land these mozconfig changes there without breaking these tests and having to hide them..?

Comment 23

[Matt Brubeck (:mbrubeck)]

I backed this out on Aurora and Beta for now:
https://hg.mozilla.org/releases/mozilla-aurora/rev/ff1c55497404
https://hg.mozilla.org/releases/mozilla-beta/rev/5c99a7357e6c

I'm still not sure of the best solution.  Maybe we could enable this only for "official" beta/release builds?

Comment 24

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

(In reply to Matt Brubeck (:mbrubeck) from comment #22)
> This broke XUL Fennec browser-chrome tests and reftests on Aurora and Beta
> (but not on mozilla-central, probably because it is only enabled in the
> Aurora/Beta mozconfigs).
> 
> Is there a way we can land these mozconfig changes there without breaking
> these tests and having to hide them..?

how did this patch break them? if it is installing we should be fine.

Comment 25

[Matt Brubeck (:mbrubeck)]

(In reply to Brad Lassey [:blassey] from comment #24)
> how did this patch break them? if it is installing we should be fine.

I have no idea why they broke, but browser-chrome and all crashtest and reftest suites went perma-orange with multiple failures:
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=ae27914c62ac

Comment 26

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: allow screen resizing - required for running reftests (1.0)

Comment 27

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

Attachment: patch to include large screens in xul

Here's a try run with this patch, plus a patch to make try act like aurora and beta:
https://tbpl.mozilla.org/?tree=Try&rev=d2185388f15a

Comment 28

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

try run is green

Comment 29

[Matt Brubeck (:mbrubeck)]

Comment on attachment 594402 [details] [diff] [review]
patch to include large screens in xul

Note that with this patch, 7" tablets will get XUL fennec from the Android Market, though they don't use the tablet layout.

(The most popular 7" tablets like the Kindle Fire and Nook Color don't use the Android Market anyway, so this change will affect a fairly small number of devices.)

Comment 30

[Ed Morley [:emorley]]

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

Comment 31

[Aaron Train [:aaronmt]]

Does the above patch need to land elsewhere?

Comment 32

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

Comment on attachment 594402 [details] [diff] [review]
patch to include large screens in xul

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:

Comment 33

[Alex Keybl [:akeybl]]

Comment on attachment 594402 [details] [diff] [review]
patch to include large screens in xul

[Triage Comment]
Approved for Aurora 12 and Beta 11.

Comment 35

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

https://hg.mozilla.org/releases/mozilla-beta/rev/610cfe719893
https://hg.mozilla.org/releases/mozilla-beta/rev/043d4b7d469d