Implement java-side browser sensor polling for device motion and device orientation DOM events.

content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 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.content.browser;
 
 import android.content.Context;
 import android.hardware.Sensor;
 import android.hardware.SensorEvent;
 import android.hardware.SensorEventListener;
 import android.hardware.SensorManager;
 import android.os.Handler;
 import android.os.Looper;
+import android.util.Log;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 import org.chromium.base.WeakContext;
 
 import java.util.List;
+import java.util.Set;
 
 /**
- * Android implementation of the DeviceOrientation API.
+ * Android implementation of the device motion and orientation APIs.
  */
 @JNINamespace("content")
-class DeviceOrientation implements SensorEventListener {
+class DeviceMotionAndOrientation implements SensorEventListener {
+
+    private static final String LOGTAG = "DeviceMotionAndOrientation";

[bulach, 2013/03/19 15:22:36]

nit: normally this is just "TAG"

[timvolodine, 2013/03/19 16:50:08]

Done.

 
     // These fields are lazily initialized by getHandler().
     private Thread mThread;
     private Handler mHandler;
 
     // The lock to access the mHandler.
     private Object mHandlerLock = new Object();
 
     // Non-zero if and only if we're listening for events.
     // To avoid race conditions on the C++ side, access must be synchronized.
     private int mNativePtr;
 
     // The lock to access the mNativePtr.
     private Object mNativePtrLock = new Object();
 
-    // The gravity vector expressed in the body frame.
-    private float[] mGravityVector;
+    // The acceleration vector including gravity expressed in the body frame.
+    private float[] mAccelerationVector;
 
     // The geomagnetic vector expressed in the body frame.
     private float[] mMagneticFieldVector;
 
     // Lazily initialized when registering for notifications.
-    private SensorManager mSensorManager;
+    private SensorManagerProxy mSensorManagerProxy;
 
     // The only instance of that class and its associated lock.
-    private static DeviceOrientation sSingleton;
+    private static DeviceMotionAndOrientation sSingleton;
     private static Object sSingletonLock = new Object();
 
-    private DeviceOrientation() {
+    /**
+     * constants for using in JNI calls, also see
+     * content/browser/device_orientation/data_fetcher_impl_android.cc
+     */
+    static final int DEVICE_ORIENTATION = 0;
+    static final int DEVICE_MOTION = 1;
+
+    static ImmutableSet<Integer> DEVICE_ORIENTATION_SENSORS = ImmutableSet.of(
+            Sensor.TYPE_ACCELEROMETER,
+            Sensor.TYPE_MAGNETIC_FIELD);
+
+    static ImmutableSet<Integer> DEVICE_MOTION_SENSORS = ImmutableSet.of(
+            Sensor.TYPE_ACCELEROMETER,
+            Sensor.TYPE_LINEAR_ACCELERATION,
+            Sensor.TYPE_GYROSCOPE);
+
+    @VisibleForTesting
+    Set<Integer> mActiveSensors = Sets.newHashSet();

[bulach, 2013/03/19 15:22:36]

nit: final here, 69 and 73

[timvolodine, 2013/03/19 16:50:08]

Done.

+    boolean mDeviceMotionIsActive = false;
+    boolean mDeviceOrientationIsActive = false;
+
+    protected DeviceMotionAndOrientation() {
     }
 
     /**
      * Start listening for sensor events. If this object is already listening
      * for events, the old callback is unregistered first.
      *
      * @param nativePtr Value to pass to nativeGotOrientation() for each event.
      * @param rateInMilliseconds Requested callback rate in milliseconds. The
      *            actual rate may be higher. Unwanted events should be ignored.
+     * @param eventType Type of event to listen to, can be either DEVICE_ORIENTATION or
+     *                  DEVICE_MOTION.
      * @return True on success.
      */
     @CalledByNative
-    public boolean start(int nativePtr, int rateInMilliseconds) {
+    public boolean start(int nativePtr, int eventType, int rateInMilliseconds) {
+        boolean success = false;
         synchronized (mNativePtrLock) {
-            stop();
-            if (registerForSensors(rateInMilliseconds)) {
+            switch (eventType) {
+                case DEVICE_ORIENTATION:
+                    success = registerSensors(DEVICE_ORIENTATION_SENSORS, rateInMilliseconds,
+                            true);
+                    break;
+                case DEVICE_MOTION:
+                    // note: device motion spec does not require all sensors to be available
+                    success = registerSensors(DEVICE_MOTION_SENSORS, rateInMilliseconds, false);
+                    break;
+                default:
+                    Log.e(LOGTAG, "Unknown event type: " + eventType);
+                    return false;
+            }
+            if (success) {
                 mNativePtr = nativePtr;
-                return true;
+                setActiveEventType(eventType, true);
             }
-            return false;
+            return success;
         }
     }
 
     /**
-     * Stop listening for sensor events. Always succeeds.
+     * Stop listening to sensors for a given event type. Ensures that sensors are not disabled
+     * if they are still in use by a different event type.
      *
-     * We strictly guarantee that nativeGotOrientation() will not be called
+     * @param eventType Type of event to listen to, can be either DEVICE_ORIENTATION or
+     *                  DEVICE_MOTION.
+     * We strictly guarantee that the corresponding native*() methods will not be called
      * after this method returns.
      */
     @CalledByNative
-    public void stop() {
+    public void stop(int eventType) {
+        Set<Integer> sensorsToRemainActive = Sets.newHashSet();
         synchronized (mNativePtrLock) {
-            if (mNativePtr != 0) {
-                mNativePtr = 0;
-                unregisterForSensors();
+            switch (eventType) {
+                case DEVICE_ORIENTATION:
+                    if (mDeviceMotionIsActive) {
+                        sensorsToRemainActive.addAll(DEVICE_MOTION_SENSORS);
+                    }
+                    break;
+                case DEVICE_MOTION:
+                    if (mDeviceOrientationIsActive) {
+                        sensorsToRemainActive.addAll(DEVICE_ORIENTATION_SENSORS);
+                    }
+                    break;
+                default:
+                    Log.e(LOGTAG, "Unknown event type: " + eventType);
+                    return;
             }
+
+            Set<Integer> sensorsToDeactivate = Sets.newHashSet(mActiveSensors);
+            sensorsToDeactivate.removeAll(sensorsToRemainActive);
+            unregisterSensors(sensorsToDeactivate);
+            mNativePtr = 0;

[bulach, 2013/03/19 15:22:36]

shouldn't the native ptr only go away if there are no more active sensors?

[timvolodine, 2013/03/19 16:50:08]

Done.

+            setActiveEventType(eventType, false);
         }
     }
 
     @Override
     public void onAccuracyChanged(Sensor sensor, int accuracy) {
         // Nothing
     }
 
     @Override
     public void onSensorChanged(SensorEvent event) {
-        switch (event.sensor.getType()) {
+        sensorChanged(event.sensor.getType(), event.values);
+    }
+
+    @VisibleForTesting
+    void sensorChanged(int type, float[] values) {
+
+        switch (type) {
             case Sensor.TYPE_ACCELEROMETER:
-                if (mGravityVector == null) {
-                    mGravityVector = new float[3];
+                if (mAccelerationVector == null) {
+                    mAccelerationVector = new float[3];
                 }
-                System.arraycopy(event.values, 0, mGravityVector, 0, mGravityVector.length);
+                System.arraycopy(values, 0, mAccelerationVector, 0,
+                        mAccelerationVector.length);
+                if (mDeviceMotionIsActive) {
+                    gotAccelerationIncludingGravity(mAccelerationVector[0], mAccelerationVector[1],
+                        mAccelerationVector[2]);
+                }
                 break;
-
+            case Sensor.TYPE_LINEAR_ACCELERATION:
+                if (mDeviceMotionIsActive) {
+                    gotAcceleration(values[0], values[1], values[2]);
+                }
+                break;
+            case Sensor.TYPE_GYROSCOPE:
+                if (mDeviceMotionIsActive) {
+                    gotRotationRate(values[0], values[1], values[2]);
+                }
+                break;
             case Sensor.TYPE_MAGNETIC_FIELD:
                 if (mMagneticFieldVector == null) {
                     mMagneticFieldVector = new float[3];
                 }
-                System.arraycopy(event.values, 0, mMagneticFieldVector, 0,
-                                 mMagneticFieldVector.length);
+                System.arraycopy(values, 0, mMagneticFieldVector, 0,
+                        mMagneticFieldVector.length);
                 break;
-
             default:
                 // Unexpected
                 return;
         }
 
-        getOrientationUsingGetRotationMatrix();
+        if (mDeviceOrientationIsActive) {
+            getOrientationUsingGetRotationMatrix();
+        }
     }
 
-    void getOrientationUsingGetRotationMatrix() {
-        if (mGravityVector == null || mMagneticFieldVector == null) {
+    private void getOrientationUsingGetRotationMatrix() {
+        if (mAccelerationVector == null || mMagneticFieldVector == null) {
             return;
         }
 
         // Get the rotation matrix.
         // The rotation matrix that transforms from the body frame to the earth
         // frame.
         float[] deviceRotationMatrix = new float[9];
-        if (!SensorManager.getRotationMatrix(deviceRotationMatrix, null, mGravityVector,
+        if (!SensorManager.getRotationMatrix(deviceRotationMatrix, null, mAccelerationVector,
                 mMagneticFieldVector)) {
             return;
         }
 
         // Convert rotation matrix to rotation angles.
         // Assuming that the rotations are appied in the order listed at
         // http://developer.android.com/reference/android/hardware/SensorEvent.html#values
         // the rotations are applied about the same axes and in the same order as required by the
         // API. The only conversions are sign changes as follows.  The angles are in radians
 
@@ skipping 11 matching lines @@
         }
 
         double gamma = Math.toDegrees(rotationAngles[2]);
         while (gamma < -90.0) {
             gamma += 360.0; // [-90, 90)
         }
 
         gotOrientation(alpha, beta, gamma);
     }
 
-    boolean registerForSensors(int rateInMilliseconds) {
-        if (registerForSensorType(Sensor.TYPE_ACCELEROMETER, rateInMilliseconds)
-                && registerForSensorType(Sensor.TYPE_MAGNETIC_FIELD, rateInMilliseconds)) {
-            return true;
+    private SensorManagerProxy getSensorManagerProxy() {
+        if (mSensorManagerProxy != null) {
+            return mSensorManagerProxy;
         }
-        unregisterForSensors();
-        return false;
+        mSensorManagerProxy = new SensorManagerProxyImpl();
+        return mSensorManagerProxy;
     }
 
-    private SensorManager getSensorManager() {
-        if (mSensorManager != null) {
-            return mSensorManager;
-        }
-        mSensorManager = (SensorManager)WeakContext.getSystemService(Context.SENSOR_SERVICE);
-        return mSensorManager;
+    @VisibleForTesting
+    void setSensorManagerProxy(SensorManagerProxy sensorManager) {
+        mSensorManagerProxy = sensorManager;
     }
 
-    void unregisterForSensors() {
-        SensorManager sensorManager = getSensorManager();
-        if (sensorManager == null) {
-            return;
+    private void setActiveEventType(int eventType, boolean value) {

[bulach, 2013/03/19 15:22:36]

nit: this would read better as "setEventTypeActive", otherwise it seems that
only one can be active at a time.

[timvolodine, 2013/03/19 16:50:08]

Done.

+        switch (eventType) {
+            case DEVICE_ORIENTATION:
+                mDeviceOrientationIsActive = value;
+                return;
+            case DEVICE_MOTION:
+                mDeviceMotionIsActive = value;
+                return;
         }
-        sensorManager.unregisterListener(this);
+    }
+
+    /**
+     * @param sensorTypes List of sensors to activate.
+     * @param rateInMilliseconds Intented delay (in milliseconds) between sensor readings.
+     * @param failOnMissingSensor If true the method returns true only if all sensors could be
+     *                            activated. When false the method return true if at least one
+     *                            sensor in sensorTypes could be activated.
+     */
+    private boolean registerSensors(Iterable<Integer> sensorTypes, int rateInMilliseconds,
+                                       boolean failOnMissingSensor) {

[bulach, 2013/03/19 15:22:36]

nit: alignment

[timvolodine, 2013/03/19 16:50:08]

Done.

+        Set<Integer> sensorsToActivate = Sets.newHashSet(sensorTypes);
+        sensorsToActivate.removeAll(mActiveSensors);
+        boolean success = false;
+
+        for (Integer sensorType : sensorsToActivate) {
+            boolean result = registerForSensorType(sensorType, rateInMilliseconds);
+            if (!result && failOnMissingSensor) {
+                // restore the previous state upon failure
+                unregisterSensors(sensorsToActivate);
+                return false;
+            }
+            if (result) {
+                mActiveSensors.add(sensorType);
+                success = true;
+            }
+        }
+        return success;
+    }
+
+    private void unregisterSensors(Iterable<Integer> sensorTypes) {
+        for (Integer sensorType : sensorTypes) {
+            if (mActiveSensors.contains(sensorType)) {
+                List<Sensor> sensors = getSensorManagerProxy().getSensorList(sensorType);

[bulach, 2013/03/19 15:22:36]

all usages of getSensorList() check for empty and then use sensors.get(0)..
how about just making it "Sensor getSensor()" then return null if it's empty and
use it directly otherwise?
or even better: it seems that this is used only to then pass it again to
un/registerListener... so how about pushing all this logic to SensorManagerProxy
itself, make "getSensorList" an internal method on the impl, and let the
un/registerListener take a Integer type rather than a Sensor?

[timvolodine, 2013/03/19 16:50:08]

nice point, simplifies the code indeed -- done!

On 2013/03/19 15:22:36, bulach wrote:

+                if (!sensors.isEmpty()) {
+                    getSensorManagerProxy().unregisterListener(this, sensors.get(0));
+                    mActiveSensors.remove(sensorType);
+                }
+            }
+        }
     }
 
     boolean registerForSensorType(int type, int rateInMilliseconds) {
-        SensorManager sensorManager = getSensorManager();
+        SensorManagerProxy sensorManager = getSensorManagerProxy();
         if (sensorManager == null) {
             return false;
         }
         List<Sensor> sensors = sensorManager.getSensorList(type);
         if (sensors.isEmpty()) {
             return false;
         }
 
         final int rateInMicroseconds = 1000 * rateInMilliseconds;
-        // We want to err on the side of getting more events than we need.
-        final int requestedRate = rateInMicroseconds / 2;
-
-        // TODO(steveblock): Consider handling multiple sensors.
-        return sensorManager.registerListener(this, sensors.get(0), requestedRate, getHandler());
+        return sensorManager.registerListener(this, sensors.get(0), rateInMicroseconds,
+                getHandler());
     }
 
-    void gotOrientation(double alpha, double beta, double gamma) {
+    protected void gotOrientation(double alpha, double beta, double gamma) {
         synchronized (mNativePtrLock) {
             if (mNativePtr != 0) {
               nativeGotOrientation(mNativePtr, alpha, beta, gamma);
             }
         }
     }
 
+    protected void gotAcceleration(double x, double y, double z) {
+        synchronized (mNativePtrLock) {
+            if (mNativePtr != 0) {
+              nativeGotAcceleration(mNativePtr, x, y, z);
+            }
+        }
+    }
+
+    protected void gotAccelerationIncludingGravity(double x, double y, double z) {
+        synchronized (mNativePtrLock) {
+            if (mNativePtr != 0) {
+              nativeGotAccelerationIncludingGravity(mNativePtr, x, y, z);
+            }
+        }
+    }
+
+    protected void gotRotationRate(double alpha, double beta, double gamma) {
+        synchronized (mNativePtrLock) {
+            if (mNativePtr != 0) {
+              nativeGotRotationRate(mNativePtr, alpha, beta, gamma);
+            }
+        }
+    }
+
     private Handler getHandler() {
         synchronized (mHandlerLock) {
             // If we don't have a background thread, start it now.
             if (mThread == null) {
                 mThread = new Thread(new Runnable() {
                     @Override
                     public void run() {
                         Looper.prepare();
                         // Our Handler doesn't actually have to do anything, because
                         // SensorManager posts directly to the underlying Looper.
@@ skipping 17 matching lines @@
     }
 
     private void setHandler(Handler handler) {
         synchronized (mHandlerLock) {
             mHandler = handler;
             mHandlerLock.notify();
         }
     }
 
     @CalledByNative
-    private static DeviceOrientation getInstance() {
+    static DeviceMotionAndOrientation getInstance() {
         synchronized (sSingletonLock) {
             if (sSingleton == null) {
-                sSingleton = new DeviceOrientation();
+                sSingleton = new DeviceMotionAndOrientation();
             }
             return sSingleton;
         }
     }
 
     /**
-     * See chrome/browser/device_orientation/data_fetcher_impl_android.cc
+     * Native JNI calls,
+     * see content/browser/device_orientation/data_fetcher_impl_android.cc
+     */
+
+    /**
+     * Orientation of the device with respect to its reference frame.
      */
     private native void nativeGotOrientation(
             int nativeDataFetcherImplAndroid,
             double alpha, double beta, double gamma);
-}
+
+    /**
+     * Linear acceleration without gravity of the device with respect to its body frame.
+     */
+    private native void nativeGotAcceleration(
+            int nativeDataFetcherImplAndroid,
+            double x, double y, double z);
+
+    /**
+     * Acceleration including gravity of the device with respect to its body frame.
+     */
+    private native void nativeGotAccelerationIncludingGravity(
+            int nativeDataFetcherImplAndroid,
+            double x, double y, double z);
+
+    /**
+     * Rotation rate of the device with respect to its body frame.
+     */
+    private native void nativeGotRotationRate(
+            int nativeDataFetcherImplAndroid,
+            double alpha, double beta, double gamma);
+
+    /**
+     * Need the an interface for SensorManager for testing.
+     */
+    interface SensorManagerProxy {
+        public List<Sensor> getSensorList(int type);

[bulach, 2013/03/19 15:22:36]

see above, I think this can be moved out of the interface and as a private
method in SensorManagerProxyImpl if the un/register methods take an "int type"
rather than a Sensor..

[timvolodine, 2013/03/19 16:50:08]

Done.

+        public boolean registerListener(SensorEventListener listener, Sensor sensor, int rate,
+                Handler handler);
+        public void unregisterListener(SensorEventListener listener, Sensor sensor);
+    }
+
+    static class SensorManagerProxyImpl implements SensorManagerProxy {
+        private SensorManager mSensorManager;
+
+        SensorManagerProxyImpl() {
+            mSensorManager = (SensorManager)WeakContext.getSystemService(Context.SENSOR_SERVICE);
+        }
+
+        public List<Sensor> getSensorList(int type) {
+            return mSensorManager.getSensorList(type);
+        }
+
+        public boolean registerListener(SensorEventListener listener, Sensor sensor, int rate,
+                Handler handler) {
+            return mSensorManager.registerListener(listener, sensor, rate, handler);
+        }
+
+        public void unregisterListener(SensorEventListener listener, Sensor sensor) {
+            mSensorManager.unregisterListener(listener, sensor);
+        }
+    }
+
+}