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

Index: content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/DeviceOrientation.java b/content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java
similarity index 36%
rename from content/public/android/java/src/org/chromium/content/browser/DeviceOrientation.java
rename to content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java
index a77cd28b27ab6901f6a12f7a2e4e012b3e7c8f71..24e847c942fad5d6db2d81cd3ad6752c7da9a6da 100644
--- a/content/public/android/java/src/org/chromium/content/browser/DeviceOrientation.java
+++ b/content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java
@@ -1,4 +1,4 @@
-// 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.
 
@@ -11,18 +11,26 @@ 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;
@@ -38,20 +46,41 @@ class DeviceOrientation implements SensorEventListener {
     // 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() {
     }
 
     /**
@@ -61,33 +90,69 @@ class DeviceOrientation implements SensorEventListener {
      * @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);
         }
     }
 
@@ -98,32 +163,53 @@ class DeviceOrientation implements SensorEventListener {
 
     @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(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]);
                 }
-                System.arraycopy(event.values, 0, mGravityVector, 0, mGravityVector.length);
                 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;
         }
 
@@ -131,7 +217,7 @@ class DeviceOrientation implements SensorEventListener {
         // 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;
         }
@@ -163,33 +249,72 @@ class DeviceOrientation implements SensorEventListener {
         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;
+    }
+
+    @VisibleForTesting
+    void setSensorManagerProxy(SensorManagerProxy sensorManager) {
+        mSensorManagerProxy = sensorManager;
     }
 
-    private SensorManager getSensorManager() {
-        if (mSensorManager != null) {
-            return mSensorManager;
+    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;
         }
-        mSensorManager = (SensorManager)WeakContext.getSystemService(Context.SENSOR_SERVICE);
-        return mSensorManager;
     }
 
-    void unregisterForSensors() {
-        SensorManager sensorManager = getSensorManager();
-        if (sensorManager == null) {
-            return;
+    /**
+     * @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);
+                }
+            }
         }
-        sensorManager.unregisterListener(this);
     }
 
     boolean registerForSensorType(int type, int rateInMilliseconds) {
-        SensorManager sensorManager = getSensorManager();
+        SensorManagerProxy sensorManager = getSensorManagerProxy();
         if (sensorManager == null) {
             return false;
         }
@@ -199,14 +324,11 @@ class DeviceOrientation implements SensorEventListener {
         }
 
         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);
@@ -214,6 +336,30 @@ class DeviceOrientation implements SensorEventListener {
         }
     }
 
+    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.
@@ -251,19 +397,77 @@ class DeviceOrientation implements SensorEventListener {
     }
 
     @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);
+        }
+    }
+
+}