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

Budget API (especially reserve() method) #169

Closed
owencm opened this Issue Apr 14, 2017 · 7 comments

Comments

Projects
None yet
8 participants
@owencm

owencm commented Apr 14, 2017

Hello TAG!

I'm requesting a TAG review of: Budget API, with a focus on the reserve() method

Further details:

  • Relevant time constraints or deadlines: Ideally aiming to get some public signals from TAG by Wednesday 4/19 as Blink API owners have requested this to unblock shipping, and if we get them by then we can ship in Chrome 59 rather than punting to 60.

You should also know that at this stage we are only proposing to ship the reserve() method and intend on exposing the other methods only experimentally as part of our Origin Trials system.

Hence, we're specifically most interested in the reserve() method and whether it is decoupled sufficiently that we feel good about shipping it even as we wish to learn more about other parts of the 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]

Thanks very much!

@foolip

This comment has been minimized.

foolip commented Apr 18, 2017

On the Intent to Ship: The Budget API’s reserve() method
on blink-dev I said that "By shipping navigator.budget.reserve("silent-push"), we are anticipating that we will eventually want to ship navigator.budget.reserve("something-else") and navigator.budget.somethingElse(), or the shape of the API would be different."

So, while reserve() is the bit of immediate interest, I believe the bit that is most in need of pondering is the dependency on https://wicg.github.io/budget-api/#user-engagement, and the implication that all background tasks would drawn from this shared budget.

@slightlyoff

This comment has been minimized.

Member

slightlyoff commented Apr 18, 2017

A few thoughts as others haven't had a chance to weigh in on this; will open issues (as requested) once others here look:

  • As always, please link your explainer or use-cases doc from your spec.
  • This API doesn't seem to have an explicit explainer; the README is all I could find.
  • There isn't any sort of "observer" or event to let an app know about budget changes (e.g., when you run out of budget for something). Is that planned?
  • The spec is defining another registry-alike. This is problematic. It seems as though this should either be part of the Permissions API or the registry of names unified with it.
  • The README doesn't discuss considered and rejected alternative designs (e.g., direct integration into the Permissions API)
  • The second example in the README shows how to understand if an operation can be afforded...why not provide a convenience method for this along with the existing (lower-level) functionality?
  • It's surprising that reserve() doesn't also report the amount of remaining budget on success
  • Why are individual methods scoped to secure origins and not the entire API?
  • Aside from precise-timer (speculative) and silent-push, what other APIs are anticipated to integrate with this API? Should the TAG be looking to encourage existing APIs to add budgets? Which ones?
  • Budget (via getBudget()) appears to be global and not a per-operation-type budget. Have you consulted with other browsers to ensure that this is a reasonable design choice for their implementations?
  • What error will be provided for unsupported operations?

/cc @beverloo @owencm

@owencm

This comment has been minimized.

owencm commented Apr 20, 2017

Thanks Alex! Answers below (including a couple TODOs for us and questions for peter)

  1. TODO: Linking to the explainer and use cases
  2. That readme is the explainer, would renaming it that solve this?
  3. I'm not aware of any plan to add an observer/event for when budget changes. I imagine that could be useful for a site that gains some budget and can now schedule a background update. Peter - WDYT?
  4. The spec says that operation names should be consistent with Permissions API where they are the same, but for those that don't make sense in the Permissions API is there a recommendation for a place for such a registry to live? Could we define this spec as the registry for such background actions?
  5. TODO: Mention some other designs we considered in the explainer
  6. It seems overly specific to me for us to define convenience function for the # of a single given operation that can be reserved in the future at a given time. It's also not complex to do and people may want to instead know how they can combine various operations etc which complicates things, so I suggest we don't add anything there.
  7. Returning remaining budget in calls to reserve complicates the return value which is a simple boolean today, and I'm not sure it's worth it given how easy it is to just call navigator.budget.getBudget()
  8. ¯_(ツ)_/¯ to why this API isn't marked as available on secure origins only. Peter - is there any reason to allow this on insecure origins?
  9. RE: which other operations/APIs are expected to be supported - I don't think we need to be proactively looking to have other specs integrate, unless they are defining some kind of ability for a site to run some code in the background while the user's not on the page. In the future we could consider expanding this concept to cover other non-background resource allocation, but I think that's a non-goal for now.
  10. RE: making a per-operation-type budget instead of a global budget, do you have thoughts on this Peter?
  11. TODO: define what error will be provided for unsupported operations

Meta-comment: it was fairly hard to go through this feedback and reply to it, this format seems like a fairly challenging one for having these discussions. Finding ways to make TAG reviews easier would be helpful, even if it's just numbering questions so they can be referred to later in the discussion etc.

@slightlyoff

This comment has been minimized.

Member

slightlyoff commented Apr 29, 2017

Hey @owencm: thanks for the reply and for marking the TODOs. Also, thanks for the notes on the format. We note that you requested that we provide feedback as issues on your repo and we'll break out the notes thus far as separate issues. We provided some quick feedback here due to timeliness concerns; apologies for the difficulty.

We took this up today at the F2F meeting in TOK. Notes below are from the group discussion with @torgo, @triblondon, and @ylafon.

  • Regarding the Explainer, thanks for noting that the README is the place to look. Looking forward to seeing it expanded.
  • @torgo mentions that the Security and Privacy considerations section might want to include notes about what this API behaves when in private browsing mode. Specifically, advice for authors of specs that will integrate with Budgets
  • What operations would be modeled as budget-able but wouldn't also be introspectable via the Permissions API? As registries are an open problem for web specs, we'll ping back when we come up with a solution.
  • @triblondon notes that the model the API presents weights all silent pushes (e.g.) to be equal. Do we have confidence that this will always be true? Has a model been considered in which the developer requests some budget for an operation and is then limited to that budget (e.g., execution time)?

We'll convert the above into issues on your repo today.

@torgo torgo modified the milestones: tag-f2f-london-2017-07-25, tag-f2f-tokyo-2017-04-27 Apr 29, 2017

@torgo

This comment has been minimized.

Member

torgo commented Apr 29, 2017

Set to London as a place-holder but hopefully we can review and close before that.

@beverloo

This comment has been minimized.

beverloo commented May 24, 2017

Thank you for the feedback so far! I've replied or sent PRs for all of the above issues, and we'll definitely continue to iterate based on feedback and implementation experience.

scheib pushed a commit to scheib/chromium that referenced this issue May 25, 2017

Ship and experiment with the Budget API
This CL enables the BudgetManager.reserve() method and activates the
Origin Trial for the BudgetManager.getBudget() and getCost() methods, in
accordance with the following Intents on blink-dev:

Intent to Ship: The Budget API’s reserve() method:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/yBtmc-4xl_o/discussion

Intent to Experiment: The Budget API’s getCost() and getBudget() methods:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/_l_fxUTWCHs/discussion

Feedback from the TAG has been received and responded to in the issues
kindly filed by Alex. This led to a number of clarifications and changes
in the specification, which are in line with Chrome's implementation.

TAG review:
w3ctag/design-reviews#169
https://github.com/WICG/budget-api/issues?q=is%3Aissue%20tag

BUG=704725

Review-Url: https://codereview.chromium.org/2891953002
Cr-Commit-Position: refs/heads/master@{#474622}

@plinss plinss added the extra time label Jul 25, 2017

@triblondon

This comment has been minimized.

triblondon commented Jul 26, 2017

It looks like the feedback here has been taken on board and we like the new privacy wording in WICG/budget-api#19. We understand that there may be a design rethink to this API and if there is, we'd like to take a look, but we're happy to close this review.

@triblondon triblondon removed their assignment Apr 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment