Commit de7c62a9 by olly Committed by Oliver Woodman

Remove unnecessary logging

As justification for why we should not have this type of logging,
it would scale up to about 13K LOC, 1800 Strings, and 36K (after
pro-guarding - in the case of the demo app) if we did it through
the whole code base*. It makes the code messier to read, and in
most cases doesn't add significant value.

Note: I left the Scheduler logging because it logs interactions
with some awkward library components outside of ExoPlayer, so is
perhaps a bit more justified.

* This is a bit unfair since realistically we wouldn't ever add
  lots of logging into trivial classes. But I think it is fair
  to say that the deltas would be non-negligible.

PiperOrigin-RevId: 246181421
parent ea0efa74
......@@ -57,6 +57,7 @@ import com.google.android.exoplayer2.util.Util;
*/
public final class JobDispatcherScheduler implements Scheduler {
private static final boolean DEBUG = false;
private static final String TAG = "JobDispatcherScheduler";
private static final String KEY_SERVICE_ACTION = "service_action";
private static final String KEY_SERVICE_PACKAGE = "service_package";
......
......@@ -81,28 +81,6 @@ public final class Download {
/** The download isn't stopped. */
public static final int STOP_REASON_NONE = 0;
/** Returns the state string for the given state value. */
public static String getStateString(@State int state) {
switch (state) {
case STATE_QUEUED:
return "QUEUED";
case STATE_STOPPED:
return "STOPPED";
case STATE_DOWNLOADING:
return "DOWNLOADING";
case STATE_COMPLETED:
return "COMPLETED";
case STATE_FAILED:
return "FAILED";
case STATE_REMOVING:
return "REMOVING";
case STATE_RESTARTING:
return "RESTARTING";
default:
throw new IllegalStateException();
}
}
/** The download request. */
public final DownloadRequest request;
/** The state of the download. */
......
......@@ -127,18 +127,18 @@ public abstract class DownloadService extends Service {
public static final String KEY_DOWNLOAD_REQUEST = "download_request";
/**
* Key for the content id in {@link #ACTION_SET_STOP_REASON} and {@link #ACTION_REMOVE_DOWNLOAD}
* intents.
* Key for the {@link String} content id in {@link #ACTION_SET_STOP_REASON} and {@link
* #ACTION_REMOVE_DOWNLOAD} intents.
*/
public static final String KEY_CONTENT_ID = "content_id";
/**
* Key for the stop reason in {@link #ACTION_SET_STOP_REASON} and {@link #ACTION_ADD_DOWNLOAD}
* intents.
* Key for the integer stop reason in {@link #ACTION_SET_STOP_REASON} and {@link
* #ACTION_ADD_DOWNLOAD} intents.
*/
public static final String KEY_STOP_REASON = "stop_reason";
/** Key for the requirements in {@link #ACTION_SET_REQUIREMENTS} intents. */
/** Key for the {@link Requirements} in {@link #ACTION_SET_REQUIREMENTS} intents. */
public static final String KEY_REQUIREMENTS = "requirements";
/**
......@@ -155,7 +155,6 @@ public abstract class DownloadService extends Service {
public static final long DEFAULT_FOREGROUND_NOTIFICATION_UPDATE_INTERVAL = 1000;
private static final String TAG = "DownloadService";
private static final boolean DEBUG = false;
// Keep DownloadManagerListeners for each DownloadService as long as there are downloads (and the
// process is running). This allows DownloadService to restart when there's no scheduler.
......@@ -506,7 +505,6 @@ public abstract class DownloadService extends Service {
@Override
public void onCreate() {
logd("onCreate");
if (channelId != null) {
NotificationUtil.createNotificationChannel(
this, channelId, channelNameResourceId, NotificationUtil.IMPORTANCE_LOW);
......@@ -541,7 +539,6 @@ public abstract class DownloadService extends Service {
if (intentAction == null) {
intentAction = ACTION_INIT;
}
logd("onStartCommand action: " + intentAction + " startId: " + startId);
switch (intentAction) {
case ACTION_INIT:
case ACTION_RESTART:
......@@ -573,7 +570,7 @@ public abstract class DownloadService extends Service {
if (!intent.hasExtra(KEY_STOP_REASON)) {
Log.e(TAG, "Ignored SET_STOP_REASON: Missing " + KEY_STOP_REASON + " extra");
} else {
int stopReason = intent.getIntExtra(KEY_STOP_REASON, Download.STOP_REASON_NONE);
int stopReason = intent.getIntExtra(KEY_STOP_REASON, /* defaultValue= */ 0);
downloadManager.setStopReason(contentId, stopReason);
}
break;
......@@ -598,13 +595,11 @@ public abstract class DownloadService extends Service {
@Override
public void onTaskRemoved(Intent rootIntent) {
logd("onTaskRemoved rootIntent: " + rootIntent);
taskRemoved = true;
}
@Override
public void onDestroy() {
logd("onDestroy");
isDestroyed = true;
DownloadManagerHelper downloadManagerHelper = downloadManagerListeners.get(getClass());
boolean unschedule = !downloadManager.isWaitingForRequirements();
......@@ -713,16 +708,8 @@ public abstract class DownloadService extends Service {
}
if (Util.SDK_INT < 28 && taskRemoved) { // See [Internal: b/74248644].
stopSelf();
logd("stopSelf()");
} else {
boolean stopSelfResult = stopSelfResult(lastStartId);
logd("stopSelf(" + lastStartId + ") result: " + stopSelfResult);
}
}
private void logd(String message) {
if (DEBUG) {
Log.d(TAG, message);
stopSelfResult(lastStartId);
}
}
......
......@@ -44,6 +44,7 @@ import com.google.android.exoplayer2.util.Util;
@TargetApi(21)
public final class PlatformScheduler implements Scheduler {
private static final boolean DEBUG = false;
private static final String TAG = "PlatformScheduler";
private static final String KEY_SERVICE_ACTION = "service_action";
private static final String KEY_SERVICE_PACKAGE = "service_package";
......
......@@ -27,7 +27,6 @@ import android.os.Parcel;
import android.os.Parcelable;
import android.os.PowerManager;
import androidx.annotation.IntDef;
import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.Util;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
......@@ -56,8 +55,6 @@ public final class Requirements implements Parcelable {
/** Requirement that the device is charging. */
public static final int DEVICE_CHARGING = 1 << 3;
private static final String TAG = "Requirements";
@RequirementFlags private final int requirements;
/** @param requirements A combination of requirement flags. */
......@@ -135,7 +132,6 @@ public final class Requirements implements Parcelable {
if (networkInfo == null
|| !networkInfo.isConnected()
|| !isInternetConnectivityValidated(connectivityManager)) {
logd("No network info, connection or connectivity.");
return requirements & (NETWORK | NETWORK_UNMETERED);
}
......@@ -172,7 +168,6 @@ public final class Requirements implements Parcelable {
}
Network activeNetwork = connectivityManager.getActiveNetwork();
if (activeNetwork == null) {
logd("No active network.");
return false;
}
NetworkCapabilities networkCapabilities =
......@@ -180,16 +175,9 @@ public final class Requirements implements Parcelable {
boolean validated =
networkCapabilities == null
|| !networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED);
logd("Network capability validated: " + validated);
return !validated;
}
private static void logd(String message) {
if (Scheduler.DEBUG) {
Log.d(TAG, message);
}
}
@Override
public boolean equals(Object o) {
if (this == o) {
......
......@@ -28,7 +28,6 @@ import android.os.Handler;
import android.os.Looper;
import android.os.PowerManager;
import androidx.annotation.RequiresApi;
import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.Util;
/**
......@@ -53,8 +52,6 @@ public final class RequirementsWatcher {
@Requirements.RequirementFlags int notMetRequirements);
}
private static final String TAG = "RequirementsWatcher";
private final Context context;
private final Listener listener;
private final Requirements requirements;
......@@ -75,7 +72,6 @@ public final class RequirementsWatcher {
this.listener = listener;
this.requirements = requirements;
handler = new Handler(Util.getLooper());
logd(this + " created");
}
/**
......@@ -110,7 +106,6 @@ public final class RequirementsWatcher {
}
receiver = new DeviceStatusChangeReceiver();
context.registerReceiver(receiver, filter, null, handler);
logd(this + " started");
return notMetRequirements;
}
......@@ -121,7 +116,6 @@ public final class RequirementsWatcher {
if (networkCallback != null) {
unregisterNetworkCallback();
}
logd(this + " stopped");
}
/** Returns watched {@link Requirements}. */
......@@ -129,14 +123,6 @@ public final class RequirementsWatcher {
return requirements;
}
@Override
public String toString() {
if (!Scheduler.DEBUG) {
return super.toString();
}
return "RequirementsWatcher{" + requirements + '}';
}
@TargetApi(23)
private void registerNetworkCallbackV23() {
ConnectivityManager connectivityManager =
......@@ -163,22 +149,14 @@ public final class RequirementsWatcher {
int notMetRequirements = requirements.getNotMetRequirements(context);
if (this.notMetRequirements != notMetRequirements) {
this.notMetRequirements = notMetRequirements;
logd("notMetRequirements has changed: " + notMetRequirements);
listener.onRequirementsStateChanged(this, notMetRequirements);
}
}
private static void logd(String message) {
if (Scheduler.DEBUG) {
Log.d(TAG, message);
}
}
private class DeviceStatusChangeReceiver extends BroadcastReceiver {
@Override
public void onReceive(Context context, Intent intent) {
if (!isInitialStickyBroadcast()) {
logd(RequirementsWatcher.this + " received " + intent.getAction());
checkRequirements();
}
}
......@@ -200,7 +178,6 @@ public final class RequirementsWatcher {
handler.post(
() -> {
if (networkCallback != null) {
logd(RequirementsWatcher.this + " NetworkCallback");
checkRequirements();
}
});
......
......@@ -22,8 +22,6 @@ import android.content.Intent;
/** Schedules a service to be started in the foreground when some {@link Requirements} are met. */
public interface Scheduler {
/* package */ boolean DEBUG = false;
/**
* Schedules a service to be started in the foreground when some {@link Requirements} are met.
* Anything that was previously scheduled will be canceled.
......
......@@ -22,9 +22,7 @@ import android.os.ConditionVariable;
import com.google.android.exoplayer2.offline.Download;
import com.google.android.exoplayer2.offline.Download.State;
import com.google.android.exoplayer2.offline.DownloadManager;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Locale;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
......@@ -138,7 +136,6 @@ public final class TestDownloadManagerListener implements DownloadManager.Listen
}
private void assertStateInternal(String taskId, int expectedState, int timeoutMs) {
ArrayList<Integer> receivedStates = new ArrayList<>();
while (true) {
Integer state = null;
try {
......@@ -150,25 +147,8 @@ public final class TestDownloadManagerListener implements DownloadManager.Listen
if (expectedState == state) {
return;
}
receivedStates.add(state);
} else {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < receivedStates.size(); i++) {
if (i > 0) {
sb.append(',');
}
int receivedState = receivedStates.get(i);
String receivedStateString =
receivedState == STATE_REMOVED ? "REMOVED" : Download.getStateString(receivedState);
sb.append(receivedStateString);
}
fail(
String.format(
Locale.US,
"for download (%s) expected:<%s> but was:<%s>",
taskId,
Download.getStateString(expectedState),
sb));
fail("Didn't receive expected state: " + expectedState);
}
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment