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

content/browser/device_orientation/data_fetcher_impl_android.cc

Index: content/browser/device_orientation/data_fetcher_impl_android.cc
diff --git a/content/browser/device_orientation/data_fetcher_impl_android.cc b/content/browser/device_orientation/data_fetcher_impl_android.cc
index 287b63a1db9c36a59db6164419d3faaa4567c9e7..a7f517fba1437d3515d29ac7209184b3ba48dbd3 100644
--- a/content/browser/device_orientation/data_fetcher_impl_android.cc
+++ b/content/browser/device_orientation/data_fetcher_impl_android.cc
@@ -7,7 +7,7 @@
 #include "base/android/jni_android.h"
 #include "base/logging.h"
 #include "content/browser/device_orientation/orientation.h"
-#include "jni/DeviceOrientation_jni.h"
+#include "jni/DeviceMotionAndOrientation_jni.h"

[Peter Beverloo, 2013/03/11 17:57:25]

What's happening to the DeviceOrientation.java file?  You added a new file, but
didn't remove this file.

[Miguel Garcia, 2013/03/11 18:25:50]

yes plese mv oldName newName, that way you remove it and as a nice side effects
the right diffs will show up in retrieveid making it way easier to review.

On 2013/03/11 17:57:25, Peter Beverloo wrote:

[timvolodine1, 2013/03/12 11:18:06]

Done.

[timvolodine1, 2013/03/12 11:18:06]

Done.

 
 using base::android::AttachCurrentThread;
 
@@ -19,11 +19,18 @@ namespace {
 // TODO(husky): Make that constant public so we can use it directly.
 const int kPeriodInMilliseconds = 100;
 
+// constants used for JNI calls to java

[Peter Beverloo, 2013/03/11 17:57:25]

Please use proper sentences and grammar.  As previously commented, I'd prefer to
expose this in a header given that external callers will need it.  It's not
platform specific either, so it should (eventually?) end up in the service's
file/class.

[timvolodine1, 2013/03/12 11:18:06]

actually reusing the DeviceData::Type enum now to reduce code duplication

On 2013/03/11 17:57:25, Peter Beverloo wrote:

+// see DeviceMotionAndOrientation.java
+enum DeviceMotionOrientationEvents {

[Miguel Garcia, 2013/03/11 18:25:50]

This enum should probably be public, you've worked around this by using an int
in the Start/Stop methods but that is not very elegant.

[timvolodine1, 2013/03/12 11:18:06]

Done.

+  DEVICE_ORIENTATION = 0,
+  DEVICE_MOTION = 1
+};
+
 }  // namespace
 
 DataFetcherImplAndroid::DataFetcherImplAndroid() {
   device_orientation_.Reset(
-      Java_DeviceOrientation_getInstance(AttachCurrentThread()));
+      Java_DeviceMotionAndOrientation_getInstance(AttachCurrentThread()));
 }
 
 void DataFetcherImplAndroid::Init(JNIEnv* env) {
@@ -76,17 +83,47 @@ void DataFetcherImplAndroid::GotOrientation(
   next_orientation_ = orientation;
 }
 
+void DataFetcherImplAndroid::GotAcceleration(
+    JNIEnv*, jobject, double x, double y, double z) {
+  // TODO: copy into shared memory buffer

[Peter Beverloo, 2013/03/11 17:57:25]

This doesn't add a lot.  Since it shouldn't be called yet, just add
NOTIMPLEMENTED()?

[Miguel Garcia, 2013/03/11 18:25:50]

Please add an owner to the TODO. I think it should at least log something.
Alternatively you could provide a mock implementation and use it in tests.

[timvolodine1, 2013/03/12 11:18:06]

Done.

[timvolodine1, 2013/03/12 11:18:06]

Done.

+}
+
+void DataFetcherImplAndroid::GotAccelerationIncludingGravity(
+    JNIEnv*, jobject, double x, double y, double z) {
+}

[Peter Beverloo, 2013/03/11 17:57:25]

Dito to line 88, please add the NOTIMPLEMENTED() everywhere for clarity.

[timvolodine1, 2013/03/12 11:18:06]

Done.

+
+void DataFetcherImplAndroid::GotRotationRate(
+    JNIEnv*, jobject, double alpha, double beta, double gamma) {
+}
+
+// TODO: remove this method later, this is for compatibility now

[Peter Beverloo, 2013/03/11 17:57:25]

nit: "// TODO: Remove this method when all callers supply the required event
type."?

[timvolodine1, 2013/03/12 11:18:06]

Done.

 bool DataFetcherImplAndroid::Start(int rate_in_milliseconds) {
+  return Start(DEVICE_ORIENTATION, rate_in_milliseconds);
+  //return Start(DEVICE_MOTION, rate_in_milliseconds);

[Peter Beverloo, 2013/03/11 17:57:25]

nit: I'd prefer not to commit commented out code.

[Miguel Garcia, 2013/03/11 18:25:50]

+1
On 2013/03/11 17:57:25, Peter Beverloo wrote:

[timvolodine1, 2013/03/12 11:18:06]

Done.

[timvolodine1, 2013/03/12 11:18:06]

Done.

+}
+
+bool DataFetcherImplAndroid::Start(int spec_event_type,

[Miguel Garcia, 2013/03/11 18:25:50]

It does not really feel we need both Start methods, in fact it is going to be
confusing. Can't you just change the callers?

If you cannot afford to do that because there are two many please add a TODO and
do it in a separate CL.

[timvolodine1, 2013/03/12 11:18:06]

Done.

+    int rate_in_milliseconds) {
   DCHECK(!device_orientation_.is_null());
-  return Java_DeviceOrientation_start(AttachCurrentThread(),
+  return Java_DeviceMotionAndOrientation_start(AttachCurrentThread(),
                                       device_orientation_.obj(),
                                       reinterpret_cast<jint>(this),
+                                      spec_event_type,
                                       rate_in_milliseconds);
 }
 
-void DataFetcherImplAndroid::Stop() {
+// TODO: possibly remove this method, or make it stop everything

[Peter Beverloo, 2013/03/11 17:57:25]

In which case would we want to do that?

[Miguel Garcia, 2013/03/11 18:25:50]

I'd remove it.

[timvolodine1, 2013/03/12 11:18:06]

Done.

[timvolodine1, 2013/03/12 11:18:06]

potentially in the destructor..

On 2013/03/11 17:57:25, Peter Beverloo wrote:

+// (e.g. both motion and orientation)
+void DataFetcherImplAndroid::Stop(){
+  Stop(DEVICE_ORIENTATION);
+  //Stop(DEVICE_MOTION);

[Miguel Garcia, 2013/03/11 18:25:50]

commented line...

[timvolodine1, 2013/03/12 11:18:06]

Done.

+}
+
+void DataFetcherImplAndroid::Stop(int spec_event_type) {
   DCHECK(!device_orientation_.is_null());
-  Java_DeviceOrientation_stop(AttachCurrentThread(), device_orientation_.obj());
+  Java_DeviceMotionAndOrientation_stop(AttachCurrentThread(),
+                               device_orientation_.obj(),
+                               spec_event_type);
 }
 
 }  // namespace content