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

Introduce user context subscriptions #836

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 69 additions & 8 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -713,11 +713,13 @@ all [=subscription/subscription ids=] that have been issued to the [=local end=]

A <dfn for=event>subscription</dfn> is a [=/struct=] consisting of a
<dfn for=subscription>subscription id</dfn> (a string),
<dfn for=subscription>event names</dfn> (a [=/set=] of event names)
and <dfn for=subscription>top-level traversable ids</dfn> (a [=/set=] of IDs of [=/top-level traversables=]).
<dfn for=subscription>event names</dfn> (a [=/set=] of event names),
<dfn for=subscription>top-level traversable ids</dfn> (a [=/set=] of IDs of [=/top-level traversables=])
and <dfn for=subscription>user context ids</dfn> (a [=/set=] of IDs of [=user contexts=]).
OrKoN marked this conversation as resolved.
Show resolved Hide resolved

A [=subscription=] |subscription| is <dfn for="subscription">global</dfn>
if |subscription|'s [=subscription/top-level traversable ids=] is an empty set.
if |subscription|'s [=subscription/top-level traversable ids=] is an empty set
and |subscription|'s [=user context ids=] is an empty set.

<div algorithm>

Expand Down Expand Up @@ -750,10 +752,19 @@ Note: |navigables| is a set because a [=shared worker=] can be associated

1. If |subscription| is [=subscription/global=] return true.

1. Let |subscription top-level traversables| be [=get navigables by ids=] with |subscription|'s [=subscription/top-level traversable ids=].
1. If [=subscription/user context ids=] is not empty:

1. If the [=set/intersection=] of |top-level traversables| and
|subscription top-level traversables| is not [=list/empty=] return true.
1. [=list/For each=] |navigable| in |top-level traversables|:

1. If |subscription|'s [=subscription/user context ids=] [=set/contains=]
|navigable|'s [=associated user context=]'s [=user context id=], return true.

1. Otherwise:

1. Let |subscription top-level traversables| be [=get navigables by ids=] with |subscription|'s [=subscription/top-level traversable ids=].

1. If the [=set/intersection=] of |top-level traversables| and
|subscription top-level traversables| is not [=list/empty=] return true.
OrKoN marked this conversation as resolved.
Show resolved Hide resolved

1. Return false.

Expand All @@ -779,6 +790,14 @@ given |event name| and |session| is:

1. [=Break=].

1. Otherwise, if [=subscription/user context ids=] is not empty:

1. For each |traversable| in remote end's [=/top-level traversables=]:

1. [=set/Append=] |traversable| to |result| if
|subscription|'s [=subscription/user context ids=] [=set/contains=]
|traversable|'s [=associated user context=]'s [=user context id=].

1. Otherwise:

1. Let |top-level traversables| be [=get navigables by ids=] with |subscription|'s [=subscription/top-level traversable ids=].
Expand Down Expand Up @@ -1436,6 +1455,31 @@ To <dfn export>get user context</dfn> given |user context id|:

</div>

When a [=user context=] is [=set/remove|removed=] from the
[=set of user contexts=], [=remove user context subscriptions=]:
OrKoN marked this conversation as resolved.
Show resolved Hide resolved

<div algorithm>

To <dfn export>remove user context subscriptions</dfn>:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this does create undefined behaviour, because it makes it observable whether a specific user context was removed or not, which is implementation defined (I guess technically that was already the case since it affects what's in storage). It isn't a blocker here, but I wonder if we should define that any user context created via createUserContext actually is removed if it goes from 1 to 0 browsing contexts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I get why this creates undefined behavior. I added this algorithm to define a clean-up step to avoid accumulating subscriptions if the client does not unsubscribe (unsubscribe requests with the ID of removed subscriptions can still be issued). I think it is not observable to the local end. Could you please clarify?

It isn't a blocker here, but I wonder if we should define that any user context created via createUserContext actually is removed if it goes from 1 to 0 browsing contexts.

In Chrome, the user context stays there if all browsing contexts in it are removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's actually not externally visible whether or not this cleanup happened, so it's fine.

The problem I'm thinking of is actually already an issue. Say you create a user context, create a tab in that user context, then close the tab. Now an implementation is permitted to remove the user context. But whether or not you do that is visible if you then try to create another new tab in that user context.

It kind of makes sense for gecko because you can delete the user context in the browser UI. So if someone actually interacts with the browser and does that all bets are off. But maybe the spec should be clearer that it's not expected that user contexts (or their associated storage) are removed without user interaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the same situation can happen in Chrome if the user closes a user context via the UI. I think it makes sense to clarify that user contexts should not get removed randomly by the implementation, but removing them as the result of a direct user action is okay.


1. For each |session| in [=active sessions=]:

1. Let |subscriptions to remove| be a [=/set=].

1. For each |subscription| in |session|'s [=subscriptions=]:

1. If |subscription|'s [=subscription/user context ids=] [=set/contains=] |navigable|'s [=associated user context=]'s [=user context id=];

1. [=set/Remove=] |navigable|'s [=associated user context=]'s [=user context id=] from |subscription|'s [=subscription/user context ids=].

1. If |subscription|'s [=subscription/user context ids=] is empty:

1. [=set/Append=] |subscription| to |subscriptions to remove|.

1. [=list/Remove=] |subscriptions to remove| from |session|'s [=subscriptions=].

</div>

# Modules # {#modules}

## The session Module ## {#module-session}
Expand Down Expand Up @@ -1661,6 +1705,7 @@ The <code>session.Subscription</code> type represents a unique subscription iden
session.SubscriptionRequest = {
events: [+text],
? contexts: [+browsingContext.BrowsingContext],
? userContexts: [+browser.UserContext],
}
</pre>

Expand Down Expand Up @@ -1918,12 +1963,26 @@ The [=remote end steps=] with |session| and |command parameters| are:

1. [=set/Append=] |navigable|'s [=navigable id=] to |top-level traversable context ids|.

1. Otherwise, if |input user context ids| is not empty:
OrKoN marked this conversation as resolved.
Show resolved Hide resolved

1. [=list/For each=] |user context id| of |input user context ids|:

1. Let |user context| be [=get user context=] with |user context id|.

1. If |user context| is null, return [=error=] with [=error code=] [=invalid argument=].

1. [=list/For each=] |top-level traversable| in the list of all [=/top-level traversables=]
whose [=associated user context=] is |user context|:

1. [=list/Append=] |top-level traversable| to |subscription navigables|.

1. Otherwise, set |subscription navigables| to a [=/set=] of all [=navigable/top-level traversables=] in the [=remote end=].

1. Let |subscription| be a [=subscription=] with
[=subscription/subscription id=] set to the string representation of a [[!RFC9562|UUID]],
[=subscription/event names=] set to |event names|, and
[=subscription/top-level traversable ids=] set to |top-level traversable context ids|.
[=subscription/event names=] set to |event names|,
[=subscription/top-level traversable ids=] set to |top-level traversable context ids| and
[=subscription/user context ids=] set to |input user context ids|.

1. Let |subscribe step events| be a new [=/map=].

Expand Down Expand Up @@ -2041,6 +2100,8 @@ The [=remote end steps=] with |session| and |command parameters| are:

1. [=list/append=] |subscription| to |new subscriptions|.

1. Otherwise, if |subscription|'s [=subscription/user context ids=] is not empty, [=continue=].

1. Otherwise:

Note: unsubscribe by contexts is deprecated and will be removed in the future versions.
Expand Down