Skip to content

Commit

Permalink
Make BackgroundTaskScheduler become a pure virtual interface.
Browse files Browse the repository at this point in the history
The BackgroundTaskScheduler is currently a concrete class, which hampers
the ability to use it in tests outside of the component itself.

This CL moves the BackgroundTaskScheduler implementation to a new class
BackgroundTaskScedulerImpl, and makes BackgroundTaskScheduler become
just pure virtual interface.

This made it possible to make both the *Impl and *Delegate classes
become package protected as well, and simplified some tests in the
//chrome layer.

Lastly, it creates a new class BackgroundTaskReflection which contains
functionality related to inspect and create BackgroundTask instances,
and it was used outside of the BackgroundTaskSchedulerImpl itself.

BUG=729156

Change-Id: Ide01a6e12176fb4cf910634f06f5681c04ec38e8
Reviewed-on: https://chromium-review.googlesource.com/612127
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494887}
  • Loading branch information
tommynyquist authored and Commit Bot committed Aug 16, 2017
1 parent 05b71cb commit c2501b4
Show file tree
Hide file tree
Showing 12 changed files with 224 additions and 176 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.components.background_task_scheduler.BackgroundTask.TaskFinishedCallback;
import org.chromium.components.background_task_scheduler.BackgroundTaskScheduler;
import org.chromium.components.background_task_scheduler.BackgroundTaskSchedulerDelegate;
import org.chromium.components.background_task_scheduler.TaskIds;
import org.chromium.components.background_task_scheduler.TaskInfo;
import org.chromium.components.background_task_scheduler.TaskParameters;
Expand Down Expand Up @@ -113,27 +112,12 @@ public boolean needsReschedule() {
}
}

private static class NoopBackgroundTaskSchedulerDelegate
implements BackgroundTaskSchedulerDelegate {
@Override
public boolean schedule(Context context, TaskInfo taskInfo) {
return true;
}

@Override
public void cancel(Context context, int taskId) {}
}

private static class TestBackgroundTaskScheduler extends BackgroundTaskScheduler {
private static class TestBackgroundTaskScheduler implements BackgroundTaskScheduler {
private HashMap<Integer, TestPrefetchBackgroundTask> mTasks = new HashMap<>();
private Semaphore mStartSemaphore = new Semaphore(0);
private int mAddCount = 0;
private int mRemoveCount = 0;

public TestBackgroundTaskScheduler() {
super(new NoopBackgroundTaskSchedulerDelegate());
}

@Override
public boolean schedule(final Context context, final TaskInfo taskInfo) {
mAddCount++;
Expand All @@ -159,6 +143,12 @@ public void cancel(Context context, int taskId) {
removeTask(taskId);
}

@Override
public void checkForOSUpgrade(Context context) {}

@Override
public void reschedule(Context context) {}

public void waitForTaskStarted() throws Exception {
assertTrue(mStartSemaphore.tryAcquire(SEMAPHORE_TIMEOUT_MS, TimeUnit.MILLISECONDS));
// Reset for next task.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

package org.chromium.chrome.browser.offlinepages.prefetch;

import android.content.Context;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
Expand All @@ -18,7 +16,8 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.junit.After;
import android.content.Context;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -30,9 +29,6 @@
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.robolectric.annotation.Config;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;
import org.robolectric.internal.ShadowExtractor;
import org.robolectric.shadows.multidex.ShadowMultiDex;

import org.chromium.base.library_loader.ProcessInitException;
Expand All @@ -52,28 +48,31 @@

/** Unit tests for {@link PrefetchBackgroundTask}. */
@RunWith(LocalRobolectricTestRunner.class)
@Config(manifest = Config.NONE,
shadows = {PrefetchBackgroundTaskUnitTest.ShadowBackgroundTaskScheduler.class,
ShadowMultiDex.class})
@Config(manifest = Config.NONE, shadows = {ShadowMultiDex.class})
public class PrefetchBackgroundTaskUnitTest {
/**
* Shadow of BackgroundTaskScheduler system service.
* Fake of BackgroundTaskScheduler system service.
*/
@Implements(BackgroundTaskScheduler.class)
public static class ShadowBackgroundTaskScheduler {
public static class FakeBackgroundTaskScheduler implements BackgroundTaskScheduler {
private HashMap<Integer, TaskInfo> mTaskInfos = new HashMap<>();

@Implementation
@Override
public boolean schedule(Context context, TaskInfo taskInfo) {
mTaskInfos.put(taskInfo.getTaskId(), taskInfo);
return true;
}

@Implementation
@Override
public void cancel(Context context, int taskId) {
mTaskInfos.remove(taskId);
}

@Override
public void checkForOSUpgrade(Context context) {}

@Override
public void reschedule(Context context) {}

public TaskInfo getTaskInfo(int taskId) {
return mTaskInfos.get(taskId);
}
Expand All @@ -89,7 +88,7 @@ public void clear() {
private ChromeBrowserInitializer mChromeBrowserInitializer;
@Captor
ArgumentCaptor<BrowserParts> mBrowserParts;
private ShadowBackgroundTaskScheduler mShadowTaskScheduler;
private FakeBackgroundTaskScheduler mFakeTaskScheduler;

@Before
public void setUp() {
Expand Down Expand Up @@ -122,21 +121,16 @@ public Object answer(InvocationOnMock invocation) {
doReturn(true).when(mPrefetchBackgroundTask).nativeOnStopTask(1);
doReturn(null).when(mPrefetchBackgroundTask).getProfile();

mShadowTaskScheduler = (ShadowBackgroundTaskScheduler) ShadowExtractor.extract(
BackgroundTaskSchedulerFactory.getScheduler());
}

@After
public void tearDown() {
mShadowTaskScheduler.clear();
mFakeTaskScheduler = new FakeBackgroundTaskScheduler();
BackgroundTaskSchedulerFactory.setSchedulerForTesting(mFakeTaskScheduler);
}

@Test
public void scheduleTask() {
final int additionalDelaySeconds = 15;
PrefetchBackgroundTask.scheduleTask(additionalDelaySeconds, true);
TaskInfo scheduledTask =
mShadowTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
mFakeTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
assertNotNull(scheduledTask);
assertEquals(TimeUnit.SECONDS.toMillis(PrefetchBackgroundTask.DEFAULT_START_DELAY_SECONDS
+ additionalDelaySeconds),
Expand All @@ -148,17 +142,17 @@ public void scheduleTask() {
@Test
public void cancelTask() {
TaskInfo scheduledTask =
mShadowTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
mFakeTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
assertNull(scheduledTask);

PrefetchBackgroundTask.scheduleTask(0, true);
scheduledTask = mShadowTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
scheduledTask = mFakeTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
assertNotNull(scheduledTask);
assertEquals(TimeUnit.SECONDS.toMillis(PrefetchBackgroundTask.DEFAULT_START_DELAY_SECONDS),
scheduledTask.getOneOffInfo().getWindowStartTimeMs());

PrefetchBackgroundTask.cancelTask();
scheduledTask = mShadowTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
scheduledTask = mFakeTaskScheduler.getTaskInfo(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID);
assertNull(scheduledTask);
}

Expand Down
4 changes: 3 additions & 1 deletion components/background_task_scheduler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ if (is_android) {
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTask.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskService.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskJobService.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskReflection.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskScheduler.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerDelegate.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerFactory.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerGcmNetworkManager.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerImpl.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerJobService.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerPrefs.java",
"android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerUma.java",
Expand Down Expand Up @@ -73,8 +75,8 @@ if (is_android) {
java_files = [
"android/junit/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerGcmNetworkManagerTest.java",
"android/junit/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskServiceTest.java",
"android/junit/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerImplTest.java",
"android/junit/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerPrefsTest.java",
"android/junit/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerTest.java",
"android/junit/src/org/chromium/components/background_task_scheduler/BackgroundTaskSchedulerUmaTest.java",
"android/junit/src/org/chromium/components/background_task_scheduler/ShadowGcmNetworkManager.java",
"android/junit/src/org/chromium/components/background_task_scheduler/TestBackgroundTask.java",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2017 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.components.background_task_scheduler;

import android.support.annotation.Nullable;

import org.chromium.base.Log;

import java.lang.reflect.Constructor;

/**
* BakgroundTaskReflection contains functionality to construct {@link BackgroundTask} instances from
* their classname and to inspect whether a particular {@link BackgroundTask} has a parameterless
* constructor, which is required for any {@link BackgroundTask}.
*/
final class BackgroundTaskReflection {
private static final String TAG = "BkgrdTaskReflect";

/**
* Uses reflection to find the given class and instantiates a {@link BackgroundTask} if the
* class is valid.
*
* @param backgroundTaskClassName the full class name to look for.
* @return a new {@link BackgroundTask} instance if successful or null when a failure occurs.
*/
@Nullable
static BackgroundTask getBackgroundTaskFromClassName(String backgroundTaskClassName) {
if (backgroundTaskClassName == null) return null;

Class<?> clazz;
try {
clazz = Class.forName(backgroundTaskClassName);
} catch (ClassNotFoundException e) {
Log.w(TAG, "Unable to find BackgroundTask class with name " + backgroundTaskClassName);
return null;
}

if (!BackgroundTask.class.isAssignableFrom(clazz)) {
Log.w(TAG, "Class " + clazz + " is not a BackgroundTask");
return null;
}

try {
return (BackgroundTask) clazz.newInstance();
} catch (InstantiationException e) {
Log.w(TAG, "Unable to instantiate class (InstExc) " + clazz);
return null;
} catch (IllegalAccessException e) {
Log.w(TAG, "Unable to instantiate class (IllAccExc) " + clazz);
return null;
}
}

/**
* Inspects all public constructors of the given class, and returns true if one of them is
* parameterless.
*
* @param clazz the class to inspect.
* @return whether the class has a parameterless constructor.
*/
static boolean hasParameterlessPublicConstructor(Class<? extends BackgroundTask> clazz) {
for (Constructor<?> constructor : clazz.getConstructors()) {
if (constructor.getParameterTypes().length == 0) return true;
}
return false;
}

private BackgroundTaskReflection() {
// No instantiation.
}
}
Loading

0 comments on commit c2501b4

Please sign in to comment.