Add observers for DIP scale change.

ui/android/java/src/org/chromium/ui/base/DIPScaleListener.java

Index: ui/android/java/src/org/chromium/ui/base/DIPScaleListener.java
diff --git a/ui/android/java/src/org/chromium/ui/base/DIPScaleListener.java b/ui/android/java/src/org/chromium/ui/base/DIPScaleListener.java
new file mode 100644
index 0000000000000000000000000000000000000000..8bad59294dbfeece451e3474d9f562a11e9141a2
--- /dev/null
+++ b/ui/android/java/src/org/chromium/ui/base/DIPScaleListener.java
@@ -0,0 +1,206 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.ui.base;
+
+import android.annotation.SuppressLint;
+import android.content.Context;
+import android.hardware.display.DisplayManager;
+import android.os.Build;
+
+import org.chromium.base.Log;
+import org.chromium.base.ObserverList;
+import org.chromium.base.ThreadUtils;
+import org.chromium.ui.gfx.DeviceDisplayInfo;
+
+/**
+ * Listens to changes in screen orientation.
+ */
+public interface DIPScaleListener {

[boliu, 2016/09/14 04:35:33]

Just call it DisplayObserver. We might hook up other things like orientation
later, and it's the observer pattern

[Tima Vaisburd, 2016/09/15 00:05:04]

Done.

+    /**
+     * Called whenever the DIP scale changes.
+     *
+     * @param dipScale Density Independent Pixel scale (display density).
+     */
+    void onDIPScaleChanged(float dipScale);
+}
+
+/**
+ * DIPScaleMonitor is a class that informs its listeners when the
+ * density independent pixel (DIP) scale changes.
+ *
+ * This class is meant to monitor changes for one window, which belongs to one display.
+ */
+class DIPScaleMonitor {
+    /**
+     * DisplayListener receives configuration change signal from the system.
+     */
+    @SuppressLint("NewApi")
+    private class DisplayListener implements DisplayManager.DisplayListener {

[boliu, 2016/09/14 04:35:33]

you could merge this with the Monitor class, and then maybe rename it to
Listener

[Tima Vaisburd, 2016/09/15 00:05:04]

kept here because of "NewApi". Because of this I did not rename DIPScaleMonitor
but can you this if you have a better name.

+        @Override
+        public void onDisplayAdded(int displayId) {}
+
+        @Override
+        public void onDisplayChanged(int displayId) {
+            updateDIPScale(displayId);
+        }
+
+        @Override
+        public void onDisplayRemoved(int displayId) {}
+    }
+
+    private static final String TAG = "cr_DIPScaleMonitor";
+
+    // A small value for floating point comparison between DIP scales.
+    // On a modern device at the time of writing the values may be
+    // 2.975, 3.5, 3.825, 4.1625, 4.5.
+    private static final float EPS = 0.0001f;
+
+    // List of observers.
+    private ObserverList<DIPScaleListener> mObservers = new ObserverList<DIPScaleListener>();
+
+    // DisplayListener receives notifications from the system.
+    private DisplayListener mDisplayListener = null;

[boliu, 2016/09/14 04:35:33]

remove = null;

[Tima Vaisburd, 2016/09/15 00:05:04]

Done.

+
+    // Current display density.
+    private float mDIPScale = 0;

[boliu, 2016/09/14 04:35:33]

remove = 0

[Tima Vaisburd, 2016/09/15 00:05:04]

Done.

+
+    // WindowAndroid we are attached to.
+    private WindowAndroid mWindowAndroid;
+
+    // DisplayManager used to start listening.
+    private DisplayManager mDisplayManager;
+
+    public DIPScaleMonitor(WindowAndroid windowAndroid) {
+        ThreadUtils.assertOnUiThread();

[boliu, 2016/09/14 04:35:33]

if you want to add thread asserts, then add them to all public methods, not just
a few random ones

[Tima Vaisburd, 2016/09/15 00:05:04]

I think the only public method I missed was getDIPScale(), added there.

+
+        mWindowAndroid = windowAndroid;
+    }
+
+    /**
+     * Shutdown procedure. To be called when associated WindowAndroid is destroyed.
+     */
+    public void onDestroy() {

[boliu, 2016/09/14 04:35:33]

this method is totally useless.. remove it

[Tima Vaisburd, 2016/09/15 00:05:04]

Done.

+        // All observers should have been removed.
+        assert mDisplayManager == null;
+        mWindowAndroid = null;
+    }
+
+    /**
+     * Returns current device scale factor for the window.
+     */
+    public float getDIPScale() {
+        // This method guarantees the up to date value only while we are monitoring, which happens
+        // only when there is at least one observer. In practice we use this method to set initial
+        // DIP scale which can be overwritten by observer any time, thus this is not a real
+        // restriction.
+        if (mDIPScale <= 0) {
+            DeviceDisplayInfo displayInfo =
+                    DeviceDisplayInfo.create(getDisplayContext(mWindowAndroid));

[boliu, 2016/09/14 04:35:33]

I guess this is ok for now.. but it would be better if we got rid of
DeviceDisplayInfo and replace it with just Display first :/

[Tima Vaisburd, 2016/09/15 00:05:04]

Do you mean just renaming DeviceDisplayInfo -> Display or a deeper surgery?
Besides, there exists android.view.Display, we might want another name.

[boliu, 2016/09/15 05:27:41]

Haven't looked into detail yet. But that whole DisplayInfo/ shared instance
thing needs to be nuked from orbit.
I think namespace will be good enough to distinguish the two.

+            mDIPScale = (float) displayInfo.getDIPScale();
+        }
+        return mDIPScale;
+    }
+
+    /**
+     * Adds |observer| in the DIPScaleListener observer list.
+     *
+     * @param listener The observer that will get notified.
+     * @param context The context associated with this observer.
+     * @return The DIP scale.
+     */
+    public void addObserver(DIPScaleListener listener) {
+        ThreadUtils.assertOnUiThread();
+
+        final boolean noPriorObserver = mObservers.isEmpty();
+
+        // Prevent notifying an already attached observer.
+        if (!mObservers.addObserver(listener)) return;

[boliu, 2016/09/14 04:35:33]

don't bother with this check, seems the client shouldn't be doing this in the
first place

[Tima Vaisburd, 2016/09/15 00:05:04]

Why to remove this functionality? Right now you can add and remove the same
observer several times.

[boliu, 2016/09/15 05:27:41]

You can remove this check and everything will still work the same way.

Seems odd that ObserverList decided to not treat repeated add as an error. I
wouldn't have just made it an error. Actually maybe you should throw in this
case. repeated observer makes no sense at all

[Tima Vaisburd, 2016/09/16 23:59:25]

Thew IllegalArgumentException.

+
+        if (noPriorObserver) {
+            // Added first observer.
+            assert mObservers.size() == 1;

[boliu, 2016/09/14 04:35:33]

asserts are useless

[Tima Vaisburd, 2016/09/15 00:05:04]

Removed in this file, I saw lots of them in WebView code however. What is our
position on asserts?

[boliu, 2016/09/15 05:27:41]

asserts don't do anything after some android version I can't remember.. K or L
there's a lot of old code that had asserts.

+
+            // Update cached DIP scale.
+            Context displayContext = getDisplayContext(mWindowAndroid);
+            mDIPScale = (float) DeviceDisplayInfo.create(displayContext).getDIPScale();

[boliu, 2016/09/14 04:35:33]

just call getDIPScale

[Tima Vaisburd, 2016/09/15 00:05:04]

I wanted to refresh. getDIPScale() could have been called first, then DIP scale
changed, then we added the first observer. In this case |mDIPScale| would have
the old value and getDIPScale() would return it. Unless getDIPScale() should
retrieve the value every time?

[boliu, 2016/09/15 05:27:41]

Sounds like you need an updateDIPScale in addition to get then. Basically don't
duplicate code

[Tima Vaisburd, 2016/09/16 23:59:25]

Added private getDIPScale(Context) which removed the code duplication, but at
the expense of retrieving the Context from mWindowAndroid more than necessary.

+
+            startListening(displayContext);
+        }
+
+        listener.onDIPScaleChanged(mDIPScale);
+    }
+
+    /**
+     * Removes the |observer| from the DIPScaleListener observer list.
+     *
+     * @param listener The observer that will no longer receive notification.
+     */
+    public void removeObserver(DIPScaleListener listener) {
+        ThreadUtils.assertOnUiThread();
+
+        final boolean wasRemoved = mObservers.removeObserver(listener);
+        if (wasRemoved && mObservers.isEmpty()) {
+            // Last observer is gone.
+            stopListening();
+        }
+    }
+
+    /**
+     * Helper methods
+     */
+
+    @SuppressLint("NewApi")
+    private void startListening(Context displayContext) {
+        Log.v(TAG, "startListening");

[boliu, 2016/09/14 04:35:33]

these seem like debug logs that should just be removed

[Tima Vaisburd, 2016/09/15 00:05:04]

Is DEBUG acceptable? I put |if (DEBUG) Log.v();|

[boliu, 2016/09/15 05:27:41]

how often is someone going to want to turn those on? seems like never..

[Tima Vaisburd, 2016/09/16 23:59:25]

Just removed the logs.

+
+        // DisplayManager is available at API level 17.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) return;
+
+        if (mDisplayListener == null) mDisplayListener = new DisplayListener();
+
+        assert mDisplayManager == null;
+
+        mDisplayManager = (DisplayManager) displayContext.getSystemService(Context.DISPLAY_SERVICE);
+        mDisplayManager.registerDisplayListener(mDisplayListener, null);
+    }
+
+    @SuppressLint("NewApi")
+    private void stopListening() {
+        Log.v(TAG, "stopListening");
+
+        // DisplayManager is available at API level 17.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) return;
+
+        mDisplayManager.unregisterDisplayListener(mDisplayListener);
+        mDisplayManager = null;
+    }
+
+    // Get current DIP scale and notify observers if necessary.
+    private void updateDIPScale(int displayId) {

[boliu, 2016/09/14 17:28:21]

is it possible to save the display id of the current window, and just skip
updates that belongs to other displays?

[Tima Vaisburd, 2016/09/14 17:38:19]

The only way I found was
  window -> context -> windowManager -> getDefaultDisplay() -> getDisplayId().

I think I can do then when I call getDisplayContext() for the first time.

[Tima Vaisburd, 2016/09/15 00:05:04]

Did in startListening().

+        Log.v(TAG, "updateDIPScale displayId:" + displayId);
+
+        ThreadUtils.assertOnUiThread();
+
+        if (mWindowAndroid == null) return;
+
+        DeviceDisplayInfo displayInfo = DeviceDisplayInfo.create(getDisplayContext(mWindowAndroid));
+        float newDIPScale = (float) displayInfo.getDIPScale();

[boliu, 2016/09/14 04:35:32]

call getDIPScale

[Tima Vaisburd, 2016/09/15 00:05:04]

Same consideration that I need to get the new value.

+
+        if (Math.abs(newDIPScale - mDIPScale) > EPS) {

[boliu, 2016/09/14 04:35:33]

I think actually equality is fine. It's not like these things are computed and
contain rounding errors.. unless the android api says to do this?

(and then get rid of EPS)

[Tima Vaisburd, 2016/09/15 00:05:04]

Yes. Silly me. Added a comment why floating point numbers comparison should be
ok here.

+            Log.v(TAG, "DIP scale changed, new DIP scale:" + newDIPScale);
+            mDIPScale = newDIPScale;
+            displayInfo.updateNativeSharedDisplayInfo();
+
+            for (DIPScaleListener listener : mObservers) listener.onDIPScaleChanged(newDIPScale);
+        }
+    }
+
+    private Context getDisplayContext(WindowAndroid windowAndroid) {
+        Context displayContext = windowAndroid.getContext().get();
+        // If displayContext does not exist the activity got destroyed and we do not care about

[boliu, 2016/09/14 04:35:33]

s/and/, /

[Tima Vaisburd, 2016/09/15 00:05:04]

Done.

+        // DIP scale any more. Use an application context as the simplest way to prevent a crash.
+        return displayContext != null ? displayContext : windowAndroid.getApplicationContext();
+    }
+}