Bug 1318667 - Do not use palette library on x86 devices (Use BitmapUtils.getDominantColor()). r?ahunt draft
authorSebastian Kaspari <s.kaspari@gmail.com>
Fri, 03 Feb 2017 18:42:36 +0100
changeset 470408 518f78414a6f04186cd04597133543feb8a0087b
parent 469731 cf3df22668918dd066df53b473d8d44f4019a0c9
child 544463 f913b9d091b6f882d0edd2c53e64c328e7fd3e91
push id44012
push users.kaspari@gmail.com
push dateFri, 03 Feb 2017 17:43:29 +0000
reviewersahunt
bugs1318667
milestone54.0a1
Bug 1318667 - Do not use palette library on x86 devices (Use BitmapUtils.getDominantColor()). r?ahunt Our version of the palette support library crashes for certain icons on x86 systems. MozReview-Commit-ID: E6eEyFXd4uK
mobile/android/base/java/org/mozilla/gecko/icons/processing/ColorProcessor.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/icons/processing/ColorProcessor.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/processing/ColorProcessor.java
@@ -6,36 +6,56 @@
 package org.mozilla.gecko.icons.processing;
 
 import android.support.v7.graphics.Palette;
 import android.util.Log;
 
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.icons.IconRequest;
 import org.mozilla.gecko.icons.IconResponse;
+import org.mozilla.gecko.util.HardwareUtils;
 
 /**
  * Processor implementation to extract the dominant color from the icon and attach it to the icon
  * response object.
  */
 public class ColorProcessor implements Processor {
     private static final String LOGTAG = "GeckoColorProcessor";
     private static final int DEFAULT_COLOR = 0; // 0 == No color
 
     @Override
     public void process(IconRequest request, IconResponse response) {
         if (response.hasColor()) {
             return;
         }
 
+        if (HardwareUtils.isX86System()) {
+            // (Bug 1318667) We are running into crashes when using the palette library with
+            // specific icons on x86 devices. They take down the whole VM and are not recoverable.
+            // Unfortunately our release icon is triggering this crash. Until we can switch to a
+            // newer version of the support library where this does not happen, we are using our
+            // own slower implementation.
+            extractColorUsingCustomImplementation(response);
+        } else {
+            extractColorUsingPaletteSupportLibrary(response);
+        }
+    }
+
+    private void extractColorUsingPaletteSupportLibrary(final IconResponse response) {
         try {
             final Palette palette = Palette.from(response.getBitmap()).generate();
             response.updateColor(palette.getVibrantColor(DEFAULT_COLOR));
         } catch (ArrayIndexOutOfBoundsException e) {
             // We saw the palette library fail with an ArrayIndexOutOfBoundsException intermittently
             // in automation. In this case lets just swallow the exception and move on without a
             // color. This is a valid condition and callers should handle this gracefully (Bug 1318560).
             Log.e(LOGTAG, "Palette generation failed with ArrayIndexOutOfBoundsException", e);
 
             response.updateColor(DEFAULT_COLOR);
         }
     }
+
+    private void extractColorUsingCustomImplementation(final IconResponse response) {
+        final int dominantColor = BitmapUtils.getDominantColor(response.getBitmap());
+
+        response.updateColor(dominantColor);
+    }
 }
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareUtils.java
@@ -74,23 +74,31 @@ public final class HardwareUtils {
     public static boolean isTelevision() {
         return sIsTelevision;
     }
 
     public static int getMemSize() {
         return SysInfo.getMemSize();
     }
 
+    public static boolean isARMSystem() {
+        return Build.CPU_ABI != null && Build.CPU_ABI.startsWith("arm");
+    }
+
+    public static boolean isX86System() {
+        return Build.CPU_ABI != null && Build.CPU_ABI.startsWith("x86");
+    }
+
     /**
      * @return false if the current system is not supported (e.g. APK/system ABI mismatch).
      */
     public static boolean isSupportedSystem() {
         // See http://developer.android.com/ndk/guides/abis.html
-        boolean isSystemARM = Build.CPU_ABI != null && Build.CPU_ABI.startsWith("arm");
-        boolean isSystemX86 = Build.CPU_ABI != null && Build.CPU_ABI.startsWith("x86");
+        final boolean isSystemARM = isARMSystem();
+        final boolean isSystemX86 = isX86System();
 
         boolean isAppARM = BuildConfig.ANDROID_CPU_ARCH.startsWith("arm");
         boolean isAppX86 = BuildConfig.ANDROID_CPU_ARCH.startsWith("x86");
 
         // Only reject known incompatible ABIs. Better safe than sorry.
         if ((isSystemX86 && isAppARM) || (isSystemARM && isAppX86)) {
             return false;
         }