Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Background fetch #279

Closed
1 of 3 tasks
jakearchibald opened this issue May 2, 2018 · 16 comments
Closed
1 of 3 tasks

Background fetch #279

jakearchibald opened this issue May 2, 2018 · 16 comments
Assignees
Labels
Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review

Comments

@jakearchibald
Copy link

jakearchibald commented May 2, 2018

Hello TAG!

I'm requesting a TAG review of:

You should also know that...

The spec isn't complete yet, so this is more of an opportunity to review the proposed API.

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@slightlyoff
Copy link
Member

My understanding is there's an implementation underway. Is there really no time pressure and/or deadline you'd like us to work under?

@jakearchibald
Copy link
Author

We're aiming to send an intend to ship in July/August, so it's a little more pressing that I initially thought.

@annevk
Copy link
Member

annevk commented May 18, 2018

How can you ship when the draft has substantive issues, such as the one below "response failed due to a failed CORS check"? (I don't think we should expose such a failure to content.)

@jakearchibald
Copy link
Author

I do need to get a move on with the spec 😢. Trying to devote more time to it.

@torgo torgo added this to the 2018-05-29-telcon milestone May 22, 2018
@travisleithead travisleithead self-assigned this May 22, 2018
@cynthia
Copy link
Member

cynthia commented Oct 31, 2018

Seems like the explainer switched branches: https://github.com/WICG/background-fetch/blob/master/README.md

@travisleithead
Copy link
Contributor

Hi! Just getting my feet wet with this spec, so I apologies if my mental map is incomplete... Here's a few points of discussion I found in my first read:

The optional downloadTotal dictionary member misled me by it's name. I think having it called something like doNotExceed would better map to the use case than a pre-predicted estimate of the download total (and then leave the UI stuff to the UA to figure out (which it should know by the server responses, etc.)

The registration.backgoundFetch object is similar to a map in many respects. I wonder if some APIs could align better? E.g., ids are the keys to the various ongoing fetches. getIds() is like keys()... but set()` isn't a great match for initiating a fetch so I'm less convinced to go all the way aligning the names :).

In the spirit of not needing to re-activate the browser code to do UI updates, I'd like to suggest we drop updateUI in favor of pre-declaring these strings at the time of the background fetch start. Probably you just need to care about three states: downloading (icon, title), finished (icon, title), and error/canceled (icon, title). It's not as generally powerful, but can make the UI transitions more smooth by avoiding the round-trip to the client for the various state transitions. (Also the download/upload states aren't really expected to change in an ad-hoc manner.)

@plinss plinss added Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review and removed Paris2018f2f labels Oct 31, 2018
@jakearchibald
Copy link
Author

The optional downloadTotal dictionary member misled me by it's name. I think having it called something like doNotExceed

doNotExceed kinda loses context. Eg, does it include upload bandwidth?

(and then leave the UI stuff to the UA to figure out (which it should know by the server responses, etc.)

If you're downloading a DASH-style movie it could be 1000s of resources. It's going to take a long time for the browser to know much about the size.

The registration.backgoundFetch object is similar to a map in many respects.

The main difference is it's async. I'd like to make it an async iterator at some point.

In the spirit of not needing to re-activate the browser code to do UI updates,

The browser needs reactivated anyway so the responses can be handled/read/moved. I kinda like the idea of specifying this stuff ahead of time, but it feels like it loses functionality for not much gain.

@webmaxru
Copy link

I've just published an article about my exploration of what we have in Chrome Canary M72 at the moment: https://medium.com/@webmaxru/background-fetch-api-get-ready-to-use-it-69cca522cd8f

@webmaxru
Copy link

From this post: "The downloadTotal property we pass during the background fetch registration is the total number of bytes of the whole set. It’s also optional. If passed, it works as a guard — we’ll get backgroundfetcherror event in the service worker if the total size of the resources is larger. Also, it’s a good helper for our UI to show the correct download percentage if needed. Unfortunately, there is another bug: at least on Mac OS this number can’t be larger than 2147483647 (2 GB) which is the maximum size of one file in the file system but it’s intended to be the maximum total size of the whole set. I’m going to submit a bug to the Chromium Bug Tracker about this. Meanwhile, if the total set size doesn’t bypass 2GB (like in our case) our calculations are correct."

But I might be wrong in my assumptions :)

@jakearchibald
Copy link
Author

jakearchibald commented Nov 30, 2018

Your assumptions are correct. The spec defines the maximum of downloadTotal to be 18,446,744,073,709,551,614, so it's a Chrome bug.

@jakearchibald
Copy link
Author

@webmaxru you might also be interested in https://bgfetch-http203.glitch.me/.

@webmaxru
Copy link

@webmaxru you might also be interested in https://bgfetch-http203.glitch.me/.

Hard to imagine the better usecase! I plan to build a PoC of bgfetch automation tool, maybe this will end up as a plugin for Workbox - will use podcast app as a guinea pig :)

@kenchris
Copy link

@travisleithead
Copy link
Contributor

Have re-read the spec and associated resources, and I'm feeling more comfortable with the design ;-).

In the explainer you note that the UA should take steps to ensure that the content that originates from the web app is distinctly separate from the trusted UI elements as rendered by the UA. Another concern that led to browsers all-but-removing the rendering of beforeunload's status message, is that fields like "title" are potential attack surface. May want to make of note of that in the Security and Privacy section of the spec as well to be sure implementors are aware.

@travisleithead
Copy link
Contributor

Have filed the above issue. All others on the call today supported closing the issue. Thanks so much for the TAG review, and we hope to be of service again soon (Come on back, ya'll!)

@jakearchibald
Copy link
Author

Thanks TAG! The review was genuinely useful & I'm looking forward to the next one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review
Projects
None yet
Development

No branches or pull requests

9 participants