591818 - Display error message for incompatible CPU

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

Our release builds are built for ARMv7,  but if a user with an incompatible cpu tries to run fennec we just crash with no explanation. An error message would at least provide a better experience.

Comment 1

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

Attachment: patch

Comment 2

[Michael Wu [:mwu]]

An alternative is to do the "right thing" and only support armv7 on android 2.2, as this allows us to specify that our apk requires armv7. Not sure how I feel about this though.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Brad: do we care about l10n here? I think a better message (or at least a more useful one) would be:

"This device does not meet the minimum system requirements for" + getAppName() +"."

Spewing information about the version of the arm processor isn't likely to be helpful to users.

I'd much prefer mwu's strategy, though, which (if I'm reading it right) would be to specify the requirement in the APK so that Android just wouldn't let someone try to install it in the first place. Not sure why he's not sure how he feels about that, though :)

Comment 4

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

(In reply to comment #3)
> Brad: do we care about l10n here? 
We will, but we don't have support for l10n in our java files yet (bug 594017)

> I think a better message (or at least a more
> useful one) would be:
> 
> "This device does not meet the minimum system requirements for" + getAppName()
> +"."

That certainly gets rid of the jargon, but given the sorts of people who are using our builds so far, perhaps having a way to get more details would be good.
> 
> Spewing information about the version of the arm processor isn't likely to be
> helpful to users.
it'll keep them from asking "I have a G1 running joe's super froyo rom, why aren't you detecting that right?" on forums and what not
> 
> I'd much prefer mwu's strategy, though, which (if I'm reading it right) would
> be to specify the requirement in the APK so that Android just wouldn't let
> someone try to install it in the first place. Not sure why he's not sure how he
> feels about that, though :)

That only exists in froyo, so we'd be upping our OS version requirement and excluding 75% of users.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

re: foyo-only: does it gracefully degrade, at least, so that people on pre-froyo builds can still use those APKs? If so, let's do that as well, no downside that I can see.

Not sure I agree that providing more information in the user-facing message will avoid forum spew, but am sure that we don't want to talk about armv7 yatta yatta in a user facing message. If there's a good URL to include with system requirements, I'd be happy to have that included.

Comment 6

[Michael Wu [:mwu]]

(In reply to comment #5)
> re: foyo-only: does it gracefully degrade, at least, so that people on
> pre-froyo builds can still use those APKs? If so, let's do that as well, no
> downside that I can see.
> 
We can degrade gracefully at the cost of nearly doubling our package size.

Comment 7

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

(In reply to comment #6)
> (In reply to comment #5)
> > re: foyo-only: does it gracefully degrade, at least, so that people on
> > pre-froyo builds can still use those APKs? If so, let's do that as well, no
> > downside that I can see.
> > 
> We can degrade gracefully at the cost of nearly doubling our package size.

This isn't really an option

Comment 8

[Michael Wu [:mwu]]

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

I don't see a splashscreen when testing with this patch.

Comment 9

[Michael Wu [:mwu]]

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

Uh wrong patch.

Comment 10

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

do you have --enable-splashscreen in your mozconfig?

Comment 11

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

oh, yea... wrong patch :-)

Comment 12

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

Attachment: patch v.2

This uses Beltzner's suggested for the string.

Bug 594017 is for localizing the android java files, but this is blocking beta 1 so it should land before the l10n support does.

Comment 13

[Michael Wu [:mwu]]

Comment on attachment 478183 [details] [diff] [review]
patch v.2

Looks ok. Cpu arch checking code is kinda sketchy looking but we don't have much choice if we want to support non-froyo.

>diff --git a/embedding/android/GeckoApp.java b/embedding/android/GeckoApp.java
>--- a/embedding/android/GeckoApp.java
>+++ b/embedding/android/GeckoApp.java
>@@ -112,7 +112,44 @@ abstract public class GeckoApp
>                                                   ViewGroup.LayoutParams.FILL_PARENT));
> 
>         if (!GeckoAppShell.sGeckoRunning) {
>-            
>+            try {
>+                BufferedReader reader =
>+                    new BufferedReader(new FileReader("/proc/cpuinfo"));
>+                String line = reader.readLine();
>+                while (line != null) {
while ((line = reader.readLine()) != null)

>+                    int index = line.indexOf("CPU architecture:");
>+                    if (index != -1) {
if (index == -1)
    continue;

>+                        String versionStr = line.substring(18);
>+                        Log.i("GeckoApp", "cpu version: " + versionStr);
>+                        int version = Integer.parseInt(versionStr);
>+                        Log.i("GeckoApp", "cpu version: " + version);
remove this line.

>+                        int minVer = getMinCPUVersion();
>+                        if (version < minVer) {
version < getMinCPUVersion()

>+                            new AlertDialog.Builder(this)
>+                                .setMessage("This device does not meet the " +
>+                                            "minimum system requirements for" +
>+                                            getAppName() + ".")
>+                                .setCancelable(false)
>+                                .setPositiveButton("Exit", 
>+                                        new DialogInterface.OnClickListener() {
>+                                    public void onClick(DialogInterface dialog, int id)
>+                                    {
>+                                        GeckoApp.this.finish();
>+                                    }
>+                                        }).show();
>+                            return;
Use the new error message function that you introduced.

>+                        }
else
    break;

>+                        line = null;
>+                    } else {
>+                        line = reader.readLine();
>+                    }
>+                }
>+                
>+            } catch(Exception ex) {
space after catch.

>diff --git a/embedding/android/Makefile.in b/embedding/android/Makefile.in
>--- a/embedding/android/Makefile.in
>+++ b/embedding/android/Makefile.in
>@@ -57,11 +57,26 @@ PROCESSEDJAVAFILES = \
>   NotificationHandler.java \
>   $(NULL)
> 
>+ifneq (,$(findstring -march=armv8,$(OS_CFLAGS)))
>+MIN_CPU_VERSION=8
>+else
>+ifneq (,$(findstring -march=armv7,$(OS_CFLAGS)))
>+MIN_CPU_VERSION=7
>+else
>+ifneq (,$(findstring -march=armv6,$(OS_CFLAGS)))
>+MIN_CPU_VERSION=6
>+else
>+MIN_CPU_VERSION=5
>+endif
>+endif
>+endif
>+
Lets just check armv5 for now and set the minimum to 7 if we can't find it. For builds on archs beyond v7, it is reasonable to expect froyo or newer to be installed, so the installer will be able to handle those cases for us.

> DEFINES += \
>   -DMOZ_APP_DISPLAYNAME=$(MOZ_APP_DISPLAYNAME) \
>   -DMOZ_APP_NAME=$(MOZ_APP_NAME) \
>   -DMOZ_APP_VERSION=$(MOZ_APP_VERSION) \
>-  -DMOZ_CHILD_PROCESS_NAME=$(MOZ_CHILD_PROCESS_NAME)
>+  -DMOZ_CHILD_PROCESS_NAME=$(MOZ_CHILD_PROCESS_NAME) \
>+  -DMOZ_MIN_CPU_VERSION=$(MIN_CPU_VERSION)
add a $(NULL) so we won't have to add more \ in the future.

Comment 14

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

Attachment: patch

Comment 15

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

pushed http://hg.mozilla.org/mozilla-central/rev/9727eb08513b

Comment 16

[Cristian Nicolae (:xti)]

I was unable to install the latest Nightly build on HTC Legend. I will close this bug as verified fixed.

--
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110919
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2