chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java - Issue 2681033002: Add new UMA to record the bytes wasted due to download resumption

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

Issue 2681033002: Add new UMA to record the bytes wasted due to download resumption (Closed)

Patch Set: Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « no previous file | chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java » ('j') | tools/metrics/histograms/histograms.xml » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

index d3449c20b7c0b7b26fc7006c092b7a7169235d57..95c593951cd5105558585071f5c0968fe4bcf828 100644

--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

@@ -80,6 +80,7 @@ public class DownloadManagerService extends BroadcastReceiver implements

// Wait 10 seconds to resume all downloads, so that we won't impact tab loading.

private static final long RESUME_DELAY_MILLIS = 10000;

private static final int UNKNOWN_DOWNLOAD_STATUS = -1;

+ public static final long UNKNOWN_BYTES_RECEIVED = -1;

private static final String PREF_IS_DOWNLOAD_HOME_ENABLED =

"org.chromium.chrome.browser.download.IS_DOWNLOAD_HOME_ENABLED";

@@ -91,6 +92,8 @@ public class DownloadManagerService extends BroadcastReceiver implements

private static final int UMA_DOWNLOAD_RESUMPTION_AUTO_STARTED = 4;

private static final int UMA_DOWNLOAD_RESUMPTION_COUNT = 5;

+ private static final int GB_IN_KILO_BYTES = 1000 * 1000;

Ilya Sherman 2017/02/08 02:11:19 Are you sure that you want 1000 * 1000 and not 102

qinmin 2017/02/09 01:05:10 Switched to use 1024 * 1024. However, we don't rea

// Set will be more expensive to initialize, so use an ArrayList here.

private static final List<String> MIME_TYPES_TO_OPEN = new ArrayList<String>(Arrays.asList(

OMADownloadHandler.OMA_DOWNLOAD_DESCRIPTOR_MIME,

@@ -723,6 +726,7 @@ public class DownloadManagerService extends BroadcastReceiver implements

? isSupportedMimeType(downloadItem.getDownloadInfo().getMimeType()) : false;

String id = downloadItem.getId();

DownloadProgress progress = mDownloadProgressMap.get(id);

+ long bytesReceived = downloadItem.getDownloadInfo().getBytesReceived();

if (progress == null) {

if (!downloadItem.getDownloadInfo().isPaused()) {

long startTime = System.currentTimeMillis();

@@ -731,9 +735,12 @@ public class DownloadManagerService extends BroadcastReceiver implements

progress.mIsUpdated = true;

progress.mIsSupportedMimeType = isSupportedMimeType;

mDownloadProgressMap.put(id, progress);

- if (getUmaStatsEntry(downloadItem.getId()) == null) {

+ DownloadUmaStatsEntry entry = getUmaStatsEntry(downloadItem.getId());

+ if (entry == null) {

addUmaStatsEntry(new DownloadUmaStatsEntry(

- downloadItem.getId(), startTime, 0, false, false));

+ downloadItem.getId(), startTime, 0, false, false, bytesReceived, 0));

+ } else if (updateBytesReceived(entry, bytesReceived)) {

+ storeUmaEntries();

}

return;

@@ -755,11 +762,13 @@ public class DownloadManagerService extends BroadcastReceiver implements

case DOWNLOAD_STATUS_INTERRUPTED:

entry = getUmaStatsEntry(downloadItem.getId());

entry.numInterruptions++;

+ updateBytesReceived(entry, bytesReceived);

storeUmaEntries();

break;

case DOWNLOAD_STATUS_IN_PROGRESS:

entry = getUmaStatsEntry(downloadItem.getId());

- if (entry.isPaused != downloadItem.getDownloadInfo().isPaused()) {

+ if (entry.isPaused != downloadItem.getDownloadInfo().isPaused()

+ || updateBytesReceived(entry, bytesReceived)) {

entry.isPaused = downloadItem.getDownloadInfo().isPaused();

storeUmaEntries();

}

@@ -770,6 +779,22 @@ public class DownloadManagerService extends BroadcastReceiver implements

}

/**

+ * Helper method to update the received bytes and wasted bytes for UMA reporting.

+ * @param entry UMA entry to update.

+ * @param bytesReceived The current received bytes.

+ * @return true if the UMA stats is updated, or false otherwise.

+ */

+ private boolean updateBytesReceived(DownloadUmaStatsEntry entry, long bytesReceived) {

+ if (bytesReceived == UNKNOWN_BYTES_RECEIVED || bytesReceived == entry.lastBytesReceived) {

+ return false;

+ }

+ if (bytesReceived < entry.lastBytesReceived) {

+ entry.bytesWasted += entry.lastBytesReceived - bytesReceived;

+ }

+ entry.lastBytesReceived = bytesReceived;

+ return true;

+ }

+ /**

* Sets the download handler for OMA downloads, for testing purpose.

* @param omaDownloadHandler Download handler for OMA contents.

@@ -920,7 +945,7 @@ public class DownloadManagerService extends BroadcastReceiver implements

mDownloadItem.getSystemDownloadId());

if (!result) {

onDownloadFailed(mDownloadItem.getDownloadInfo().getFileName(), mFailureReason);

- recordDownloadCompletionStats(true, DOWNLOAD_STATUS_FAILED, 0, 0, 0);

+ recordDownloadCompletionStats(true, DOWNLOAD_STATUS_FAILED, 0, 0, 0, 0);

if (isPendingOMADownload) {

mOMADownloadHandler.onDownloadFailed(

mDownloadItem.getDownloadInfo(), mDownloadItem.getSystemDownloadId(),

@@ -948,7 +973,7 @@ public class DownloadManagerService extends BroadcastReceiver implements

new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE));

}

addUmaStatsEntry(new DownloadUmaStatsEntry(

- String.valueOf(mDownloadId), mStartTime, 0, false, true));

+ String.valueOf(mDownloadId), mStartTime, 0, false, true, 0, 0));

mDownloadItem.setSystemDownloadId(mDownloadId);

mDownloadItem.setStartTime(mStartTime);

mSystemDownloadIdMap.put(mDownloadId, mDownloadItem);

@@ -1171,7 +1196,8 @@ public class DownloadManagerService extends BroadcastReceiver implements

if (progress != null && (progress.mDownloadStatus == DOWNLOAD_STATUS_INTERRUPTED

|| progress.mDownloadStatus == DOWNLOAD_STATUS_IN_PROGRESS)) {

DownloadInfo info = DownloadInfo.Builder.fromDownloadInfo(

- progress.mDownloadItem.getDownloadInfo()).setIsPaused(true).build();

+ progress.mDownloadItem.getDownloadInfo()).setIsPaused(true)

+ .setBytesReceived(UNKNOWN_BYTES_RECEIVED).build();

onDownloadUpdated(info);

}

@@ -1278,7 +1304,7 @@ public class DownloadManagerService extends BroadcastReceiver implements

* @param numInterruptions Number of interruptions during the download.

private void recordDownloadCompletionStats(boolean useDownloadManager, int status,

- long totalDuration, long bytesDownloaded, int numInterruptions) {

+ long totalDuration, long bytesDownloaded, int numInterruptions, long bytesWasted) {

switch (status) {

case DOWNLOAD_STATUS_COMPLETE:

if (useDownloadManager) {

@@ -1298,6 +1324,8 @@ public class DownloadManagerService extends BroadcastReceiver implements

RecordHistogram.recordCountHistogram(

"MobileDownload.InterruptionsCount.ChromeNetworkStack.Success",

numInterruptions);

+ recordBytesWasted(

+ "MobileDownload.BytesWasted.ChromeNetworkStack.Success", bytesWasted);

}

break;

case DOWNLOAD_STATUS_FAILED:

@@ -1318,6 +1346,8 @@ public class DownloadManagerService extends BroadcastReceiver implements

RecordHistogram.recordCountHistogram(

"MobileDownload.InterruptionsCount.ChromeNetworkStack.Failure",

numInterruptions);

+ recordBytesWasted(

+ "MobileDownload.BytesWasted.ChromeNetworkStack.Failure", bytesWasted);

}

break;

case DOWNLOAD_STATUS_CANCELLED:

@@ -1328,6 +1358,8 @@ public class DownloadManagerService extends BroadcastReceiver implements

RecordHistogram.recordCountHistogram(

"MobileDownload.InterruptionsCount.ChromeNetworkStack.Cancel",

numInterruptions);

+ recordBytesWasted(

+ "MobileDownload.BytesWasted.ChromeNetworkStack.Cancel", bytesWasted);

}

break;

default:

@@ -1335,6 +1367,16 @@ public class DownloadManagerService extends BroadcastReceiver implements

}

+ /**

+ * Helper method to record the bytes wasted metrics when a download completes.

+ * @param name Histogram name

+ * @param bytesWasted Bytes wasted during download.

+ */

+ private void recordBytesWasted(String name, long bytesWasted) {

+ RecordHistogram.recordCustomCountHistogram(

+ name, (int) (bytesWasted / 1024), 1, GB_IN_KILO_BYTES, 50);

+ }

@Override

public void onQueryCompleted(

DownloadManagerDelegate.DownloadQueryResult result, boolean showNotification) {

@@ -1361,7 +1403,7 @@ public class DownloadManagerService extends BroadcastReceiver implements

}

recordDownloadCompletionStats(true, result.downloadStatus,

- result.downloadTimeInMilliseconds, result.bytesDownloaded, 0);

+ result.downloadTimeInMilliseconds, result.bytesDownloaded, 0, 0);

removeUmaStatsEntry(result.item.getId());

}

@@ -1525,7 +1567,8 @@ public class DownloadManagerService extends BroadcastReceiver implements

long currentTime = System.currentTimeMillis();

long totalTime = Math.max(0, currentTime - entry.downloadStartTime);

recordDownloadCompletionStats(

- false, downloadStatus, totalTime, bytesDownloaded, entry.numInterruptions);

+ false, downloadStatus, totalTime, bytesDownloaded, entry.numInterruptions,

+ entry.bytesWasted);

removeUmaStatsEntry(entryId);

}