API Review: Periodic Background Sync and Background Sync API#4540
API Review: Periodic Background Sync and Background Sync API#4540JosephJin0815 wants to merge 17 commits intoapi-periodic-sync-and-background-syncfrom
Conversation
| auto webViewProfile3 = webView2Profile.try_query<ICoreWebView2Profile3>(); | ||
| CHECK_FEATURE_RETURN_EMPTY(webViewProfile3); | ||
| wil::com_ptr<ICoreWebView2ServiceWorkerManager> serviceWorkerManager; | ||
| CHECK_FAILURE(webViewProfile3->get_ServiceWorkerManager(&serviceWorkerManager)); |
There was a problem hiding this comment.
It looks like this API depends on the worker API. Do we need to wait for the worker API spec to finish first? Or will it be good to consider them at the same time?
| } | ||
| else if (wcscmp(message.c_str(), L"GetPeriodicSyncRegistrations") == 0) | ||
| { | ||
| ShowAllSyncRegistrationInfos(COREWEBVIEW2_SERVICE_WORKER_SYNCHRONIZATION_KIND_PERIODIC_SYNC); |
There was a problem hiding this comment.
Same question as previous about the scenario for the sample code. This looks like sandbox / playground sort of code to play with the API rather than sample code that demonstrates something an end dev would actually do with this API in their code.
| /// The minimum interval time, in milliseconds, at which the minimum time | ||
| /// interval between periodic background synchronizations should occur. | ||
| /// You should not call `CoreWebView2ServiceWorkerSyncRegistrationManager.DispatchPeriodicSyncEventAsync` | ||
| /// to dispatch periodic synchronization tasks less than its minimum time interval. |
There was a problem hiding this comment.
Can you say in here why we end devs shouldn't do this?
There was a problem hiding this comment.
I believe we said in chat that you would add something like 'This is because the periodic sync web APIs standard says it will not be dispatched more often than the minimum time interval and the web content may not handle the event being raised more often.'
| /// The minimum interval time, in milliseconds, at which the minimum time | ||
| /// interval between periodic background synchronizations should occur. | ||
| /// You should not call `CoreWebView2ServiceWorkerSyncRegistrationManager.DispatchPeriodicSyncEventAsync` | ||
| /// to dispatch periodic synchronization tasks less than its minimum time interval. |
There was a problem hiding this comment.
I believe we said in chat that you would add something like 'This is because the periodic sync web APIs standard says it will not be dispatched more often than the minimum time interval and the web content may not handle the event being raised more often.'
|
|
||
| event Windows.Foundation.TypedEventHandler<CoreWebView2ServiceWorkerSyncRegistrationManager, CoreWebView2ServiceWorkerSyncRegisteredEventArgs> PeriodicSyncRegistered; | ||
|
|
||
| Windows.Foundation.IAsyncOperation<IVectorView<CoreWebView2ServiceWorkerSyncRegistrationInfo>> GetSyncRegistrationsAsync(CoreWebView2ServiceWorkerSynchronizationKind Kind); |
There was a problem hiding this comment.
There's already two events for background and periodic seems like it would make sense to also have two separate methods to get background and periodic registrations?
This is a review for the new Periodic Background Sync and Background Sync API.