Skip to content

Commit

Permalink
Revert of [DeviceService] Add service tests for VibrationManager. (pa…
Browse files Browse the repository at this point in the history
…tchset #7 id:410001 of https://codereview.chromium.org/2774783003/ )

Reason for revert:
Appears to have broken service_unittests on Nexus 5X.

Original issue's description:
> [DeviceService] Add service tests for VibrationManager.
>
> VibrationManagerImpl.java is in //device/vibration while the
> VibrationManagerImplTest.java is still in //content,
> to be able to hide all //device/vibration impl inside Device Service,
> we must eliminate this concrete dependency on //device/vibration from
> content layer.
>
> As VibrationManagerImplTest.java is an integration test based on
> content shell test infra, we can't move it outside of //content.
> So we consider to use other tests to replace it, while the renderer-side
> testing has already been covered by layout tests created at
> https://codereview.chromium.org/2731953003/, so we create this CL to
> cover the VibrationManager interface impl testing:
>  - creates an infra for Device Service service tests.
>  - adds a new service test for VibrationManager interface.
>
> Then after this CL we'll be able to hide all vibration impl inside
> Device Service with a follow-up CL.
>
> BUG=689379
> TEST=service_unittests
>
> Review-Url: https://codereview.chromium.org/2774783003
> Cr-Commit-Position: refs/heads/master@{#463612}
> Committed: https://chromium.googlesource.com/chromium/src/+/9c17bee62f1e16224fabb7c83bbeaab719015761

TBR=rockot@chromium.org,avi@chromium.org,blundell@chromium.org,jam@chromium.org,timvolodine@chromium.org,leon.han@intel.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=689379

Review-Url: https://codereview.chromium.org/2815623003
Cr-Commit-Position: refs/heads/master@{#463629}
  • Loading branch information
rsolomakhin authored and Commit bot committed Apr 11, 2017
1 parent 4d28782 commit 1a5a2cd
Show file tree
Hide file tree
Showing 14 changed files with 166 additions and 335 deletions.
3 changes: 3 additions & 0 deletions content/public/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ android_library("content_javatests") {
"//content/shell/android:content_shell_test_java",
"//device/geolocation:geolocation_java",
"//device/geolocation:geolocation_java_test_support",
"//device/vibration:mojo_bindings_java",
"//device/vibration/android:vibration_manager_java",
"//media/base/android:media_java",
"//media/capture/content/android:screen_capture_java",
"//media/capture/video/android:capture_java",
Expand Down Expand Up @@ -434,6 +436,7 @@ android_library("content_javatests") {
"javatests/src/org/chromium/content/browser/VideoFullscreenOrientationLockTest.java",
"javatests/src/org/chromium/content/browser/VSyncMonitorTest.java",
"javatests/src/org/chromium/content/browser/VSyncPausedTest.java",
"javatests/src/org/chromium/content/browser/VibrationManagerImplTest.java",
"javatests/src/org/chromium/content/browser/ViewportTest.java",
"javatests/src/org/chromium/content/browser/WebContentsObserverAndroidTest.java",
"javatests/src/org/chromium/content/browser/accessibility/captioning/CaptioningChangeDelegateTest.java",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// Copyright 2015 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.os.Vibrator;
import android.support.test.filters.MediumTest;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.UrlUtils;
import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper;
import org.chromium.content_shell_apk.ContentShellActivityTestRule;
import org.chromium.device.vibration.VibrationManagerImpl;

/**
* Tests java implementation of VibrationManager mojo service on android.
*/
@RunWith(BaseJUnit4ClassRunner.class)
public class VibrationManagerImplTest {
@Rule
public ContentShellActivityTestRule mActivityTestRule = new ContentShellActivityTestRule();

private static final String URL_VIBRATOR_VIBRATE = UrlUtils.encodeHtmlDataUri("<html><body>"
+ " <script type=\"text/javascript\">"
+ " navigator.vibrate(3000);"
+ " </script>"
+ "</body></html>");
private static final String URL_VIBRATOR_CANCEL = UrlUtils.encodeHtmlDataUri("<html><body>"
+ " <script type=\"text/javascript\">"
+ " navigator.vibrate(10000);"
+ " navigator.vibrate(0);"
+ " </script>"
+ "</body></html>");

private FakeAndroidVibratorWrapper mFakeWrapper;

// Override AndroidVibratorWrapper API to record the calling.
private static class FakeAndroidVibratorWrapper
extends VibrationManagerImpl.AndroidVibratorWrapper {
// Record the parameters of vibrate() and cancel().
public long mMilliSeconds;
public boolean mCancelled;

protected FakeAndroidVibratorWrapper() {
super();
mMilliSeconds = -1;
mCancelled = false;
}

@Override
public void vibrate(Vibrator vibrator, long milliseconds) {
mMilliSeconds = milliseconds;
}

@Override
public void cancel(Vibrator vibrator) {
mCancelled = true;
}
}

@Before
public void setUp() throws Exception {
mActivityTestRule.launchContentShellWithUrl("about:blank");
mActivityTestRule.waitForActiveShellToBeDoneLoading();

mFakeWrapper = new FakeAndroidVibratorWrapper();
VibrationManagerImpl.setVibratorWrapperForTesting(mFakeWrapper);
Assert.assertEquals(-1, mFakeWrapper.mMilliSeconds);
Assert.assertFalse(mFakeWrapper.mCancelled);
}

/**
* Inject our fake wrapper into VibrationManagerImpl,
* load the webpage which will request vibrate for 3000 milliseconds,
* the fake wrapper vibrate() should be called and 3000 milliseconds should be recorded
* correctly.
*/
// @MediumTest
// @Feature({"Vibration"})
@Test
@DisabledTest
public void testVibrate() throws Throwable {
mActivityTestRule.loadNewShell(URL_VIBRATOR_VIBRATE);

// Waits until VibrationManagerImpl.Vibrate() got called.
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
return mFakeWrapper.mMilliSeconds != -1;
}
});

Assert.assertEquals(
"Did not get vibrate mMilliSeconds correctly", 3000, mFakeWrapper.mMilliSeconds);
}

/**
* Inject our fake wrapper into VibrationManagerImpl,
* load the webpage which will request vibrate and then request cancel,
* the fake wrapper cancel() should be called.
*/
@Test
@MediumTest
@Feature({"Vibration"})
public void testCancel() throws Throwable {
mActivityTestRule.loadNewShell(URL_VIBRATOR_CANCEL);

// Waits until VibrationManagerImpl.Cancel() got called.
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
return mFakeWrapper.mCancelled;
}
});

Assert.assertTrue("Did not get cancelled", mFakeWrapper.mCancelled);
}
}
7 changes: 0 additions & 7 deletions device/vibration/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@
import("//build/config/android/config.gni")
import("//build/config/android/rules.gni")

generate_jni("vibration_jni_headers") {
sources = [
"java/src/org/chromium/device/vibration/VibrationManagerImpl.java",
]
jni_package = "vibration"
}

android_library("vibration_manager_java") {
java_files =
[ "java/src/org/chromium/device/vibration/VibrationManagerImpl.java" ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
import android.os.Vibrator;
import android.util.Log;

import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.VisibleForTesting;
import org.chromium.device.mojom.VibrationManager;
import org.chromium.mojo.system.MojoException;
import org.chromium.services.service_manager.InterfaceFactory;
Expand All @@ -20,7 +19,6 @@
* 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";

Expand All @@ -31,12 +29,35 @@ public class VibrationManagerImpl implements VibrationManager {
private final Vibrator mVibrator;
private final boolean mHasVibratePermission;

private static long sVibrateMilliSecondsForTesting = -1;
private static boolean sVibrateCancelledForTesting = false;
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;
}

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)
Expand All @@ -61,18 +82,14 @@ public void vibrate(long milliseconds, VibrateResponse callback) {

if (mAudioManager.getRingerMode() != AudioManager.RINGER_MODE_SILENT
&& mHasVibratePermission) {
mVibrator.vibrate(sanitizedMilliseconds);
sVibratorWrapper.vibrate(mVibrator, sanitizedMilliseconds);
}
setVibrateMilliSecondsForTesting(sanitizedMilliseconds);
callback.call();
}

@Override
public void cancel(CancelResponse callback) {
if (mHasVibratePermission) {
mVibrator.cancel();
}
setVibrateCancelledForTesting(true);
if (mHasVibratePermission) sVibratorWrapper.cancel(mVibrator);
callback.call();
}

Expand All @@ -90,22 +107,4 @@ public VibrationManager createImpl() {
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;
}
}
3 changes: 0 additions & 3 deletions device/vibration/vibration_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ class VibrationManagerImpl {
public:
DEVICE_VIBRATION_EXPORT static void Create(
mojo::InterfaceRequest<mojom::VibrationManager> request);

DEVICE_VIBRATION_EXPORT static int64_t milli_seconds_for_testing_;
DEVICE_VIBRATION_EXPORT static bool cancelled_for_testing_;
};

} // namespace device
Expand Down
9 changes: 1 addition & 8 deletions device/vibration/vibration_manager_impl_default.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@

namespace device {

int64_t VibrationManagerImpl::milli_seconds_for_testing_ = -1;
bool VibrationManagerImpl::cancelled_for_testing_ = false;

namespace {

class VibrationManagerEmptyImpl : public mojom::VibrationManager {
Expand All @@ -22,14 +19,10 @@ class VibrationManagerEmptyImpl : public mojom::VibrationManager {
~VibrationManagerEmptyImpl() override {}

void Vibrate(int64_t milliseconds, const VibrateCallback& callback) override {
VibrationManagerImpl::milli_seconds_for_testing_ = milliseconds;
callback.Run();
}

void Cancel(const CancelCallback& callback) override {
VibrationManagerImpl::cancelled_for_testing_ = true;
callback.Run();
}
void Cancel(const CancelCallback& callback) override { callback.Run(); }
};

} // namespace
Expand Down
7 changes: 2 additions & 5 deletions services/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ service_test("service_unittests") {
]

if (is_android) {
deps += [
# Some tests need to initialize V8.
"//v8:v8_external_startup_data_assets",
]
# Some tests need to initialize V8.
deps += [ "//v8:v8_external_startup_data_assets" ]
} else {
# NOTE: We do not currently support standalone service binaries on Android,
# so any tests which use the ServiceTest framework to connect to standalone
Expand All @@ -40,7 +38,6 @@ service_test("service_unittests") {
catalog("service_unittests_catalog") {
testonly = true
catalog_deps = [
"//services/device:tests_catalog",
"//services/preferences:tests_catalog",
"//services/video_capture:tests_catalog",
]
Expand Down
Loading

0 comments on commit 1a5a2cd

Please sign in to comment.