API review: Add Workers management APIs. #4556
Conversation
specs/Workers.md
Outdated
| wil::unique_cotaskmem_string scriptUrl; | ||
| CHECK_FAILURE(worker->get_ScriptUrl(&scriptUrl)); | ||
|
|
||
| message << L"ScriptUrl: " << scriptUrl.get(); |
There was a problem hiding this comment.
We want sample code that demonstrates doing something we expect end developers to actually write code for in their apps. The code you have here is more of sandbox / playground sort of thing where someone can try out the API rather than what we would expect the API to actually be used for.
There was a problem hiding this comment.
As discussed, I added sample code for apps to utilize this worker monitoring for posting messages to worker for one sample of each worker. Please do suggest if it is not conveying it properly.
| { | ||
| String Scope { get; }; | ||
|
|
||
| event Windows.Foundation.TypedEventHandler<CoreWebView2ServiceWorkerRegistration, IInspectable> Destroyed; |
There was a problem hiding this comment.
I've added this to notify apps that either the SW is unregistered, or we are destroying the object during profile destruction and such but I'm not sure if helps devs much. @david-risney What do you having of having this event on SWR? Do we notify apps that we are destroying the object and such?
specs/Workers.md
Outdated
|
|
||
| event Windows.Foundation.TypedEventHandler<CoreWebView2ServiceWorkerRegistration, IInspectable> Destroyed; | ||
|
|
||
| Windows.Foundation.IAsyncOperation<CoreWebView2ServiceWorker> GetServiceWorkerAsync(); |
There was a problem hiding this comment.
This gives info on active service worker as host apps in general interested in communicating with the active ones. Do you think we should match with the DOM one's like a method to get installing SW or waiting SW?
| { | ||
| String ScriptUri { get; }; | ||
|
|
||
| CoreWebView2ServiceWorkerState State { get; }; |
There was a problem hiding this comment.
Right now, we are dealing with active service workers, may be not needed as long as we provide a event to app that notifies SW state changes or methods to give access to other workers.
specs/Workers.md
Outdated
|
|
||
| [uuid(0c0b03bd-ced2-5518-851a-3d3f71fb5c4b), object, pointer_default(unique)] | ||
| interface ICoreWebView2ServiceWorkerRegistration : IUnknown { | ||
| /// A string representing a URL that defines a service worker's registration scope. It is relative to the base URL of the application. |
There was a problem hiding this comment.
This text says the property is relative. The next paragraph says it is a fully qualified URL. The fourth paragraph says it corresponds to the ServiceWorkerRegistration's scope which at least when calling register the scope is relative. Please make this clearer. This also applies to the above definition of scope in the registered event args.
There was a problem hiding this comment.
Updated, during registration the scope is relative but in our API's it is full URL.
Also, I removed duplicated Scope from ICoreWebView2ServiceWorkerRegistrationEventArgs as devs can access the scope from the registration object.
specs/Workers.md
Outdated
| /// | ||
| /// This corresponds to the `active` property of the `ServiceWorkerRegistration` object in the DOM. | ||
| /// For more information, see the [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerRegistration/active). | ||
| HRESULT GetServiceWorker( |
There was a problem hiding this comment.
Should this be a property instead of a method? And should it be named 'ActiveServiceWorker' to sort of match the DOM name?
There was a problem hiding this comment.
The reason it is created as async method because the CoreWebView2ServiceWokerRegistration object is created as soon as worker is registered in the web but there won't be any SW's available to interact with. The DOM has StateChanged event that notifies when the SW state changes so web can access install/active or waiting workers when it fires.
Do you think we should also provide similar event and make ActiveServiceWorker as property so when app receives this state they can access from ActiveServiceWorker property
Or
make ActiveServiceWorker property default to null and in the impl we monitor the SW state changes to active and create CoreWebView2ServiceWorker object and when app queries for this later we return the object?
Co-authored-by: David Risney <dave@deletethis.net>
This is the review for Workers Support in WebView2.