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

API review: Add Workers management APIs. (#4556) #4682

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

monica-ch
Copy link
Contributor

This is an API review for the new Workers API.

* Add workers spec

* added checks

* Per feedback - reference dom apis and example of api usage in workers

* Update specs/Workers.md

Co-authored-by: David Risney <[email protected]>

* address feedback-2

* remove state and add activated event

* minor updates to the spec

---------

Co-authored-by: David Risney <[email protected]>
@monica-ch monica-ch added the API Proposal Review WebView2 API Proposal for review. label Jul 15, 2024
@monica-ch monica-ch requested a review from david-risney July 15, 2024 18:04
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Show resolved Hide resolved
specs/Workers.md Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated
ServiceWorkerManager_ = WebViewProfile.ServiceWorkerManager;
}
CoreWebView2ServiceWorkerRegistration registration = await
ServiceWorkerManager_.GetServiceWorkerRegistrationAsync(dialog.Input.Text);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix GetServiceWorkerRegistration docs to note about URI normalization.

specs/Workers.md Outdated Show resolved Hide resolved
specs/Workers.md Outdated

if(registration != null)
{
CoreWebView2ServiceWorker worker = registration.ActiveServiceWorker;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please ensure that it does: ActiveServiceWorker won't destroy until messages pumped and so on.

specs/Workers.md Outdated

if(registration != null)
{
CoreWebView2ServiceWorker worker = registration.ActiveServiceWorker;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the worker instance exits the registration continues to exist.

-- NOTE this is where we stopped in the review meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Proposal Review WebView2 API Proposal for review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants