[DeviceService] Add service tests for VibrationManager.

device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java

Index: device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java
diff --git a/device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java b/device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java
index 98c24ec1501c13b7269b380224e192465e3a260b..e781f71df9e193761e93209c435258d5fbbf2b0f 100644
--- a/device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java
+++ b/device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java
@@ -10,7 +10,8 @@ import android.media.AudioManager;
 import android.os.Vibrator;
 import android.util.Log;
 
-import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
 import org.chromium.device.mojom.VibrationManager;
 import org.chromium.mojo.system.MojoException;
 import org.chromium.services.service_manager.InterfaceFactory;
@@ -19,6 +20,7 @@ import org.chromium.services.service_manager.InterfaceFactory;
  * Android implementation of the vibration manager service defined in
  * device/vibration/vibration_manager.mojom.
  */
+@JNINamespace("device")
 public class VibrationManagerImpl implements VibrationManager {
     private static final String TAG = "VibrationManagerImpl";
 
@@ -29,35 +31,12 @@ public class VibrationManagerImpl implements VibrationManager {
     private final Vibrator mVibrator;
     private final boolean mHasVibratePermission;
 
-    private static AndroidVibratorWrapper sVibratorWrapper;
-
-    /**
-     * Android Vibrator wrapper class provided to test code to extend.
-     */
-    @VisibleForTesting
-    public static class AndroidVibratorWrapper {
-        protected AndroidVibratorWrapper() {}
-
-        public void vibrate(Vibrator vibrator, long milliseconds) {
-            vibrator.vibrate(milliseconds);
-        }
-
-        public void cancel(Vibrator vibrator) {
-            vibrator.cancel();
-        }
-    }
-
-    // Test code can use this function to inject other wrapper for testing.
-    public static void setVibratorWrapperForTesting(AndroidVibratorWrapper wrapper) {
-        sVibratorWrapper = wrapper;
-    }
+    private static long sVibrateMilliSecondsForTesting = -1;
+    private static boolean sVibrateCancelledForTesting = false;

[timvolodine, 2017/04/05 17:16:28]

Is there any particular reason for eliminating the Wrapper here? Exposing
testable primitive values / static methods (as below) in production code here
and in other places does not seem very clean and scalable..

[leonhsl(Using Gerrit), 2017/04/06 03:19:16]

We were creating/setting this AndroidVibratorWrapper from Java testing code
before, but now we're in C++ testing code, seems it's not so easy to do that
like before, so I chose such a way considering that the 2 static testing
variables are so simple and with small size..

 
     public VibrationManagerImpl(Context context) {
         mAudioManager = (AudioManager) context.getSystemService(Context.AUDIO_SERVICE);
         mVibrator = (Vibrator) context.getSystemService(Context.VIBRATOR_SERVICE);
-        if (sVibratorWrapper == null) {
-            sVibratorWrapper = new AndroidVibratorWrapper();
-        }
         // TODO(mvanouwerkerk): What happens if permission is revoked? Handle this better.
         mHasVibratePermission =
                 context.checkCallingOrSelfPermission(android.Manifest.permission.VIBRATE)
@@ -82,14 +61,18 @@ public class VibrationManagerImpl implements VibrationManager {
 
         if (mAudioManager.getRingerMode() != AudioManager.RINGER_MODE_SILENT
                 && mHasVibratePermission) {
-            sVibratorWrapper.vibrate(mVibrator, sanitizedMilliseconds);
+            mVibrator.vibrate(sanitizedMilliseconds);

[timvolodine, 2017/04/07 10:39:07]

Will the device actually vibrate when testing?

[leonhsl(Using Gerrit), 2017/04/08 00:51:01]

Nope :) The test app service_unittests has no Vibration permission. I think we
just want to verify VibrationManager mojo interface impl inside Device Service
so I did not add that permission additionally.

         }
+        setVibrateMilliSecondsForTesting(sanitizedMilliseconds);

[timvolodine, 2017/04/07 10:39:07]

I believe we generally try to avoid mixing production and explicit testing code.

[leonhsl(Using Gerrit), 2017/04/08 00:51:01]

Yeah.. Having no better idea to handle such case though..

         callback.call();
     }
 
     @Override
     public void cancel(CancelResponse callback) {
-        if (mHasVibratePermission) sVibratorWrapper.cancel(mVibrator);
+        if (mHasVibratePermission) {
+            mVibrator.cancel();
+        }
+        setVibrateCancelledForTesting(true);
         callback.call();
     }
 
@@ -107,4 +90,22 @@ public class VibrationManagerImpl implements VibrationManager {
             return new VibrationManagerImpl(mContext);
         }
     }
+
+    static void setVibrateMilliSecondsForTesting(long milliseconds) {
+        sVibrateMilliSecondsForTesting = milliseconds;
+    }
+
+    static void setVibrateCancelledForTesting(boolean cancelled) {
+        sVibrateCancelledForTesting = cancelled;
+    }
+
+    @CalledByNative
+    static long getVibrateMilliSecondsForTesting() {
+        return sVibrateMilliSecondsForTesting;
+    }
+
+    @CalledByNative
+    static boolean getVibrateCancelledForTesting() {
+        return sVibrateCancelledForTesting;
+    }
 }