-
Notifications
You must be signed in to change notification settings - Fork 56
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
New API proposal : extension.getContexts() #334
New API proposal : extension.getContexts() #334
Conversation
Yes, this api is very useful in this scenario. Because Currently, I use some workaround ways. For example,
Another example, find an extension tab from foreground by
A typical use case is that implement single tab mode: only open a page in one tab, if it is already open then active it (not open multiple same pages). Another problem of current API is that |
So I suggest |
Address comment in PR by @hanguokai: w3c#334 (comment)
@hanguokai Hi Jackie! Agreed that this could be a useful place to allow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This review contains a number of suggestions to
- normalize the use of HTML in GitHub Flavored Markdown
- clean up minor grammar issues
More substantive issues will be addressed in separate comments.
Here's my feedback, grouped by three themes:
Use cases?What are the use cases depending on this proposal?
1 and 2 are vague, and it would be nice to include more background on why such functionality is sufficiently important to be a necessary part of the extension APIs, and not already achievable in other (not super hacky) ways. 3 is useful on its own, and the need for a way to uniquely target a specific destination has been requested before (#294). It is even a prerequisite to improve the performance of extension messaging (elaborated at #293 (comment)). Prior artThe proposal here is a successor to
"Context" and "ContextType" are quite generic. There is even already a Properties and TOCTOUThe proposed API is async and resolves to an object whose properties are conceptually not immutable. Without clear use cases, this may be a recipe for TOCTOU bugs. So let's take a critical look at the fields to determine whether they are really necessary (which is dependent on the use cases).
|
@Rob--W Apologies I think by committing I prevented a directly reply to your comment, responding below
Use cases: a possible example is to check that a popup is open. Otherwise the hacky way is to have For uniquely targeting a specific destination, this brings us closer to that goal. Currently there's just broadcast, but now an extension could check if a I'll change TOCTOU: Something I probably didn't elaborate on when describing these properties is that they might be empty/not defined for the items where it doesn't make sense -- but altogether would cover the various extension contexts that might be returned.
For |
You didn't break anything; I posted a standalone comment because my feedback was not tied to one specific line.
That hacky approach wouldn't even work because it implies some persistent in-memory state in the background, whereas service workers are forcibly shut down at some point (currently 5 minutes in Chrome).
Even without querying the state of the popup, the existing extension messaging APIs ( A common use of
Non-broadcast semantics is desirable and requested at #294. It is currently missing a way to unambiguously describe any extension context (whether a tab, worker, content script, popup, sidebar/sidepanel, devtools panel, ...). Solving that would be valuable.
A context cannot change its private status. It is fixed at the start of the context (dependent on where the context is loaded) and immutable afterwards. Note that the table from my previous comment (#334 (comment)) lists the APIs that the recipient of the message can use to get the information to respond to the sender.
I asked for use cases because the value of the
Use case 2 and 3 do not need any properties. Other use cases that I have encountered in practice is determining whether there is already an instance of an extension tab, and if not open it (and if there is, focus it). There are already various UI-specific APIs for this scenario (designed and implemented after GapsIf I rewind, and start from the premise of "What gaps does the absence of
The first one is easy to satisfy by merely introducing the An easy answer could be to include "contextId" in the property set, and then support
A more complicated answer would be to extend APIs that target a context ( I think that it's a must-have to define how extension messaging is intended to be used with this |
I think the ultimate goal should be two parts:
Provide a way to find the targetMy first example in #334 (comment) is that the service worker communicate with the popup page by sending a message(note: this popup page can also be opened as a tab). This is a workaround for the problem 1) can't use My second example is finding a specific extension page first, then communicate with it by calling a method on it's The point of these two example is on how to find a specific target, not how to communicate. Here communication is the means to find the target. Current problem of finding the target
I know Communicate with this specific target@Rob--W think this is unclear at present. He thinks this should be clarified with this proposal and proposes #294. I think the main purpose of finding a target is to communicate with it, so it should be clear how to do that in this proposal. |
Thank you all for the thoughtful feedback. I agree with most of what's been said here. In my mind, I think there are two main benefits to this API:
As has been mentioned, for 1), there are workarounds to this that exist today: predominantly, message passing itself (send a message from a created context and track them in the interested context; or, send out a message from the interested context and see if there's a reply). This, to me, seems hacky and undesirable — it pollutes the message channels (which are currently broadcast everywhere), it's a bad fit for ephemeral contexts like service workers and event pages (since a message port can extend the lifetime of that context, and any incoming messages could result in the background context being spun up, even if it's not needed), and just feels generally "clunky". There's also existing requests for a way to tell if an context is open, such as determining if an action popup is open and determining if an offscreen document is present. Rather than requiring either the use of message sending to determine if these contexts are active or introducing different methods for each individual context type we might be interested in ( This API is targeted at achieving 1). After we have this API established, we'd like to use it to solve 2) — targeted messaging. I'd like to avoid blocking this proposal on fully spec'ing what that change looks like, because I think it has its own set of considerations (do we want to more fully differentiate between "broadcast" methods and non-"broadcast" methods? Are there other implications in the existing messaging APIs that we'd need to reconsider?). Proposal: We introduce Given that, does this proposal seem reasonable at a high level? I.e., providing an asynchronous method to determine which contexts are active for an extension by providing a bag-of-properties descriptor for each, which can later be used to target specific messages. If so, I agree there are other aspects we may want to discuss more. E.g.:
Regarding TOCTOU: In short, yes, there are TOCTOU risks. Given extension contexts can run on different threads (worker vs main) and in different processes (content scripts, incognito contexts), I think asynchronous behavior is largely unavoidable. (And, even if the extension were able to synchronously determine if a context were active, subsequent actions — like sending a message — are also async, and so have the same issue.) Rather than trying to fully avoid TOCTOU issues, I think it's better to ensure we provide developers with the proper tools to cope with asynchronous APIs and events. For instance, |
This looks reasonable at a high level.
Agreed. "url" / "frameUrl" should be renamed to "documentUrl".
There may be multiple scripts running in a worker or content script world. The only stable choice would be the first script that is running in it.
I think that So far we've mostly explored the returned properties. I'd also like to examine the input to With |
|
||
``` | ||
extension.ContextProperties = { | ||
tabId? = int, // Find context by tab id, omitted returns all tabs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to rephrase "returns all tabs" - there are contexts that are not tabs. Similarly for windows".
|
||
`sandbox` documents and iframes have an | ||
[opaque origin](https://html.spec.whatwg.org/multipage/browsers.html#concept-origin-opaque), | ||
should we return them? What if they come from within the extension (or outside |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO: no. Sandboxed documents have a "null"
origin and don't have access to extension APIs. They're hardly different from regular sandboxed iframes from the web.
run in a separate | ||
[Renderer](https://developer.chrome.com/blog/inside-browser-part3/) (and | ||
process) from the extension process. This proposal would not return those | ||
`Context`s since this adds significant complexity to the design in querying all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/design/implementation/.
Hey folks — just a quick status update here. This has been on the back burner for the last couple weeks, but we still plan on making more progress on it this quarter. |
I'm going to be taking over this work for now. Since Github doesn't (I think?) support taking over someone else's PR, there's a new PR for this here: |
Closing this per Devlin's comment. |
@dotproto