New issue
Advanced search Search tips

Issue 855856 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ScheduleDownload shouldn't resume paused jobs

Project Member Reported by na...@chromium.org, Jun 23 2018

Issue description

This is a recently introduced regression.

Repro steps:
Start a fetch containing multiple downloads. Pause it through the UI.

Expected behavior: The fetch remains paused until we hit Resume.
Observed behavior: The fetch auto-resumes.

I think it's a bug in our ScheduleDownload logic, where we schedule one whenever there are fewer than max_concurrent_downloads active downloads.
Since we are pausing all downloads correctly in our PauseDownload() logic, we should simply check if a download has been paused when scheduling one.

In the future, we might need to add reason_for_pause, if we want to support auto-resumes on wifi connectivity.
 
Owner: na...@chromium.org
Status: Assigned (was: Available)
Status: Available (was: Assigned)
Labels: BlocksMVP
Owner: ----
Owner: rayankans@chromium.org
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76e026f167fb2eadd321df5ba234db43dd5fe792

commit 76e026f167fb2eadd321df5ba234db43dd5fe792
Author: Rayan Kanso <rayankans@chromium.org>
Date: Fri Sep 28 14:35:38 2018

[Background Fetch] Add a pause state for the job details.

This solves the issue of auto-resuming notifications. OnItemUpdated was
being called after the notification was paused with an offline item
containing an out-of-sync status.

There is also a race condition between pausing the job (OIC) and the
download completing (DS), so new downlaods need to be throttled in case
the job is paused.

Bug:  855856 
Change-Id: I537789e7b1607fc4ce69b9cc7593a6499152475c
Reviewed-on: https://chromium-review.googlesource.com/1241157
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595083}
[modify] https://crrev.com/76e026f167fb2eadd321df5ba234db43dd5fe792/chrome/browser/background_fetch/background_fetch_browsertest.cc
[modify] https://crrev.com/76e026f167fb2eadd321df5ba234db43dd5fe792/chrome/browser/background_fetch/background_fetch_delegate_impl.cc
[modify] https://crrev.com/76e026f167fb2eadd321df5ba234db43dd5fe792/chrome/browser/background_fetch/background_fetch_delegate_impl.h

Status: Fixed (was: Started)

Sign in to add a comment