Add spec for IFrameMemoryUsage.md #3234

JosephJin0815 · 2023-02-22T17:41:12Z

This is a review for the new IFrame memory usage API.

david-risney · 2023-02-22T18:14:01Z

specs/IFrameMemoryUsage

+        [interface_name("Microsoft.Web.WebView2.Core.ICoreWebView2_18")]
+        {
+            // ICoreWebView2_18 members
+            UInt32 ProcessId { get; };


We should probably name this RendererProcessId (and rename all the rest too). There's different kinds of processes and we should make it clear what this is.

david-risney · 2023-02-22T18:15:49Z

specs/IFrameMemoryUsage

+```c# (but really MIDL3)
+namespace Microsoft.Web.WebView2.Core
+{   
+    runtimeclass CoreWebView2FrameCreatedEventArgs


This is the same runtimeclass that already exists correct? No modifications? If so and you want to include it please add a comment noting that this is an existing unchanged class.

Yep. Will remove this from spec.

david-risney · 2023-02-22T18:17:12Z

specs/IFrameMemoryUsage

+        }
+    }
+
+    // Skipping class CoreWebView2FrameInfoCollection


You can remove this comment. The frame info is not relevant to this doc

david-risney · 2023-02-22T18:20:59Z

specs/IFrameMemoryUsage

+```
+C#
+```c#
+void WebView_HandleIFrames(object sender, CoreWebView2FrameCreatedEventArgs args)


The C# and C++ sample code should be the same. Currently the C++ code creates a message whereas the C# code creates a frames list.

Will refine that later.

Refined. I added some comments for both when renderer process id changed/obtained.

david-risney · 2023-02-22T18:22:10Z

specs/IFrameMemoryUsage

+                        unsigned int processId;
+                        CHECK_FAILURE(webviewFrame5->get_ProcessId(&processId));
+                        message +=
+                            L",\"render process id\": " + std::to_wstring((int)processId);


We don't do anything with this message we create. Is there a real scenario we can demonstrate with the FrameCreated event? What did we do for the sample code in the CoreWebView2.FrameCreated spec?

In Win Sample App -> Window Event Monitor, we print out this message. I will add that to the spec.

But what is the real world scenario? I guess we're adding it so apps can monitor performance and such via telemetry so this is perhaps fine.

Apps can get memory usage based on process id.

david-risney · 2023-03-04T00:43:05Z

specs/IFrameMemoryUsage

+                        unsigned int processId;
+                        CHECK_FAILURE(webviewFrame5->get_ProcessId(&processId));
+                        message +=
+                            L",\"render process id\": " + std::to_wstring((int)processId);


But what is the real world scenario? I guess we're adding it so apps can monitor performance and such via telemetry so this is perhaps fine.

david-risney · 2023-03-08T22:08:58Z

specs/IFrameMemoryUsage

+  // MSOWNERS: [email protected]
+  HRESULT remove_FrameCreated([in] EventRegistrationToken token);
+
+  /// Get the render process id of the iframe.


Would be nice to explain just a bit about the renderer process for a frame: The render process ID can change when navigating to a new document and you can use the RenderProcessIdChanged event to know when it changes. Child frames may have different renderer processes. See the process model documentation for more information.

david-risney · 2023-03-08T22:10:40Z

specs/IFrameMemoryUsage

+
+  /// Get the render process id of the iframe.
+  // MSOWNERS: [email protected]
+  [propget] HRESULT RenderProcessId([out, retval] UINT32* value);


Chromium code and documentation (https://www.chromium.org/developers/design-documents/multi-process-architecture/) calls this 'renderer process' not 'render process'. Do we already have reference to 'render process' in our APIs? If so we can match that existing term. Otherwise would be good to name 'renderer process'

No existing APIs use render process as naming. Agree!

Looks like both show up already. Since we're already inconsistent we can go with the more accurate one renderer. COREWEBVIEW2_PROCESS_FAILED_KIND uses 'render' and COREWEBVIEW2_PROCESS_KIND uses 'renderer'

david-risney · 2023-03-09T00:15:27Z

specs/IFrameMemoryUsage

+  // MSOWNERS: [email protected]
+  [propget] HRESULT RenderProcessId([out, retval] UINT32* value);
+
+  /// Subscribe to render process Id changed event.


Would be good to add some additional context, "The render process ID can change when X happens"

david-risney

Looks good. Just some small wording updates. Please fix wording, complete PR, open new PR to main, and let me know link to new PR and I will schedule API review meeting.

david-risney · 2023-03-15T21:58:26Z

specs/IFrameMemoryUsage.md

+# Background
+The WebView2 team has been asked to provide support for monitoring iframe 
+memory usage. Developer needs to know how much memory each iframe consumes. 
+To do that, they need to get renderer process id for each iframe". However, 


Suggested change

To do that, they need to get renderer process id for each iframe". However,

To do that, they need to get the renderer process ID for each iframe. However,

david-risney · 2023-03-15T21:58:42Z

specs/IFrameMemoryUsage.md

+
+# Background
+The WebView2 team has been asked to provide support for monitoring iframe 
+memory usage. Developer needs to know how much memory each iframe consumes. 


Suggested change

memory usage. Developer needs to know how much memory each iframe consumes.

memory usage. The end developer needs to know how much memory each iframe consumes.

david-risney · 2023-03-15T21:59:07Z

specs/IFrameMemoryUsage.md

+The WebView2 team has been asked to provide support for monitoring iframe 
+memory usage. Developer needs to know how much memory each iframe consumes. 
+To do that, they need to get renderer process id for each iframe". However, 
+WV2 only tracks the render frame host for top-level iframes. This limits the 


Suggested change

WV2 only tracks the render frame host for top-level iframes. This limits the

WebView2 only tracks the render frame host for top-level iframes. This limits the

david-risney · 2023-03-15T22:00:20Z

specs/IFrameMemoryUsage.md

+memory usage. Developer needs to know how much memory each iframe consumes. 
+To do that, they need to get renderer process id for each iframe". However, 
+WV2 only tracks the render frame host for top-level iframes. This limits the 
+[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) apis for first-level iframes only. For instance, we can handle iframe created event for first-level iframe, 


Break long line and wording updates

Suggested change

[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) apis for first-level iframes only. For instance, we can handle iframe created event for first-level iframe,

[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) APIs

for first-level iframes only. For instance, we raise the FrameCreated event for first-level iframes,

david-risney · 2023-03-15T22:00:35Z

specs/IFrameMemoryUsage.md

+To do that, they need to get renderer process id for each iframe". However, 
+WV2 only tracks the render frame host for top-level iframes. This limits the 
+[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) apis for first-level iframes only. For instance, we can handle iframe created event for first-level iframe, 
+but not for nested iframes with depth greater than 1. Also, WV2 lacks an API to 


Suggested change

but not for nested iframes with depth greater than 1. Also, WV2 lacks an API to

but not for nested iframes with depth greater than 1. Also, WebView2 lacks an API to

david-risney · 2023-03-15T22:00:46Z

specs/IFrameMemoryUsage.md

+WV2 only tracks the render frame host for top-level iframes. This limits the 
+[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) apis for first-level iframes only. For instance, we can handle iframe created event for first-level iframe, 
+but not for nested iframes with depth greater than 1. Also, WV2 lacks an API to 
+get the renderer process id for its iframe.


Suggested change

get the renderer process id for its iframe.

get the renderer process ID for iframes.

david-risney · 2023-03-15T22:01:33Z

specs/IFrameMemoryUsage.md

+the renderer process id is changed for `CoreWebView2` or `CoreWebView2Frame`.
+
+Additionally, we propose to add the `FrameCreated` event to `CoreWebView2Frame`. 
+This event fires when a new frame is created.


Suggested change

This event fires when a new frame is created.

This event is raised when a new frame is created.

Add iframe memory usage API

JosephJin0815 added the API Proposal Review WebView2 API Proposal for review. label Feb 22, 2023

JosephJin0815 requested a review from david-risney February 22, 2023 17:41

david-risney requested changes Feb 22, 2023

View reviewed changes

david-risney requested changes Mar 4, 2023

View reviewed changes

david-risney requested changes Mar 9, 2023

View reviewed changes

aluhrs13 mentioned this pull request Mar 14, 2023

Get render process information from ICoreWebView2 object #3298

Open

david-risney requested changes Mar 15, 2023

View reviewed changes

Add iframe memory usage API

f354939

Add iframe memory usage API

JosephJin0815 force-pushed the IFrameMemoryUsage-draft branch from ec49b4c to f354939 Compare March 15, 2023 23:45

JosephJin0815 merged commit a22b3ac into IFrameMemoryUsage Mar 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add spec for IFrameMemoryUsage.md #3234

Add spec for IFrameMemoryUsage.md #3234

JosephJin0815 commented Feb 22, 2023

david-risney Feb 22, 2023

david-risney Feb 22, 2023

JosephJin0815 Feb 22, 2023

david-risney Feb 22, 2023

david-risney Feb 22, 2023

JosephJin0815 Feb 22, 2023

JosephJin0815 Mar 14, 2023

david-risney Feb 22, 2023

JosephJin0815 Feb 22, 2023

david-risney Mar 4, 2023

JosephJin0815 Mar 4, 2023

david-risney Mar 4, 2023

david-risney Mar 8, 2023

david-risney Mar 8, 2023

JosephJin0815 Mar 9, 2023

david-risney Mar 9, 2023

JosephJin0815 Mar 14, 2023

david-risney Mar 9, 2023

JosephJin0815 Mar 9, 2023

david-risney left a comment

david-risney Mar 15, 2023

david-risney Mar 15, 2023

david-risney Mar 15, 2023

david-risney Mar 15, 2023

david-risney Mar 15, 2023

david-risney Mar 15, 2023

david-risney Mar 15, 2023

	To do that, they need to get renderer process id for each iframe". However,
	To do that, they need to get the renderer process ID for each iframe. However,

	memory usage. Developer needs to know how much memory each iframe consumes.
	memory usage. The end developer needs to know how much memory each iframe consumes.

	WV2 only tracks the render frame host for top-level iframes. This limits the
	WebView2 only tracks the render frame host for top-level iframes. This limits the

	[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) apis for first-level iframes only. For instance, we can handle iframe created event for first-level iframe,
	[ICoreWebView2Frame2](https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2frame?view=webview2-1.0.1518.46) APIs
	for first-level iframes only. For instance, we raise the FrameCreated event for first-level iframes,

	but not for nested iframes with depth greater than 1. Also, WV2 lacks an API to
	but not for nested iframes with depth greater than 1. Also, WebView2 lacks an API to

	get the renderer process id for its iframe.
	get the renderer process ID for iframes.

	This event fires when a new frame is created.
	This event is raised when a new frame is created.

Add spec for IFrameMemoryUsage.md #3234

Add spec for IFrameMemoryUsage.md #3234

Conversation

JosephJin0815 commented Feb 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-risney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment