API Review: PostWebMessageAsJson and PostWebMessageAsString APIs for workers #5170
Conversation
…workers (#5151) This is a review for the new PostWebMessageAsJson and PostWebMessageAsString APIs for dedicated and service workers.
dhveerap
requested review from
david-risney,
shrinaths,
chetanpandey1266 and
Copilot
|
It might be useful to expose the name of dedicated workers (i.e. the name specified by |
| # Description | ||
| We propose the following APIs: | ||
|
|
||
| **PostWebMessageAsJson**: This API gives ability to post a message to |
There was a problem hiding this comment.
These signatures intentionally align with PostWebMessageAs* from CoreWebView2 that offer the equivalent functionality on the main thread, correct?
Should documentation cross-link?
https://learn.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.core.corewebview2?view=webview2-dotnet-1.0.3124.44
There was a problem hiding this comment.
Please update docs of all PostWebMessage APIs in CoreWebView2, CoreWebView2Frame, and these new ones to link to one another saying something like 'See the equivalent for CorewebView2...' something along those lines.
| /// `PostWebMessageAsJson` if you want to communicate using simple strings | ||
| /// rather than JSON objects. | ||
| HRESULT PostWebMessageAsString( | ||
| [in] LPCWSTR messageAsString |
There was a problem hiding this comment.
nit, param name webMessageAsString to align with CoreWebView2.PostWebMessageAsString?
There was a problem hiding this comment.
Yes please match existing methods and use: webMessageAsString.
Also for above PostWebMessageAsJson.
| **PostWebMessageAsJson**: This API gives ability to post a message to | ||
| dedicated/service worker. The worker receives the message by subscribing to | ||
| message event of `self.chrome.webview`. | ||
|
|
There was a problem hiding this comment.
The CoreWebView2 overload of PostWebMessageAsJson that has params= (String, List) doesn't have a matching proposal here because the DOM objects in the List are only applicable on the main thread, is that right?
There was a problem hiding this comment.
Yes we do want to do that but it will be in a future spec.
| //! [chromeWebView] | ||
| }; | ||
|
|
||
| self.addEventListener('install', (event) => { |
There was a problem hiding this comment.
Why the sample difference between self.chrome.webview.addEventListener and self.addEventListener ? Is this an existing asymmetry of dedicated v service workers?
There was a problem hiding this comment.
self.addEventListener is for subscribing to existing web event listeners vs self.chrome.webview.addEventListener for new WebView2 related events.
| } | ||
|
|
||
| [uuid(66833876-edba-5a60-8508-7da64504a9d2), object, pointer_default(unique)] | ||
| interface ICoreWebView2DedicatedWorker : IUnknown { |
There was a problem hiding this comment.
I see there is a CoreWebView2DedicatedWorker in .net prerelease. This interface doesn't need a "2", just checking is that because the class hasn't gone to stable release yet?
There was a problem hiding this comment.
Please investigate why we have this empty class in the SDK. The .NET prerelease contains this class with no members or methods. This looks like a mistake: https://learn.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.core.corewebview2dedicatedworker?view=webview2-dotnet-1.0.2950-prerelease
In the future we'll add a comment to interfaces that are in staging that we're updating
| === | ||
|
|
||
| # Background | ||
| Currently, if developers want to post a message to or from a worker, they must |
There was a problem hiding this comment.
Folks were confused by these comments. Provider better explanation of how this is working currently eg: CoreWebView2.PostWebMessage -> DOM window -> worker. And be sure to say main thread of web content since that can be confused for the host app's main thread.
| # Description | ||
| We propose the following APIs: | ||
|
|
||
| **PostWebMessageAsJson**: This API gives ability to post a message to |
There was a problem hiding this comment.
Please update docs of all PostWebMessage APIs in CoreWebView2, CoreWebView2Frame, and these new ones to link to one another saying something like 'See the equivalent for CorewebView2...' something along those lines.
| **PostWebMessageAsJson**: This API gives ability to post a message to | ||
| dedicated/service worker. The worker receives the message by subscribing to | ||
| message event of `self.chrome.webview`. | ||
|
|
There was a problem hiding this comment.
Yes we do want to do that but it will be in a future spec.
| void ScenarioDedicatedWorkerPostMessage::ComputeWithDedicatedWorker( | ||
| wil::com_ptr<ICoreWebView2DedicatedWorker> dedicatedWorker) | ||
| { | ||
| TextInputDialog dialog( |
There was a problem hiding this comment.
Throughout sample please ensure we don't show synchronous UI from webview2 event handlers / async completion handlers. Using the AsyncMessageBox addresses this otherwise you'll need to explicitly get an async code to run to show the message box.
| //! [chromeWebView] | ||
| }; | ||
|
|
||
| self.addEventListener('install', (event) => { |
There was a problem hiding this comment.
self.addEventListener is for subscribing to existing web event listeners vs self.chrome.webview.addEventListener for new WebView2 related events.
| } | ||
|
|
||
| [uuid(66833876-edba-5a60-8508-7da64504a9d2), object, pointer_default(unique)] | ||
| interface ICoreWebView2DedicatedWorker : IUnknown { |
There was a problem hiding this comment.
Please investigate why we have this empty class in the SDK. The .NET prerelease contains this class with no members or methods. This looks like a mistake: https://learn.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.core.corewebview2dedicatedworker?view=webview2-dotnet-1.0.2950-prerelease
In the future we'll add a comment to interfaces that are in staging that we're updating
| /// `PostWebMessageAsJson` if you want to communicate using simple strings | ||
| /// rather than JSON objects. | ||
| HRESULT PostWebMessageAsString( | ||
| [in] LPCWSTR messageAsString |
There was a problem hiding this comment.
Yes please match existing methods and use: webMessageAsString.
Also for above PostWebMessageAsJson.
| /// arg of the worker's `self.chrome.webview` message is a string with the same | ||
| /// value as `messageAsString`. Use this instead of | ||
| /// `PostWebMessageAsJson` if you want to communicate using simple strings | ||
| /// rather than JSON objects. |
There was a problem hiding this comment.
Please update doc comment here to say "please see PostWebMessageAsJson for additional information."
This is a review for the new PostWebMessageAsJson and PostWebMessageAsString APIs for dedicated and service workers.