Skip to content

Commit

Permalink
Revert "Report precache metrics even if native library is not loaded"
Browse files Browse the repository at this point in the history
This reverts commit e0392f8.

Reason for revert:
Broke Instrumentation test chrome_public_test_apk on
https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29

TBR=rajendrant@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2076903002
Cr-Commit-Position: refs/heads/master@{#400379}
  • Loading branch information
grunell authored and Commit bot committed Jun 17, 2016
1 parent 3b634a9 commit 5c5abd0
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 384 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.chromium.chrome.browser.offlinepages.BackgroundSchedulerProcessorImpl;
import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
import org.chromium.chrome.browser.precache.PrecacheController;
import org.chromium.chrome.browser.precache.PrecacheUMA;

/**
* {@link ChromeBackgroundService} is scheduled through the {@link GcmNetworkManager} when the
Expand All @@ -51,36 +50,34 @@ public class ChromeBackgroundService extends GcmTaskService {
@Override
@VisibleForTesting
public int onRunTask(final TaskParams params) {
final String taskTag = params.getTag();
Log.i(TAG, "[" + taskTag + "] Woken up at " + new java.util.Date().toString());
Log.i(TAG, "[" + params.getTag() + "] Woken up at " + new java.util.Date().toString());
final ChromeBackgroundServiceWaiter waiter = getWaiterIfNeeded(params.getExtras());
final Context context = this;
ThreadUtils.runOnUiThread(new Runnable() {
@Override
public void run() {
switch (taskTag) {
switch(params.getTag()) {
case BackgroundSyncLauncher.TASK_TAG:
handleBackgroundSyncEvent(context, taskTag);
handleBackgroundSyncEvent(context);
break;

case OfflinePageUtils.TASK_TAG:
handleOfflinePageBackgroundLoad(
context, params.getExtras(), waiter, taskTag);
handleOfflinePageBackgroundLoad(context, params.getExtras(), waiter);
break;

case SnippetsLauncher.TASK_TAG_WIFI_CHARGING:
case SnippetsLauncher.TASK_TAG_WIFI:
case SnippetsLauncher.TASK_TAG_FALLBACK:
handleFetchSnippets(context, taskTag);
handleFetchSnippets(context);
break;

case SnippetsLauncher.TASK_TAG_RESCHEDULE:
handleRescheduleSnippets(context, taskTag);
handleRescheduleSnippets(context);
break;

case PrecacheController.PERIODIC_TASK_TAG:
case PrecacheController.CONTINUATION_TASK_TAG:
handlePrecache(context, taskTag);
handlePrecache(context, params.getTag());
break;

case DownloadResumptionScheduler.TASK_TAG:
Expand All @@ -89,7 +86,7 @@ public void run() {
break;

default:
Log.i(TAG, "Unknown task tag " + taskTag);
Log.i(TAG, "Unknown task tag " + params.getTag());
break;
}
}
Expand All @@ -100,21 +97,21 @@ public void run() {
return GcmNetworkManager.RESULT_SUCCESS;
}

private void handleBackgroundSyncEvent(Context context, String tag) {
private void handleBackgroundSyncEvent(Context context) {
if (!BackgroundSyncLauncher.hasInstance()) {
// Start the browser. The browser's BackgroundSyncManager (for the active profile) will
// start, check the network, and run any necessary sync events. This task runs with a
// wake lock, but has a three minute timeout, so we need to start the browser in its
// own task.
// TODO(jkarlin): Protect the browser sync event with a wake lock.
// See crbug.com/486020.
launchBrowser(context, tag);
launchBrowser(context);
}
}

private void handleFetchSnippets(Context context, String tag) {
private void handleFetchSnippets(Context context) {
if (!SnippetsLauncher.hasInstance()) {
launchBrowser(context, tag);
launchBrowser(context);
}
fetchSnippets();
}
Expand All @@ -124,9 +121,9 @@ protected void fetchSnippets() {
SnippetsBridge.fetchSnippets();
}

private void handleRescheduleSnippets(Context context, String tag) {
private void handleRescheduleSnippets(Context context) {
if (!SnippetsLauncher.hasInstance()) {
launchBrowser(context, tag);
launchBrowser(context);
}
rescheduleSnippets();
}
Expand All @@ -138,7 +135,7 @@ protected void rescheduleSnippets() {

private void handlePrecache(Context context, String tag) {
if (!hasPrecacheInstance()) {
launchBrowser(context, tag);
launchBrowser(context);
}
precache(context, tag);
}
Expand All @@ -154,9 +151,9 @@ protected void precache(Context context, String tag) {
}

private void handleOfflinePageBackgroundLoad(
Context context, Bundle bundle, ChromeBackgroundServiceWaiter waiter, String tag) {
Context context, Bundle bundle, ChromeBackgroundServiceWaiter waiter) {
if (!LibraryLoader.isInitialized()) {
launchBrowser(context, tag);
launchBrowser(context);
}

// Call BackgroundTask, provide context.
Expand Down Expand Up @@ -196,22 +193,12 @@ public void waitForTaskIfNeeded(ChromeBackgroundServiceWaiter waiter) {

@VisibleForTesting
@SuppressFBWarnings("DM_EXIT")
protected void launchBrowser(Context context, String tag) {
protected void launchBrowser(Context context) {
Log.i(TAG, "Launching browser");
try {
ChromeBrowserInitializer.getInstance(this).handleSynchronousStartup();
} catch (ProcessInitException e) {
Log.e(TAG, "ProcessInitException while starting the browser process");
switch (tag) {
case PrecacheController.PERIODIC_TASK_TAG:
case PrecacheController.CONTINUATION_TASK_TAG:
// Record the failure persistently, and upload to UMA when the library
// successfully loads next time.
PrecacheUMA.record(PrecacheUMA.Event.PRECACHE_TASK_LOAD_LIBRARY_FAIL);
break;
default:
break;
}
// Since the library failed to initialize nothing in the application
// can work, so kill the whole application not just the activity.
System.exit(-1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ public class PrecacheBootReceiver extends BroadcastReceiver {
@Override
public void onReceive(Context context, Intent intent) {
Log.v(TAG, "received boot signal");
if (PrecacheController.schedulePeriodicPrecacheTask(context)) {
PrecacheUMA.record(PrecacheUMA.Event.PERIODIC_TASK_SCHEDULE_BOOT_SIGNAL);
} else {
PrecacheUMA.record(PrecacheUMA.Event.PERIODIC_TASK_SCHEDULE_BOOT_SIGNAL_FAIL);
}
PrecacheController.schedulePeriodicPrecacheTask(context);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ public void onReceive(Context context, Intent intent) {
if (isPrecaching() && (!mDeviceState.isPowerConnected(context)
|| !mDeviceState.isUnmeteredNetworkAvailable(context))) {
recordFailureReasons(context);
cancelPrecaching(!mDeviceState.isPowerConnected(context)
? PrecacheUMA.Event.PRECACHE_CANCEL_NO_POWER
: PrecacheUMA.Event.PRECACHE_CANCEL_NO_UNMETERED_NETWORK);
cancelPrecaching();
}
}
};
Expand All @@ -128,7 +126,7 @@ public void onReceive(Context context, Intent intent) {
@Override
public void run() {
Log.v(TAG, "precache session timed out");
cancelPrecaching(PrecacheUMA.Event.PRECACHE_SESSION_TIMEOUT);
cancelPrecaching();
}
};

Expand Down Expand Up @@ -157,13 +155,9 @@ public static boolean hasInstance() {
return sInstance != null;
}

/**
* Schedules a periodic task to precache resources.
* @param context The application context.
* @return false if the task cannot be scheduled.
*/
public static boolean schedulePeriodicPrecacheTask(Context context) {
if (!canScheduleTasks(context)) return true;
/** Schedules a periodic task to precache resources. */
public static void schedulePeriodicPrecacheTask(Context context) {
if (!canScheduleTasks(context)) return;

PeriodicTask task = new PeriodicTask.Builder()
.setPeriod(WAIT_UNTIL_NEXT_PRECACHE_SECONDS)
Expand All @@ -173,12 +167,14 @@ public static boolean schedulePeriodicPrecacheTask(Context context) {
.setService(ChromeBackgroundService.class)
.setTag(PERIODIC_TASK_TAG)
.build();
return sTaskScheduler.scheduleTask(context, task);
sTaskScheduler.scheduleTask(context, task);
// TODO(rajenrant): Track any failure via UMA.
}

private static void cancelPeriodicPrecacheTask(Context context) {
Log.v(TAG, "canceling a periodic precache task");
sTaskScheduler.cancelTask(context, PERIODIC_TASK_TAG);
// TODO(rajenrant): Track any failure via UMA.
}

/**
Expand All @@ -201,28 +197,18 @@ private static void schedulePrecacheCompletionTask(Context context) {
.setTag(CONTINUATION_TASK_TAG)
.setUpdateCurrent(true)
.build();
if (sTaskScheduler.scheduleTask(context, task)) {
PrecacheUMA.record(PrecacheUMA.Event.ONEOFF_TASK_SCHEDULE);
} else {
PrecacheUMA.record(PrecacheUMA.Event.ONEOFF_TASK_SCHEDULE_FAIL);
}
sTaskScheduler.scheduleTask(context, task);
// TODO(rajenrant): Track any failure via UMA.
}

private static void cancelPrecacheCompletionTask(Context context) {
Log.v(TAG, "canceling a precache completion task");
sTaskScheduler.cancelTask(context, CONTINUATION_TASK_TAG);
// TODO(rajenrant): Track any failure via UMA.
}

/**
* Called when Chrome package is upgraded to reschedule the precache periodic task.
* @param context The application context.
*/
public static void rescheduleTasksOnUpgrade(Context context) {
if (schedulePeriodicPrecacheTask(context)) {
PrecacheUMA.record(PrecacheUMA.Event.PERIODIC_TASK_SCHEDULE_UPGRADE);
} else {
PrecacheUMA.record(PrecacheUMA.Event.PERIODIC_TASK_SCHEDULE_UPGRADE_FAIL);
}
schedulePeriodicPrecacheTask(context);
}

@VisibleForTesting
Expand Down Expand Up @@ -292,7 +278,7 @@ public static void setIsPrecachingEnabled(Context context, boolean enabled) {
sInstance.runOnInstanceThread(new Runnable() {
@Override
public void run() {
sInstance.cancelPrecaching(PrecacheUMA.Event.PRECACHE_CANCEL_DISABLED_PREF);
sInstance.cancelPrecaching();
}
});
}
Expand Down Expand Up @@ -346,8 +332,6 @@ void handlePrecacheCompleted(boolean precachingIncomplete) {
if (setIsPrecaching(false)) {
shutdownPrecaching(precachingIncomplete);
}
PrecacheUMA.record(precachingIncomplete ? PrecacheUMA.Event.PRECACHE_SESSION_INCOMPLETE
: PrecacheUMA.Event.PRECACHE_SESSION_COMPLETE);
}

/** {@link PrecacheLauncher} used to run a precache session. */
Expand All @@ -365,15 +349,11 @@ protected void onPrecacheCompleted(boolean tryAgainSoon) {
*/
public int precache(String tag) {
assert mNonThreadSafe.calledOnValidThread();
PrecacheUMA.record(PERIODIC_TASK_TAG.equals(tag)
? PrecacheUMA.Event.PRECACHE_TASK_STARTED_PERIODIC
: PrecacheUMA.Event.PRECACHE_TASK_STARTED_ONEOFF);
Log.v(TAG, "precache task (%s) started", tag);
if (!isPrecachingEnabled()) {
Log.v(TAG, "precaching isn't enabled");
cancelPeriodicPrecacheTask(mAppContext);
cancelPrecacheCompletionTask(mAppContext);
PrecacheUMA.record(PrecacheUMA.Event.DISABLED_IN_PRECACHE_PREF);
return GcmNetworkManager.RESULT_SUCCESS;
}
if (setIsPrecaching(true)) {
Expand All @@ -385,7 +365,6 @@ public int precache(String tag) {
return GcmNetworkManager.RESULT_SUCCESS;
}
Log.v(TAG, "precache session was already running");
PrecacheUMA.record(PrecacheUMA.Event.PRECACHE_TASK_STARTED_DUPLICATE);
return GcmNetworkManager.RESULT_FAILURE;
}

Expand All @@ -400,7 +379,8 @@ public void onDataTypesActive() {

@Override
public void onFailureOrTimedOut() {
cancelPrecaching(PrecacheUMA.Event.SYNC_SERVICE_TIMEOUT);
// TODO(rajendrant): Add UMA histogram to track this failure.
cancelPrecaching();
}
}, MAX_SYNC_SERVICE_INIT_TIMOUT_MS);
}
Expand All @@ -413,7 +393,6 @@ void startPrecaching() {
acquirePrecachingWakeLock();

mHandler.postDelayed(mTimeoutRunnable, MAX_PRECACHE_DURATION_SECONDS * 1000);
PrecacheUMA.record(PrecacheUMA.Event.PRECACHE_SESSION_STARTED);

// In certain cases, the PrecacheLauncher will skip precaching entirely and call
// finishPrecaching() before this call to mPrecacheLauncher.start() returns, so the call to
Expand All @@ -422,17 +401,13 @@ void startPrecaching() {
mPrecacheLauncher.start();
}

/**
* Cancels the current precache session.
* @param event the failure reason.
*/
private void cancelPrecaching(int event) {
/** Cancels a precache session. */
private void cancelPrecaching() {
Log.v(TAG, "canceling precache session");
if (setIsPrecaching(false)) {
mPrecacheLauncher.cancel();
shutdownPrecaching(true);
}
PrecacheUMA.record(event);
}

/**
Expand Down
Loading

0 comments on commit 5c5abd0

Please sign in to comment.