Let each permission refine its algorithms and store data.

[jyasskin]

This is now ready to review. See it in action at https://rawgit.com/jyasskin/permissions/allow-choosers/index.html, and a use of the new extension hooks at https://rawgit.com/jyasskin/web-bluetooth-1/permission-api/index.html#permission-api-integration.

[domenic]

enumerator-value is a weird term I've never heard before. Is it a C++ thing? Prefer "enumeration value" or "enum value".

[jyasskin]

It's a funny mix of C++ and WebIDL. Oops. Fixing.

[jyasskin]

I think this is ready for a serious look now, @marcoscaceres @mounirlamouri. Feel free to tell me to split it up into a couple different PRs.

[domenic]

This paragraph needs a few "instance of"s inserted as well.

[jyasskin]

Done.

[jyasskin]

See https://rawgit.com/jyasskin/web-bluetooth-1/permission-api/index.html#permission-api-integration for the beginning of Web Bluetooth's use of these new hooks.

[raymeskhoury]

Hey Jeffrey, a few random thoughts (some are not solely related to your edits):
-It initially hard to see which parts were external/exposed APIs vs internal constructs. e.g. PermissionDescriptor and PermissionStatus are external JS APIs - they must be complied with precisely whereas PermissionStorage is merely meant to help define storage behavior. Would it be helpful to draw a more clear line between those two groups?
-I also found it a bit hard to draw the distinction between required and recommended behaviors. It seems as though there should be room for UAs to handle permissions in ways that are outside of those currently specified behavior (for example, we have policy settings in Chrome that can override permissions). I don't think this is directly related to your edits.
-In relation to the above two points, perhaps a top-down restructuring of the spec could be helpful? External APIs and high level, required behaviors specified first and per-API behaviors, more subtle recommended behaviors, implementation details, etc. specified after? I'm no spec writer though :)
-I didn't think we were going to draw a distinction between an origin-based and embedding-based permission storage identifier instead leaving it up to the UA. I just pinged Mounir asking about this because we had chatted about it earlier. If it turns out to be the case does the permission storage identifier need to vary per-permission?

[jyasskin]

Thanks for the comments!

It initially hard to see which parts were external/exposed APIs vs internal constructs. e.g. PermissionDescriptor and PermissionStatus are external JS APIs - they must be complied with precisely whereas PermissionStorage is merely meant to help define storage behavior. Would it be helpful to draw a more clear line between those two groups?

Yeah. The traditional way to distinguish these would be to avoid describing PermissionStorage in IDL. However, that loses IDL's precision about what's stored, which I think is helping Bluetooth (I should be able to show you that tomorrow). I left a note saying PermissionStorage isn't exposed, but if you have other suggestions, I'm happy to take them.

I also found it a bit hard to draw the distinction between required and recommended behaviors. It seems as though there should be room for UAs to handle permissions in ways that are outside of those currently specified behavior (for example, we have policy settings in Chrome that can override permissions). I don't think this is directly related to your edits.

Right, I think that's pre-existing. In most cases, I think we can say "the UA may customize the permission prompt, including auto-granting and auto-denying it" and "the UA may revoke at any time", and that's enough? I'd like to fix that in a separate PR, to keep this one from growing much more.

In relation to the above two points, perhaps a top-down restructuring of the spec could be helpful? External APIs and high level, required behaviors specified first and per-API behaviors, more subtle recommended behaviors, implementation details, etc. specified after? I'm no spec writer though :)

Probably. Again, I want to keep rearrangements separate from the API extension.

I didn't think we were going to draw a distinction between an origin-based and embedding-based permission storage identifier instead leaving it up to the UA. I just pinged Mounir asking about this because we had chatted about it earlier. If it turns out to be the case does the permission storage identifier need to vary per-permission?

Yeah, if it's always PermissionName + origin + UA-specific data, I think we can drop the identifier hook. I'll let Mounir confirm and then do that in my next update.

[jyasskin]

The identifier hook is gone. PTAL.

[mounirlamouri]

At a high level I agree with the changes here. Though, it's a very large CL and I can't realistically review this. Could you split this in a few parts:

• the commit that export the retrieve a permission (CC @tobie)
• the PermissionStorage definition
• the rest of the changes (I might ask you to split more but at least, I will have a better visibility -- keep the commits separated, it might be easier)

Also, could you remove tidy changes? It seems that we have issues with your tidy runs getting other results than mine. You are not the first person. I would like to figure this out before every one changes the spec to match its own tidy runs.

[jyasskin]

Ok, I've split out a bunch of separate PRs that build on each other. I haven't yet undone the Tidy changes, and I've squashed the remaining changes in this PR since the individual commits were getting incoherent.

[tobie]

I haven't yet undone the Tidy changes

OK, sorry for bitching in the other PR, then. :/

[jyasskin]

I've noticed that PushMessagingPermissionContext::DecidePermission checks for requesting_origin == embedding_origin, which doesn't fit into the current permission request algorithm arguments.

If it's ok with you folks, I'd like to get the current PR merged, and then fix up details like this in subsequent smaller patches.