Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Revert "Closes #5284: Adds progress bar to downloads" #5427

Merged
merged 1 commit into from
Dec 30, 2019
Merged

Revert "Closes #5284: Adds progress bar to downloads" #5427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 53 additions & 98 deletions ...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import android.os.IBinder
import android.os.ParcelFileDescriptor
import android.provider.MediaStore
import android.widget.Toast
import androidx.annotation.GuardedBy
import androidx.annotation.VisibleForTesting
import androidx.core.app.NotificationManagerCompat
import androidx.core.content.FileProvider
Expand All @@ -31,10 +30,6 @@ import androidx.localbroadcastmanager.content.LocalBroadcastManager
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers.IO
import kotlinx.coroutines.Job
import kotlinx.coroutines.MainScope
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.concept.fetch.Client
Expand Down Expand Up @@ -67,8 +62,6 @@ import kotlin.random.Random
@Suppress("TooManyFunctions", "LargeClass")
abstract class AbstractFetchDownloadService : Service() {

private val notificationUpdateScope = MainScope()

protected abstract val httpClient: Client
@VisibleForTesting
internal val broadcastManager by lazy { LocalBroadcastManager.getInstance(this) }
Expand All @@ -81,7 +74,7 @@ abstract class AbstractFetchDownloadService : Service() {
var job: Job? = null,
@Volatile var state: DownloadState,
var currentBytesCopied: Long = 0,
@GuardedBy("context") var status: DownloadJobStatus,
@Volatile var status: DownloadJobStatus,
var foregroundServiceId: Int = 0,
var downloadDeleted: Boolean = false
)
Expand All @@ -99,25 +92,20 @@ abstract class AbstractFetchDownloadService : Service() {

internal val broadcastReceiver by lazy {
object : BroadcastReceiver() {
@Suppress("LongMethod")
override fun onReceive(context: Context, intent: Intent?) {
val downloadId =
intent?.extras?.getLong(DownloadNotification.EXTRA_DOWNLOAD_ID) ?: return
val currentDownloadJobState = downloadJobs[downloadId] ?: return

when (intent.action) {
ACTION_PAUSE -> {
synchronized(context) {
currentDownloadJobState.status = DownloadJobStatus.PAUSED
}
currentDownloadJobState.status = DownloadJobStatus.PAUSED
currentDownloadJobState.job?.cancel()
emitNotificationPauseFact()
}

ACTION_RESUME -> {
synchronized(context) {
currentDownloadJobState.status = DownloadJobStatus.ACTIVE
}
currentDownloadJobState.status = DownloadJobStatus.ACTIVE

currentDownloadJobState.job = CoroutineScope(IO).launch {
startDownloadJob(downloadId)
Expand All @@ -130,9 +118,8 @@ abstract class AbstractFetchDownloadService : Service() {
NotificationManagerCompat.from(context).cancel(
currentDownloadJobState.foregroundServiceId
)
synchronized(context) {
currentDownloadJobState.status = DownloadJobStatus.CANCELLED
}
currentDownloadJobState.status = DownloadJobStatus.CANCELLED

currentDownloadJobState.job?.cancel()

currentDownloadJobState.job = CoroutineScope(IO).launch {
Expand All @@ -147,10 +134,7 @@ abstract class AbstractFetchDownloadService : Service() {
NotificationManagerCompat.from(context).cancel(
currentDownloadJobState.foregroundServiceId
)

synchronized(context) {
currentDownloadJobState.status = DownloadJobStatus.ACTIVE
}
currentDownloadJobState.status = DownloadJobStatus.ACTIVE

currentDownloadJobState.job = CoroutineScope(IO).launch {
startDownloadJob(downloadId)
Expand Down Expand Up @@ -198,63 +182,12 @@ abstract class AbstractFetchDownloadService : Service() {
startDownloadJob(download.id)
}

notificationUpdateScope.launch {
while (isActive) {
delay(PROGRESS_UPDATE_INTERVAL)
updateDownloadNotification()
}
}

return super.onStartCommand(intent, flags, startId)
}

/***
* Android rate limits notifications being sent, so we must send them on a delay so that
* notifications are not dropped
*/
private fun updateDownloadNotification() {
synchronized(context) {
for (download in downloadJobs.values) {
// Dispatch the corresponding notification based on the current status
val notification = when (download.status) {
DownloadJobStatus.ACTIVE -> {
DownloadNotification.createOngoingDownloadNotification(
context,
download.state,
download.currentBytesCopied
)
}

DownloadJobStatus.PAUSED -> {
DownloadNotification.createPausedDownloadNotification(context, download.state)
}

DownloadJobStatus.COMPLETED -> {
DownloadNotification.createDownloadCompletedNotification(context, download.state)
}

DownloadJobStatus.FAILED -> {
DownloadNotification.createDownloadFailedNotification(context, download.state)
}

DownloadJobStatus.CANCELLED -> { return }
}

NotificationManagerCompat.from(context).notify(
download.foregroundServiceId,
notification
)

if (download.status != DownloadJobStatus.ACTIVE) {
sendDownloadNotInProgress(download.state.id, download.status)
}
}
}
}

override fun onTaskRemoved(rootIntent: Intent?) {
// If the task gets removed (app gets swiped away in the task switcher) our process and this
// service gets killed. In this situation we remove the notification since the download will
// servies gets killed. In this situation we remove the notification since the download will
// stop and cannot be controlled via the notification anymore (until we persist enough data
// to resume downloads from a new process).

Expand All @@ -271,18 +204,40 @@ abstract class AbstractFetchDownloadService : Service() {
downloadJobs.values.forEach {
it.job?.cancel()
}

notificationUpdateScope.cancel()
}

internal fun startDownloadJob(downloadId: Long) {
val currentDownloadJobState = downloadJobs[downloadId] ?: return

try {
val notification = try {
performDownload(currentDownloadJobState.state)
when (currentDownloadJobState.status) {
DownloadJobStatus.PAUSED -> {
DownloadNotification.createPausedDownloadNotification(context, currentDownloadJobState.state)
}

DownloadJobStatus.ACTIVE -> {
currentDownloadJobState.status = DownloadJobStatus.COMPLETED
DownloadNotification.createDownloadCompletedNotification(context, currentDownloadJobState.state)
}

DownloadJobStatus.FAILED -> {
DownloadNotification.createDownloadFailedNotification(context, currentDownloadJobState.state)
}

else -> return
}
} catch (e: IOException) {
currentDownloadJobState.status = DownloadJobStatus.FAILED
DownloadNotification.createDownloadFailedNotification(context, currentDownloadJobState.state)
}

NotificationManagerCompat.from(context).notify(
currentDownloadJobState.foregroundServiceId,
notification
)

sendDownloadCompleteBroadcast(downloadId, currentDownloadJobState.status)
}

internal fun deleteDownloadingFile(downloadState: DownloadState) {
Expand All @@ -302,10 +257,21 @@ abstract class AbstractFetchDownloadService : Service() {
context.registerReceiver(broadcastReceiver, filter)
}

private fun displayOngoingDownloadNotification(download: DownloadState) {
val ongoingDownloadNotification = DownloadNotification.createOngoingDownloadNotification(
context,
download
)

NotificationManagerCompat.from(context).notify(
downloadJobs[download.id]?.foregroundServiceId ?: 0,
ongoingDownloadNotification
)
}

@Suppress("ComplexCondition")
internal fun performDownload(download: DownloadState) {
val currentBytes = downloadJobs[download.id]?.currentBytesCopied ?: 0L
val isResumingDownload = currentBytes > 0L
val isResumingDownload = downloadJobs[download.id]?.currentBytesCopied ?: 0L > 0L
val headers = MutableHeaders()

if (isResumingDownload) {
Expand All @@ -329,6 +295,8 @@ abstract class AbstractFetchDownloadService : Service() {
val newDownloadState = download.withResponse(response.headers, inStream)
downloadJobs[download.id]?.state = newDownloadState

displayOngoingDownloadNotification(newDownloadState)

useFileStream(newDownloadState, isResumingDownload) { outStream ->
copyInChunks(downloadJobs[download.id]!!, inStream, outStream)
}
Expand All @@ -337,21 +305,14 @@ abstract class AbstractFetchDownloadService : Service() {
}
}

/**
* Updates the status of an ACTIVE download to completed or failed based on bytes copied
*/
internal fun verifyDownload(download: DownloadJobState) {
synchronized(context) {
if (download.status == DownloadJobStatus.ACTIVE &&
download.currentBytesCopied < download.state.contentLength ?: 0) {
download.status = DownloadJobStatus.FAILED
} else if (download.status == DownloadJobStatus.ACTIVE) {
download.status = DownloadJobStatus.COMPLETED
}
if (download.status == DownloadJobStatus.ACTIVE &&
download.currentBytesCopied < download.state.contentLength ?: 0) {
download.status = DownloadJobStatus.FAILED
}
}

internal fun copyInChunks(downloadJobState: DownloadJobState, inStream: InputStream, outStream: OutputStream) {
private fun copyInChunks(downloadJobState: DownloadJobState, inStream: InputStream, outStream: OutputStream) {
// To ensure that we copy all files (even ones that don't have fileSize, we must NOT check < fileSize
while (downloadJobState.status == DownloadJobStatus.ACTIVE) {
val data = ByteArray(CHUNK_SIZE)
Expand All @@ -368,9 +329,9 @@ abstract class AbstractFetchDownloadService : Service() {

/**
* Informs [mozilla.components.feature.downloads.manager.FetchDownloadManager] that a download
* is no longer in progress due to being paused, completed, or failed
* has been completed.
*/
private fun sendDownloadNotInProgress(downloadID: Long, status: DownloadJobStatus) {
private fun sendDownloadCompleteBroadcast(downloadID: Long, status: DownloadJobStatus) {
val download = downloadJobs[downloadID]?.state ?: return

val intent = Intent(ACTION_DOWNLOAD_COMPLETE)
Expand Down Expand Up @@ -496,12 +457,6 @@ abstract class AbstractFetchDownloadService : Service() {
private const val CHUNK_SIZE = 4 * 1024
private const val PARTIAL_CONTENT_STATUS = 206
private const val OK_STATUS = 200
/**
* This interval was decided on by balancing the limit of the system (200ms) and allowing
* users to press buttons on the notification. If a new notification is presented while a
* user is tapping a button, their press will be cancelled.
*/
private const val PROGRESS_UPDATE_INTERVAL = 750L

const val EXTRA_DOWNLOAD = "mozilla.components.feature.downloads.extras.DOWNLOAD"
const val EXTRA_DOWNLOAD_STATUS = "mozilla.components.feature.downloads.extras.DOWNLOAD_STATUS"
Expand Down
17 changes: 3 additions & 14 deletions ...ture/downloads/src/main/java/mozilla/components/feature/downloads/DownloadNotification.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,34 +30,23 @@ internal object DownloadNotification {

private const val NOTIFICATION_CHANNEL_ID = "mozac.feature.downloads.generic"
private const val LEGACY_NOTIFICATION_CHANNEL_ID = "Downloads"
private const val PERCENTAGE_MULTIPLIER = 100

internal const val EXTRA_DOWNLOAD_ID = "downloadId"

/**
* Build the notification to be displayed while the download service is active.
*/
fun createOngoingDownloadNotification(
context: Context,
downloadState: DownloadState,
bytesCopied: Long
): Notification {
fun createOngoingDownloadNotification(context: Context, downloadState: DownloadState): Notification {
val channelId = ensureChannelExists(context)
val fileSizeText = (downloadState.contentLength?.toMegabyteString() ?: "")
val isIndeterminate = downloadState.contentLength == null
val contentText = if (isIndeterminate) {
fileSizeText
} else {
"${PERCENTAGE_MULTIPLIER * bytesCopied / downloadState.contentLength!!}%"
}

return NotificationCompat.Builder(context, channelId)
.setSmallIcon(R.drawable.mozac_feature_download_ic_ongoing_download)
.setContentTitle(downloadState.fileName)
.setContentText(contentText)
.setContentText(fileSizeText)
.setColor(ContextCompat.getColor(context, R.color.mozac_feature_downloads_notification))
.setCategory(NotificationCompat.CATEGORY_PROGRESS)
.setProgress(downloadState.contentLength?.toInt() ?: 0, bytesCopied.toInt(), isIndeterminate)
.setProgress(1, 0, true)
.setOngoing(true)
.addAction(getPauseAction(context, downloadState.id))
.addAction(getCancelAction(context, downloadState.id))
Expand Down
Loading