Last Comment Bug 719560 - Can't publish split native and xul builds under the same product on android market
: Can't publish split native and xul builds under the same product on android m...
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: Firefox 13
Assigned To: Brad Lassey [:blassey] (use needinfo?)
:
Mentors:
: 721551 (view as bug list)
Depends on: 721551 752681 768613
Blocks: 747531
  Show dependency treegraph
 
Reported: 2012-01-19 12:59 PST by Chris AtLee [:catlee]
Modified: 2012-06-26 13:05 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
affected
fixed
beta+
11+


Attachments
patch (1.67 KB, patch)
2012-01-19 13:56 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch (2.48 KB, patch)
2012-01-20 10:12 PST, Brad Lassey [:blassey] (use needinfo?)
mbrubeck: review+
Details | Diff | Review
patch (6.57 KB, patch)
2012-01-23 19:15 PST, Brad Lassey [:blassey] (use needinfo?)
mbrubeck: review+
Details | Diff | Review
patch to not restrict based on OS version (4.11 KB, patch)
2012-02-01 16:44 PST, Brad Lassey [:blassey] (use needinfo?)
mbrubeck: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review
allow screen resizing - required for running reftests (1.0) (763 bytes, patch)
2012-02-03 11:56 PST, Joel Maher (:jmaher)
blassey.bugs: review+
Details | Diff | Review
patch to include large screens in xul (791 bytes, patch)
2012-02-04 00:14 PST, Brad Lassey [:blassey] (use needinfo?)
mbrubeck: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description Chris AtLee [:catlee] 2012-01-19 12:59:56 PST
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?) 2012-01-19 13:56:20 PST
Created attachment 589981 [details] [diff] [review]
patch

making builds for catlee to test now
Comment 2 Brad Lassey [:blassey] (use needinfo?) 2012-01-20 10:12:58 PST
Created attachment 590240 [details] [diff] [review]
patch
Comment 3 Matt Brubeck (:mbrubeck) 2012-01-20 10:29:43 PST
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) 2012-01-20 11:56:55 PST
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) 2012-01-20 12:06:18 PST
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?) 2012-01-23 19:15:22 PST
Created attachment 590977 [details] [diff] [review]
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) 2012-01-27 14:30:08 PST
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) 2012-01-28 10:58:39 PST
Flyer is downloading the Native version with the split APK.
Comment 9 Alex Keybl [:akeybl] 2012-01-29 19:49:01 PST
(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 [:mwargers] (not working for Mozilla) 2012-01-30 07:09:08 PST
I've made a document here: https://wiki.mozilla.org/User:MartijnWargers/splitbuild_testing/
Comment 11 Matt Brubeck (:mbrubeck) 2012-01-30 17:23:32 PST
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?) 2012-01-30 17:56:08 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/934ce089223c
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2012-01-30 20:36:46 PST
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?) 2012-02-01 16:44:49 PST
Created attachment 593653 [details] [diff] [review]
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?) 2012-02-01 16:45:48 PST
try run: https://tbpl.mozilla.org/?tree=Try&rev=e37c6dc2bcab
Comment 16 Matt Brubeck (:mbrubeck) 2012-02-01 17:22:05 PST
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?) 2012-02-01 18:14:21 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/00d5bbcc98c8
Comment 18 Brad Lassey [:blassey] (use needinfo?) 2012-02-01 20:58:13 PST
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] 2012-02-02 06:40:03 PST
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] 2012-02-02 06:56:55 PST
https://hg.mozilla.org/mozilla-central/rev/00d5bbcc98c8
Comment 22 Matt Brubeck (:mbrubeck) 2012-02-02 16:28:11 PST
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) 2012-02-02 16:38:54 PST
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?) 2012-02-02 22:33:40 PST
(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) 2012-02-03 08:28:41 PST
(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) 2012-02-03 11:56:16 PST
Created attachment 594265 [details] [diff] [review]
allow screen resizing - required for running reftests (1.0)
Comment 27 Brad Lassey [:blassey] (use needinfo?) 2012-02-04 00:14:59 PST
Created attachment 594402 [details] [diff] [review]
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?) 2012-02-04 09:00:20 PST
try run is green
Comment 29 Matt Brubeck (:mbrubeck) 2012-02-04 10:34:47 PST
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] 2012-02-05 04:04:52 PST
https://hg.mozilla.org/mozilla-central/rev/e3bb8af42f98
Comment 31 Aaron Train [:aaronmt] 2012-02-06 08:32:31 PST
Does the above patch need to land elsewhere?
Comment 32 Brad Lassey [:blassey] (use needinfo?) 2012-02-06 14:24:43 PST
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] 2012-02-06 14:27:41 PST
Comment on attachment 594402 [details] [diff] [review]
patch to include large screens in xul

[Triage Comment]
Approved for Aurora 12 and Beta 11.
Comment 34 Brad Lassey [:blassey] (use needinfo?) 2012-02-06 14:58:40 PST
*** Bug 721551 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.