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

Move all download state to browser store #7050

Closed
csadilek opened this issue May 20, 2020 · 10 comments
Closed

Move all download state to browser store #7050

csadilek opened this issue May 20, 2020 · 10 comments
Labels
⌨️ code Technical debt, code clean up, small API change .. <download> Component: feature-download E13 Estimation points: 13

Comments

@csadilek
Copy link
Contributor

csadilek commented May 20, 2020

We currently keep additional application state for downloads in the DownloadService (see downloadJobs) and DownloadManager (see queuedDownloads).

Having all this state in the store will make it easier to implement additional download UI in the future. It also solves the problem described in #5217 where we have to pass the download object (and the full URL) to the download intent which may be too large in case of data URIs. If the service has access to the store it can simply read the state from there.

┆Issue is synchronized with this Jira Task

@csadilek csadilek added ⌨️ code Technical debt, code clean up, small API change .. <download> Component: feature-download labels May 20, 2020
@csadilek
Copy link
Contributor Author

csadilek commented May 20, 2020

This is a bigger refactoring and may need to be broken down more once work starts.

@csadilek
Copy link
Contributor Author

First part here is done in: #7103

@csadilek csadilek added E13 Estimation points: 13 and removed E21 Estimation points: 21 labels May 26, 2020
bors bot pushed a commit that referenced this issue May 28, 2020
7136: Closes #7103 #5217: DownloadService: Read initial download state from store r=Amejia481 a=csadilek

This moves the queued download state from the manager to the store, which cleans things up and also fixes #5217.

Keeping the changeset as small as possible so we can continue in #7050 and move the remaining state as well.

Tested various cases will do more now and also open PRs for R-B and Fenix.

Co-authored-by: Christian Sadilek <[email protected]>
@codrut-topliceanu
Copy link
Contributor

codrut-topliceanu commented Jun 9, 2020

Changes made for this have affected the fix for mozilla-mobile/fenix#9044 . I'll take a look on the Fenix side and see how I can resolve those conflicts.

Edit: Or should I wait for this to be finished?

@Amejia481
Copy link
Contributor

Changes made for this have affected the fix for mozilla-mobile/fenix#9044 . I'll take a look on the Fenix side and see how I can resolve those conflicts.

Edit: Or should I wait for this to be finished?

I think you could continue on Fenix, Is there something in particular that it's blocking you?

@codrut-topliceanu
Copy link
Contributor

@Amejia481 Yeah, you're right, if the DownloadState move has been completed, I should be able to make the needed changes in mozilla-mobile/fenix#11371 without any conflicts from the proposed changes in #7227 .

@marekchen
Copy link

I want to add a Download Manager Center Activity in my browser powered by android-components.
I want show all download tasks' progress in my activity.But now DownloadState in BrowserState's queuedDownloads don't include download progress.
So I can wait this change to be done.Then I can get download progress from BrowserState?
English is not my mother tongue, if you have any doubt about what I say,please comment me. Thank you very much.

@Amejia481
Copy link
Contributor

I want to add a Download Manager Center Activity in my browser powered by android-components.
I want show all download tasks' progress in my activity.But now DownloadState in BrowserState's queuedDownloads don't include download progress.
So I can wait this change to be done.Then I can get download progress from BrowserState?
English is not my mother tongue, if you have any doubt about what I say,please comment me. Thank you very much.

We are going to work on a similar feature on Fenix mozilla-mobile/fenix#349
This looks like a great starting issue.

cc @kglazko

@Amejia481
Copy link
Contributor

I filed #7673 for it

@pocmo pocmo added this to the Triage Archive milestone Jul 27, 2020
@Amejia481
Copy link
Contributor

I want to add a Download Manager Center Activity in my browser powered by android-components.
I want show all download tasks' progress in my activity.But now DownloadState in BrowserState's queuedDownloads don't include download progress.
So I can wait this change to be done.Then I can get download progress from BrowserState?
English is not my mother tongue, if you have any doubt about what I say,please comment me. Thank you very much.

@marcinwiacek now #7698 landed this should include the values that you need. We are going to work on #7761 next as we want downloads to be on memory in the store even if they are completed, after that we want to work on #7762 for this data to be persisted on disk.

@data-sync-user data-sync-user changed the title Move all download state to browser store FNX3-22721 ⁃ Move all download state to browser store Aug 4, 2020
@data-sync-user data-sync-user changed the title FNX3-22721 ⁃ Move all download state to browser store FNX2-18156 ⁃ Move all download state to browser store Aug 5, 2020
@st3fan st3fan changed the title FNX2-18156 ⁃ Move all download state to browser store Move all download state to browser store Aug 5, 2020
@pocmo pocmo removed this from the Triage Archive milestone Oct 8, 2020
@jonalmeida
Copy link
Contributor

We have downloads in the browser store now, so I'm going to close this. 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⌨️ code Technical debt, code clean up, small API change .. <download> Component: feature-download E13 Estimation points: 13
Projects
None yet
Development

No branches or pull requests

6 participants