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

Update API spec in explainer to match latest discussion #16

Merged
merged 5 commits into from
Aug 1, 2022

Conversation

mingyc
Copy link
Collaborator

@mingyc mingyc commented Jul 27, 2022

- Fix WICG#13: Replace `pageHideTimeout` with `backgroundTimeout` and
  `timeout`. Allow to set them in constructor & data/url setters.
- Address WICG#14: Rename `isPending` to `pending` and specify its
  behaviors.
- Address WICG#14: Keep the low-level sync API, and mark it as chosen.
- Fix WICG#6: Add `PendingGETBeacon` and `PendingPOSTBeacon`.
@mingyc mingyc added the api Issue with API specs label Jul 27, 2022
@mingyc mingyc requested a review from fergald July 27, 2022 09:41
@mingyc mingyc self-assigned this Jul 27, 2022
@mingyc
Copy link
Collaborator Author

mingyc commented Jul 27, 2022

cc @philipwalton @fergald PTAL. Please also suggest better naming than PendingGETBeacon, PendingPOSTBeacon, thanks!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
- WICG#16 (comment)
  Specify when a timer will start for `timeout`
@mingyc mingyc merged commit 57bdb38 into WICG:main Aug 1, 2022
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 5, 2022
- `pageHideTimeout`
  - Replace pageHideTimeout with `backgroundTimeout` and `timeout`.
  - Allow to set them in constructor & via property setters.
- `isPending`
  - Rename isPending to `pending` and specify its behaviors.
- `setData(data)`
  - Remove setData(data) from base class PendingBeacon.
  - Add subclass `PendingGetBeacon` which supports `setURL(url)`.
    - Also define a new mojom method `setRequestURL(url)` to help update
      browser-side beacon.
  - Add subclass `PendingPostBeacon` which supports `setData(data)`.

See [the pull request][1] or [explainer][2] for more details.

[1]: WICG/pending-beacon#16 (comment)
[2]: https://github.com/WICG/unload-beacon#readme

Bug: 1293679
Change-Id: I8b73bdd3de7f5e8a3358564be52e2c0f2f6774de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3789544
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Ming-Ying Chung <mych@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031758}
@mingyc mingyc deleted the update-explainer branch August 6, 2022 08:10
mingyc added a commit that referenced this pull request Sep 14, 2022
Update Privacy section to clarify why beacon needs to be stopped when network changes (#27 #28)

Also added links to related privacy discussions: #3, #27, #30, #34
Updated beacon TTL to 30min per suggestion in #16 (comment)
@fergald
Copy link
Collaborator

fergald commented Oct 12, 2022

I think in the case of

Tab A: top level is userfoo.blog.com, frame is map.company.com
Tab B: top level is map.company.com

I think Tab A's map.company.com beacons should still be delivered because

  1. that frame can't create new beacons, the data was put into the beacons before it entered BFCache, it could have sent them immediately, wasting radio, quota, CPU etc, instead it used a beacon with a delay to try save resources in the case that comes back out quickly.
    Or to put it another way, there is no user expectation being violated about what data is sent.
  2. It's hard to argue that the user has any expectation about when the data is sent since beacon is write-only but since map.company.com is open in another tab, having connections occur to its servers does not seem to violate expectations.

In general I would really like to not impose restrictions on beacon that are not present already because all that does is keep devs doing things the hard way which also tends to be wasteful of user resources.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
- `pageHideTimeout`
  - Replace pageHideTimeout with `backgroundTimeout` and `timeout`.
  - Allow to set them in constructor & via property setters.
- `isPending`
  - Rename isPending to `pending` and specify its behaviors.
- `setData(data)`
  - Remove setData(data) from base class PendingBeacon.
  - Add subclass `PendingGetBeacon` which supports `setURL(url)`.
    - Also define a new mojom method `setRequestURL(url)` to help update
      browser-side beacon.
  - Add subclass `PendingPostBeacon` which supports `setData(data)`.

See [the pull request][1] or [explainer][2] for more details.

[1]: WICG/pending-beacon#16 (comment)
[2]: https://github.com/WICG/unload-beacon#readme

Bug: 1293679
Change-Id: I8b73bdd3de7f5e8a3358564be52e2c0f2f6774de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3789544
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Ming-Ying Chung <mych@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031758}
NOKEYCHECK=True
GitOrigin-RevId: d65b7474fc01ad30750fbbca7ad55b09b8070157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue with API specs
Projects
None yet
4 participants