Skip to content

Commit

Permalink
For mozilla-mobile#7673: Move DownloadProgress and Status to Download…
Browse files Browse the repository at this point in the history 
…State
  • Loading branch information
Kate Glazko authored and Kate Glazko committed Jul 20, 2020
1 parent f074469 commit fde00d5
Show file tree
Hide file tree
Showing 13 changed files with 275 additions and 210 deletions.
3 changes: 3 additions & 0 deletions ...browser/session/src/main/java/mozilla/components/browser/session/engine/EngineObserver.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import mozilla.components.browser.session.ext.toElement
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.MediaAction
import mozilla.components.browser.state.action.TrackingProtectionAction
import mozilla.components.browser.state.state.content.DownloadState.Status
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.EngineSession
Expand Down Expand Up @@ -192,6 +193,8 @@ internal class EngineObserver(
fileName,
contentType,
fileSize,
0,
Status.INITIATED,
userAgent,
Environment.DIRECTORY_DOWNLOADS
)
Expand Down
35 changes: 35 additions & 0 deletions ...owser/state/src/main/java/mozilla/components/browser/state/state/content/DownloadState.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import kotlin.random.Random
* @property fileName A canonical filename for this download.
* @property contentType Content type (MIME type) to indicate the media type of the download.
* @property contentLength The file size reported by the server.
* @property currentBytesCopied The number of current bytes copied.
* @property status The current status of the download.
* @property userAgent The user agent to be used for the download.
* @property destinationDirectory The matching destination directory for this type of download.
* @property filePath The file path the file was saved at.
Expand All @@ -32,6 +34,8 @@ data class DownloadState(
val fileName: String? = null,
val contentType: String? = null,
val contentLength: Long? = null,
val currentBytesCopied: Long = 0,
val status: Status = Status.INITIATED,
val userAgent: String? = null,
val destinationDirectory: String = Environment.DIRECTORY_DOWNLOADS,
val referrerUrl: String? = null,
Expand All @@ -41,4 +45,35 @@ data class DownloadState(
) : Parcelable {
val filePath: String get() =
Environment.getExternalStoragePublicDirectory(destinationDirectory).path + "/" + fileName

/**
* Status that represents every state that a download can be in
*/
enum class Status {
/**
* Indicates that the download is in the first state after creation but not yet [DOWNLOADING].
*/
INITIATED,
/**
* Indicates that an [INITIATED] download is now actively being downloaded.
*/
DOWNLOADING,
/**
* Indicates that the download that has been [DOWNLOADING] has been paused.
*/
PAUSED,
/**
* Indicates that the download that has been [DOWNLOADING] has been cancelled.
*/
CANCELLED,
/**
* Indicates that the download that has been [DOWNLOADING] has moved to failed because
* something unexpected has happened.
*/
FAILED,
/**
* Indicates that the [DOWNLOADING] download has been completed.
*/
COMPLETED
}
}
106 changes: 49 additions & 57 deletions ...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import android.os.ParcelFileDescriptor
import android.provider.MediaStore
import android.widget.Toast
import android.webkit.MimeTypeMap
import androidx.annotation.GuardedBy
import androidx.annotation.VisibleForTesting
import androidx.core.app.NotificationManagerCompat
import androidx.core.content.FileProvider
Expand All @@ -41,18 +40,14 @@ import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import mozilla.components.browser.state.action.DownloadAction
import mozilla.components.browser.state.state.content.DownloadState.Status
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.fetch.Client
import mozilla.components.concept.fetch.Headers.Names.CONTENT_RANGE
import mozilla.components.concept.fetch.Headers.Names.RANGE
import mozilla.components.concept.fetch.MutableHeaders
import mozilla.components.concept.fetch.Request
import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.CANCELLED
import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.COMPLETED
import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.ACTIVE
import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.FAILED
import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.PAUSED
import mozilla.components.feature.downloads.DownloadNotification.NOTIFICATION_DOWNLOAD_GROUP_ID
import mozilla.components.feature.downloads.ext.addCompletedDownload
import mozilla.components.feature.downloads.ext.isScheme
Expand Down Expand Up @@ -102,8 +97,6 @@ abstract class AbstractFetchDownloadService : Service() {
internal data class DownloadJobState(
var job: Job? = null,
@Volatile var state: DownloadState,
var currentBytesCopied: Long = 0,
@GuardedBy("context") var status: DownloadJobStatus,
var foregroundServiceId: Int = 0,
var downloadDeleted: Boolean = false,
var notifiedStopped: Boolean = false,
Expand All @@ -130,30 +123,20 @@ abstract class AbstractFetchDownloadService : Service() {
}
}

internal fun setDownloadJobStatus(downloadJobState: DownloadJobState, status: DownloadJobStatus) {
internal fun setDownloadJobStatus(downloadJobState: DownloadJobState, status: Status) {
synchronized(context) {
if (status == DownloadJobStatus.ACTIVE) { downloadJobState.notifiedStopped = false }
downloadJobState.status = status
if (status == Status.DOWNLOADING) { downloadJobState.notifiedStopped = false }
val newState = downloadJobState.state.copy(status = status)
updateDownloadState(newState)
}
}

internal fun getDownloadJobStatus(downloadJobState: DownloadJobState): DownloadJobStatus {
internal fun getDownloadJobStatus(downloadJobState: DownloadJobState): Status? {
synchronized(context) {
return downloadJobState.status
return downloadJobState.state.status
}
}

/**
* Status of an ongoing download
*/
enum class DownloadJobStatus {
ACTIVE,
PAUSED,
CANCELLED,
FAILED,
COMPLETED
}

internal val broadcastReceiver by lazy {
object : BroadcastReceiver() {
@Suppress("LongMethod")
Expand All @@ -164,14 +147,14 @@ abstract class AbstractFetchDownloadService : Service() {

when (intent.action) {
ACTION_PAUSE -> {
setDownloadJobStatus(currentDownloadJobState, PAUSED)
setDownloadJobStatus(currentDownloadJobState, Status.PAUSED)
currentDownloadJobState.job?.cancel()
emitNotificationPauseFact()
logger.debug("ACTION_PAUSE for ${currentDownloadJobState.state.url}")
}

ACTION_RESUME -> {
setDownloadJobStatus(currentDownloadJobState, ACTIVE)
setDownloadJobStatus(currentDownloadJobState, Status.DOWNLOADING)

currentDownloadJobState.job = CoroutineScope(IO).launch {
startDownloadJob(currentDownloadJobState)
Expand All @@ -184,7 +167,7 @@ abstract class AbstractFetchDownloadService : Service() {
ACTION_CANCEL -> {
removeNotification(context, currentDownloadJobState)
currentDownloadJobState.lastNotificationUpdate = System.currentTimeMillis()
setDownloadJobStatus(currentDownloadJobState, CANCELLED)
setDownloadJobStatus(currentDownloadJobState, Status.CANCELLED)
currentDownloadJobState.job?.cancel()

currentDownloadJobState.job = CoroutineScope(IO).launch {
Expand All @@ -200,7 +183,7 @@ abstract class AbstractFetchDownloadService : Service() {
ACTION_TRY_AGAIN -> {
removeNotification(context, currentDownloadJobState)
currentDownloadJobState.lastNotificationUpdate = System.currentTimeMillis()
setDownloadJobStatus(currentDownloadJobState, ACTIVE)
setDownloadJobStatus(currentDownloadJobState, Status.DOWNLOADING)

currentDownloadJobState.job = CoroutineScope(IO).launch {
startDownloadJob(currentDownloadJobState)
Expand Down Expand Up @@ -258,10 +241,12 @@ abstract class AbstractFetchDownloadService : Service() {
// Create a new job and add it, with its downloadState to the map
val downloadJobState = DownloadJobState(
state = download,
foregroundServiceId = foregroundServiceId,
status = ACTIVE
foregroundServiceId = foregroundServiceId
)

val newState = downloadJobState.state.copy(status = Status.DOWNLOADING)
updateDownloadState(newState)

downloadJobState.job = CoroutineScope(IO).launch {
startDownloadJob(downloadJobState)
}
Expand Down Expand Up @@ -302,7 +287,7 @@ abstract class AbstractFetchDownloadService : Service() {
// Dispatch the corresponding notification based on the current status
updateDownloadNotification(uiStatus, download)

if (uiStatus != ACTIVE) {
if (uiStatus != Status.DOWNLOADING) {
sendDownloadStopped(download)
}
}
Expand All @@ -314,20 +299,23 @@ abstract class AbstractFetchDownloadService : Service() {
* from another thread, causing inconsistencies in the ui.
*/
@VisibleForTesting
internal fun updateDownloadNotification(latestUIStatus: DownloadJobStatus, download: DownloadJobState) {
internal fun updateDownloadNotification(latestUIStatus: Status?, download: DownloadJobState) {
val notification = when (latestUIStatus) {
ACTIVE -> DownloadNotification.createOngoingDownloadNotification(context, download)
PAUSED -> DownloadNotification.createPausedDownloadNotification(context, download)
FAILED -> DownloadNotification.createDownloadFailedNotification(context, download)
COMPLETED -> {
Status.DOWNLOADING -> DownloadNotification.createOngoingDownloadNotification(context, download)
Status.PAUSED -> DownloadNotification.createPausedDownloadNotification(context, download)
Status.FAILED -> DownloadNotification.createDownloadFailedNotification(context, download)
Status.COMPLETED -> {
addToDownloadSystemDatabaseCompat(download.state)
DownloadNotification.createDownloadCompletedNotification(context, download)
}
CANCELLED -> {
Status.CANCELLED -> {
removeNotification(context, download)
download.lastNotificationUpdate = System.currentTimeMillis()
null
}
else -> {
null
}
}

notification?.let {
Expand Down Expand Up @@ -372,7 +360,7 @@ abstract class AbstractFetchDownloadService : Service() {
performDownload(currentDownloadJobState)
} catch (e: Exception) {
logger.error("Unable to complete download for ${currentDownloadJobState.state.url} marked as FAILED", e)
setDownloadJobStatus(currentDownloadJobState, FAILED)
setDownloadJobStatus(currentDownloadJobState, Status.FAILED)
}
}

Expand Down Expand Up @@ -513,7 +501,7 @@ abstract class AbstractFetchDownloadService : Service() {
* it always deletes the previous notification :(
*/
previousDownload?.let {
updateDownloadNotification(previousDownload.status, it)
updateDownloadNotification(previousDownload.state.status, it)
}
}

Expand All @@ -530,11 +518,11 @@ abstract class AbstractFetchDownloadService : Service() {
* the foreground notification id, when the previous one gets a state that
* is likely to be dismissed.
*/
val status = download.status
val status = download.state.status
val foregroundId = download.foregroundServiceId
val isSelectedForegroundId = compatForegroundNotificationId == foregroundId
val needNewForegroundNotification = when (status) {
COMPLETED, FAILED, CANCELLED -> true
Status.COMPLETED, Status.FAILED, Status.CANCELLED -> true
else -> false
}

Expand All @@ -544,7 +532,8 @@ abstract class AbstractFetchDownloadService : Service() {
stopForeground(false)

// Now we need to find a new foreground notification, if needed.
val newSelectedForegroundDownload = downloadJobs.values.firstOrNull { it.status == ACTIVE }
val newSelectedForegroundDownload =
downloadJobs.values.firstOrNull { it.state.status == Status.DOWNLOADING }
newSelectedForegroundDownload?.let {
setForegroundNotification(it)
}
Expand All @@ -558,11 +547,11 @@ abstract class AbstractFetchDownloadService : Service() {
@Suppress("ComplexCondition")
internal fun performDownload(currentDownloadJobState: DownloadJobState) {
val download = currentDownloadJobState.state
val isResumingDownload = currentDownloadJobState.currentBytesCopied > 0L
val isResumingDownload = currentDownloadJobState.state.currentBytesCopied > 0L
val headers = MutableHeaders()

if (isResumingDownload) {
headers.append(RANGE, "bytes=${currentDownloadJobState.currentBytesCopied}-")
headers.append(RANGE, "bytes=${currentDownloadJobState.state.currentBytesCopied}-")
}

val request = Request(download.url.sanitizeURL(), headers = headers)
Expand All @@ -574,8 +563,9 @@ abstract class AbstractFetchDownloadService : Service() {
if (response.status != PARTIAL_CONTENT_STATUS && response.status != OK_STATUS ||
(isResumingDownload && !response.headers.contains(CONTENT_RANGE))) {
// We experienced a problem trying to fetch the file, send a failure notification
currentDownloadJobState.currentBytesCopied = 0
setDownloadJobStatus(currentDownloadJobState, FAILED)
val newState = currentDownloadJobState.state.copy(currentBytesCopied = 0)
updateDownloadState(newState)
setDownloadJobStatus(currentDownloadJobState, Status.FAILED)
logger.debug("Unable to fetching Download for ${currentDownloadJobState.state.url} status FAILED")
return
}
Expand All @@ -596,43 +586,45 @@ abstract class AbstractFetchDownloadService : Service() {
* Updates the status of an ACTIVE download to completed or failed based on bytes copied
*/
internal fun verifyDownload(download: DownloadJobState) {
if (getDownloadJobStatus(download) == DownloadJobStatus.ACTIVE &&
download.currentBytesCopied < download.state.contentLength ?: 0) {
setDownloadJobStatus(download, FAILED)
if (getDownloadJobStatus(download) == Status.DOWNLOADING &&
download.state.currentBytesCopied < download.state.contentLength ?: 0) {
setDownloadJobStatus(download, Status.FAILED)
logger.error("verifyDownload for ${download.state.url} FAILED")
} else if (getDownloadJobStatus(download) == DownloadJobStatus.ACTIVE) {
setDownloadJobStatus(download, COMPLETED)
} else if (getDownloadJobStatus(download) == Status.DOWNLOADING) {
setDownloadJobStatus(download, Status.COMPLETED)
/**
* In cases when we don't get the file size provided initially, we have to
* use downloadState.currentBytesCopied as a fallback.
*/
val fileSizeNotFound = download.state.contentLength == null || download.state.contentLength == 0L
if (fileSizeNotFound) {
val newState = download.state.copy(contentLength = download.currentBytesCopied)
val newState = download.state.copy(contentLength = download.state.currentBytesCopied)
updateDownloadState(newState)
}
logger.debug("verifyDownload for ${download.state.url} ${download.status}")
logger.debug("verifyDownload for ${download.state.url} ${download.state.status}")
}
}

private fun copyInChunks(downloadJobState: DownloadJobState, inStream: InputStream, outStream: OutputStream) {
val data = ByteArray(CHUNK_SIZE)
logger.debug("starting copyInChunks ${downloadJobState.state.url}" +
" currentBytesCopied ${downloadJobState.currentBytesCopied}")
" currentBytesCopied ${downloadJobState.state.currentBytesCopied}")

// To ensure that we copy all files (even ones that don't have fileSize, we must NOT check < fileSize
while (getDownloadJobStatus(downloadJobState) == DownloadJobStatus.ACTIVE) {
while (getDownloadJobStatus(downloadJobState) == Status.DOWNLOADING) {
val bytesRead = inStream.read(data)

// If bytesRead is -1, there's no data left to read from the stream
if (bytesRead == -1) { break }

downloadJobState.currentBytesCopied += bytesRead
val newState = downloadJobState.state.copy(currentBytesCopied =
downloadJobState.state.currentBytesCopied + bytesRead)
updateDownloadState(newState)

outStream.write(data, 0, bytesRead)
}
logger.debug("Finishing copyInChunks ${downloadJobState.state.url} " +
"currentBytesCopied ${downloadJobState.currentBytesCopied}")
"currentBytesCopied ${downloadJobState.state.currentBytesCopied}")
}

/**
Expand Down
Loading

0 comments on commit fde00d5

Please sign in to comment.