Commit dffc60cb authored by Min Qin's avatar Min Qin Committed by Commit Bot

Fix UMA_DOWNLOAD_RESUMPTION_BROWSER_KILLED

This UMA currently records all resumption attempts.
This CL fixes that by updating the UMA only on the first
resumption attempt after a browser restart.

BUG=804522

Change-Id: I8a591b0b0fdaf411693df374c2ad81b5979f3456
Reviewed-on: https://chromium-review.googlesource.com/883405Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531972}
parent 8ac6530d
...@@ -115,6 +115,8 @@ public class DownloadManagerService ...@@ -115,6 +115,8 @@ public class DownloadManagerService
"application/pkix-cert", "application/pkix-cert",
"application/x-wifi-config")); "application/x-wifi-config"));
private static final Set<String> sFirstSeenDownloadIds = new HashSet<String>();
private static DownloadManagerService sDownloadManagerService; private static DownloadManagerService sDownloadManagerService;
private static boolean sIsNetworkListenerDisabled; private static boolean sIsNetworkListenerDisabled;
private static boolean sIsNetworkMetered; private static boolean sIsNetworkMetered;
...@@ -607,6 +609,7 @@ public class DownloadManagerService ...@@ -607,6 +609,7 @@ public class DownloadManagerService
progress.mIsUpdated = true; progress.mIsUpdated = true;
progress.mIsSupportedMimeType = isSupportedMimeType; progress.mIsSupportedMimeType = isSupportedMimeType;
mDownloadProgressMap.put(id, progress); mDownloadProgressMap.put(id, progress);
sFirstSeenDownloadIds.add(id);
DownloadUmaStatsEntry entry = getUmaStatsEntry(downloadItem.getId()); DownloadUmaStatsEntry entry = getUmaStatsEntry(downloadItem.getId());
if (entry == null) { if (entry == null) {
addUmaStatsEntry(new DownloadUmaStatsEntry( addUmaStatsEntry(new DownloadUmaStatsEntry(
...@@ -636,21 +639,22 @@ public class DownloadManagerService ...@@ -636,21 +639,22 @@ public class DownloadManagerService
case DOWNLOAD_STATUS_COMPLETE: case DOWNLOAD_STATUS_COMPLETE:
case DOWNLOAD_STATUS_FAILED: case DOWNLOAD_STATUS_FAILED:
case DOWNLOAD_STATUS_CANCELLED: case DOWNLOAD_STATUS_CANCELLED:
recordDownloadFinishedUMA(downloadStatus, downloadItem.getId(), recordDownloadFinishedUMA(
downloadItem.getDownloadInfo().getBytesReceived()); downloadStatus, id, downloadItem.getDownloadInfo().getBytesReceived());
clearDownloadRetryCount(downloadItem.getId(), true); clearDownloadRetryCount(id, true);
clearDownloadRetryCount(downloadItem.getId(), false); clearDownloadRetryCount(id, false);
updateNotification(progress); updateNotification(progress);
sFirstSeenDownloadIds.remove(id);
break; break;
case DOWNLOAD_STATUS_INTERRUPTED: case DOWNLOAD_STATUS_INTERRUPTED:
entry = getUmaStatsEntry(downloadItem.getId()); entry = getUmaStatsEntry(id);
entry.numInterruptions++; entry.numInterruptions++;
updateBytesReceived(entry, bytesReceived); updateBytesReceived(entry, bytesReceived);
storeUmaEntries(); storeUmaEntries();
updateNotification(progress); updateNotification(progress);
break; break;
case DOWNLOAD_STATUS_IN_PROGRESS: case DOWNLOAD_STATUS_IN_PROGRESS:
entry = getUmaStatsEntry(downloadItem.getId()); entry = getUmaStatsEntry(id);
if (entry.isPaused != downloadItem.getDownloadInfo().isPaused() if (entry.isPaused != downloadItem.getDownloadInfo().isPaused()
|| updateBytesReceived(entry, bytesReceived)) { || updateBytesReceived(entry, bytesReceived)) {
entry.isPaused = downloadItem.getDownloadInfo().isPaused(); entry.isPaused = downloadItem.getDownloadInfo().isPaused();
...@@ -903,10 +907,14 @@ public class DownloadManagerService ...@@ -903,10 +907,14 @@ public class DownloadManagerService
recordDownloadResumption(uma); recordDownloadResumption(uma);
if (progress == null) { if (progress == null) {
assert !item.getDownloadInfo().isPaused(); assert !item.getDownloadInfo().isPaused();
// If the download was not resumed before, the browser must have been killed while the
// download is active.
if (!sFirstSeenDownloadIds.contains(item.getId())) {
sFirstSeenDownloadIds.add(item.getId());
recordDownloadResumption(UMA_DOWNLOAD_RESUMPTION_BROWSER_KILLED);
}
updateDownloadProgress(item, DOWNLOAD_STATUS_IN_PROGRESS); updateDownloadProgress(item, DOWNLOAD_STATUS_IN_PROGRESS);
progress = mDownloadProgressMap.get(item.getId()); progress = mDownloadProgressMap.get(item.getId());
// If progress is null, the browser must have been killed while the download is active.
recordDownloadResumption(UMA_DOWNLOAD_RESUMPTION_BROWSER_KILLED);
} }
if (hasUserGesture) { if (hasUserGesture) {
// If user manually resumes a download, update the connection type that the download // If user manually resumes a download, update the connection type that the download
...@@ -1235,6 +1243,7 @@ public class DownloadManagerService ...@@ -1235,6 +1243,7 @@ public class DownloadManagerService
private void removeDownloadProgress(String guid) { private void removeDownloadProgress(String guid) {
mDownloadProgressMap.remove(guid); mDownloadProgressMap.remove(guid);
removeAutoResumableDownload(guid); removeAutoResumableDownload(guid);
sFirstSeenDownloadIds.remove(guid);
} }
@Override @Override
......
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 to comment