Reland of Android: support multiple displays on C++ side

ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java

Index: ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java
diff --git a/ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java b/ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java
index 075754c76eb64468255b561e9b672d44ef45acac..b4fa44b97dda2b83de3cc473d0ba580361f0bf7c 100644
--- a/ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java
+++ b/ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java
@@ -6,13 +6,16 @@ package org.chromium.ui.display;
 
 import android.annotation.TargetApi;
 import android.content.Context;
+import android.graphics.PixelFormat;
 import android.graphics.Point;
 import android.os.Build;
 import android.util.DisplayMetrics;
 import android.view.Display;
+import android.view.Surface;
 
 import org.chromium.base.CommandLine;
 import org.chromium.base.Log;
+import org.chromium.base.annotations.JNINamespace;
 
 import java.util.WeakHashMap;
 
@@ -23,6 +26,7 @@ import java.util.WeakHashMap;
  * anywhere, as long as the corresponding WindowAndroids are destroyed. The observers are
  * held weakly so to not lead to leaks.
  */
+@JNINamespace("ui")

[mthiesse, 2016/10/26 14:49:49]

I think the native code for Displays on android should be in
DisplayAndroidManager. I think DisplayAndroid should be equivalent to the native
Display class that's usually passed by value and stay as lightweight as
possible.

[boliu, 2016/10/26 17:24:20]

Pass by value is not really a thing in java, but yeah, java DisplayAndroid
should be as light weight as possible. That doesn't mean it can't talk to native
things. As long as the common accessors don't do jni, it's just as light weight
as before.

 public class DisplayAndroid {
     /**
      * DisplayAndroidObserver interface for changes to this Display.
@@ -49,6 +53,7 @@ public class DisplayAndroid {
     private final Point mSize;
     private final Point mPhysicalSize;
     private final DisplayMetrics mDisplayMetrics;
+    private int mPixelFormatId;
     private int mRotation;
 
     // When this object exists, a positive value means that the forced DIP scale is set and
@@ -97,6 +102,13 @@ public class DisplayAndroid {
     }
 
     /**
+     * @return Display id as defined in Android's Display.
+     */
+    public int getSdkDisplayId() {

[mthiesse, 2016/10/26 14:49:49]

If we store a mapping from window to display in both java and native
DisplayAndroidManager, we don't need to expose DisplayIds anywhere. window would
just call into DisplayAndroidManager to get the display for itself.

[boliu, 2016/10/26 17:24:20]

yeah, this is one of those weird things where we really wish to expose this to
WindowAndroid only, not the world. But maybe a comment, or
getSdkDisplayIdInternal would be good enough here.

I think I'd rather not have native side have an explicit window -> display
mapping because feels like keeping that mapping up-to-date is a lot of work.
Unless you see some advantage?

[mthiesse, 2016/10/26 17:52:49]

The main benefit is consistency between native and java sides. It's weird that
WindowAndroid java stores a cached DisplayAndroid, but native stores an ID and
gets the display from somewhere else. I'm okay with them both just storing an ID
and fetching the display from the Manager, but that requires exposing a
getDisplayById-style function in DisplayAndroidManager, which you seemed to be
against. Also public methods with names like getSdkDisplayIdInternal kind of
suck :(

I don't see storing both a native and java window->display map as being
relevantly more work than the implicit mapping done by passing a display ID
around, and it makes things much easier to reason about. Both maps would be
updated only by DisplayAndroidManager,
DisplayAndroidManager::SetDisplayForWindow would just call
nativeSetDisplayForWindow, etc.

[boliu, 2016/10/26 18:24:43]

Ok I objected to that because the current thing seems cleaner, but the
inconsistency does seem worse. How about this. Let's assume we've nailed down
the concept of the primary display, which we have to guarantee (at least in
code) that it never goes away. Then on both the java and native side, if the
display the window was referring to goes away, it just falls back to primary
one.

That still exposes display id to WindowAndroid though. I don't think it's really
necessary to hide it since it's visible from native side already. And we still
don't need a window->display map anywhere :p

[mthiesse, 2016/10/26 18:48:24]

sgtm

[Tima Vaisburd, 2016/10/27 07:55:58]

Done, except for Java side.

+        return mSdkDisplayId;
+    }
+
+    /**
      * @return Display height in physical pixels.
      */
     public int getDisplayHeight() {
@@ -132,6 +144,26 @@ public class DisplayAndroid {
     }
 
     /**
+     * @return current orientation in degrees. One of the values 0, 90, 180, 270.
+     */
+    public int getRotationDegrees() {
+        switch (mRotation) {
+            case Surface.ROTATION_0:
+                return 0;
+            case Surface.ROTATION_90:
+                return 90;
+            case Surface.ROTATION_180:
+                return 180;
+            case Surface.ROTATION_270:
+                return 270;
+        }
+
+        // This should not happen.
+        assert false;
+        return 0;
+    }
+
+    /**
      * @return A scaling factor for the Density Independent Pixel unit.
      */
     public float getDIPScale() {
@@ -139,6 +171,50 @@ public class DisplayAndroid {
     }
 
     /**
+     * @return Number of bits per pixel.
+     */
+    public int getBitsPerPixel() {
+        PixelFormat info = new PixelFormat();
+        PixelFormat.getPixelFormatInfo(mPixelFormatId, info);
+        return info.bitsPerPixel;
+    }
+
+    /**
+     * @return Number of bits per each color component.
+     */
+    @SuppressWarnings("deprecation")
+    public int getBitsPerComponent() {
+        switch (mPixelFormatId) {
+            case PixelFormat.RGBA_4444:
+                return 4;
+
+            case PixelFormat.RGBA_5551:
+                return 5;
+
+            case PixelFormat.RGBA_8888:
+            case PixelFormat.RGBX_8888:
+            case PixelFormat.RGB_888:
+                return 8;
+
+            case PixelFormat.RGB_332:
+                return 2;
+
+            case PixelFormat.RGB_565:
+                return 5;
+
+            // Non-RGB formats.
+            case PixelFormat.A_8:
+            case PixelFormat.LA_88:
+            case PixelFormat.L_8:
+                return 0;
+
+            // Unknown format. Use 8 as a sensible default.
+            default:
+                return 8;
+        }
+    }
+
+    /**
      * Add observer. Note repeat observers will be called only one.
      * Observers are held only weakly by Display.
      */
@@ -175,11 +251,19 @@ public class DisplayAndroid {
         mSize = new Point();
         mPhysicalSize = new Point();
         mDisplayMetrics = new DisplayMetrics();
+        mPixelFormatId = PixelFormat.RGBA_8888;
         updateFromDisplay(display);
     }
 
+    @SuppressWarnings("deprecation")
     @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1)
     /* package */ void updateFromDisplay(Display display) {
+        final Point oldSize = new Point(mSize);
+        final Point oldPhysicalSize = new Point(mPhysicalSize);
+        final float oldDensity = mDisplayMetrics.density;
+        final int oldPixelFormatId = mPixelFormatId;
+        final int oldRotation = mRotation;
+
         display.getSize(mSize);
         display.getMetrics(mDisplayMetrics);
 
@@ -189,11 +273,21 @@ public class DisplayAndroid {
             display.getRealSize(mPhysicalSize);
         }
 
-        int newRotation = display.getRotation();
-        boolean rotationChanged = newRotation != mRotation;
-        mRotation = newRotation;
+        // JellyBean MR1 and later always uses RGBA_8888.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
+            mPixelFormatId = display.getPixelFormat();
+        }
+
+        mRotation = display.getRotation();
 
-        if (rotationChanged) {
+        final boolean noChanges = oldSize.equals(mSize) && oldPhysicalSize.equals(mPhysicalSize)
+                && oldDensity == mDisplayMetrics.density && oldPixelFormatId == mPixelFormatId
+                && oldRotation == mRotation;
+        if (noChanges) return;
+
+        updateNativeSide();
+
+        if (oldRotation != mRotation) {
             DisplayAndroidObserver[] observers = getObservers();
             for (DisplayAndroidObserver o : observers) {
                 o.onRotationChanged(mRotation);
@@ -201,8 +295,27 @@ public class DisplayAndroid {
         }
     }
 
+    /* package */ void onAddedToMap() {
+        updateNativeSide();
+    }
+
+    /* package */ void onRemovedFromMap() {
+        nativeRemoveDisplayAndroid(mSdkDisplayId);
+    }
+
+    private void updateNativeSide() {
+        nativeUpdateDisplayAndroid(mSdkDisplayId, mPhysicalSize.x, mPhysicalSize.y, mSize.x,
+                mSize.y, mDisplayMetrics.density, getRotationDegrees(), getBitsPerPixel(),
+                getBitsPerComponent());
+    }
+
     private DisplayAndroidObserver[] getObservers() {
         // Makes a copy to allow concurrent edit.
         return mObservers.keySet().toArray(EMPTY_OBSERVER_ARRAY);
     }
+
+    private static native void nativeUpdateDisplayAndroid(int sdkDisplayId, int physicalWidth,
+            int physicalHeight, int width, int height, float dipScale, int rotationDegrees,
+            int bitsPerPixel, int bitsPerComponent);
+    private static native void nativeRemoveDisplayAndroid(int sdkDisplayId);
 }