-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
api(chromium): add ChromiumBrowserContext.serviceWorkers() #1416
Conversation
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 looks good, but if discovery is there only until the roll, we can wait a little. Up to you.
@@ -52,6 +52,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser { | |||
const promises = [ | |||
session.send('Target.setDiscoverTargets', { discover: true }), | |||
session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }), | |||
session.send('Target.setDiscoverTargets', { discover: false }), |
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.
Is this temporary before you land your chromium patch?
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.
No, it's permanent, we don't need targetCreated/Destroyed notifications anymore, just attachedToTarget/detachedFromTarget.
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.
Wonderful!
@@ -3861,6 +3862,9 @@ Emitted when new service worker is created in the context. | |||
- `page` <[Page]> Page to create new session for. | |||
- returns: <[Promise]<[CDPSession]>> Promise that resolves to the newly created session. | |||
|
|||
#### chromiumBrowserContext.serviceWorkers() | |||
- returns: <[Array]<[Worker]>> All existing service workers in the context. |
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.
Do we have origin on Worker? Determining who owns a service worker via its script url is unfortunate.
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.
We don't have it at the moment. We could expose similar Page.serviceWorkers(), or add ServiceWorker class that would allow to get a list of its clients.
References: #1101