search

privacycg / storage-access

The Storage Access API
https://privacycg.github.io/storage-access/
199 stars 27 forks source link

Fix async steps in hasStorageAccess and requestStorageAccess. #70

Closed johannhof closed 3 years ago

johannhof commented 3 years ago

This includes another commit because it is based on #68

Fixes #50, #69

This is based on some of Anne's feedback around how we run async steps in parallel and queue tasks for resolving promises instead of what we were doing before.

This also moves a bunch of things that probably don't need to happen in parallel (and don't happen in parallel in at least Gecko and WebKit) out of the async steps.

johannhof commented 3 years ago

Oh and I also removed references to specific code lines since these were quite outdated now, I left the top-level references to which files these implementations live in.

johannhof commented 3 years ago

I did not look in detail as there are still some other issues to be resolved as discussed offline (e.g., same origin needing origins), but overall this seems like a reasonable improvement to me.

So I said that I'd prefer to resolve these in separate commits since I don't want to overload this commit to solve all the problems. Does that make sense? If so, I would appreciate another look in detail :)

johannhof commented 3 years ago

I wonder if it's possible to setup https://github.com/tobie/pr-preview for this repository to make these kind of changes a bit easier to review.

I requested the GitHub App to be installed for the privacycg org. @TanviHacks @erik-anderson @hober, it would be great if either of you could accept that request :)

annevk commented 3 years ago

@johannhof so another thing that would help a lot going forward is pushing fixup commits during review as GitHub does not give you interdiffs. And you can use squash & merge when merging most of the time anyway.

johannhof commented 3 years ago

@johannhof so another thing that would help a lot going forward is pushing fixup commits during review as GitHub does not give you interdiffs. And you can use squash & merge when merging most of the time anyway.

Yeah, sorry, working on Gecko most of my time has burned the Phabricator flow of "always squash" into my brain. :)

johannhof commented 3 years ago

@johnwilander ping :)