API Review: multiple profile support by leonhsl · Pull Request #1717 · MicrosoftEdge/WebView2Feedback

leonhsl · 2021-09-08T03:38:46Z

This is an API review for WebView2 multiple profile support. Thanks.

Co-authored-by: David Risney <dave@deletethis.net>

Add spec for MultiProfile.md

MikeHillberg · 2021-09-14T01:28:04Z

+public CreateWebView2ControllerWithOptions(IntPtr parentHWND, string profileName, bool isInPrivate)
+{
+    CoreWebView2ControllerOptions options = _webViewEnvironment.CreateCoreWebView2ControllerOptions(profileName, isInPrivate);
+    CoreWebView2Controller webView2Controller = await _webViewEnvironment.CreateCoreWebView2ControllerWithOptionsAsync(parentHWND, options);


Hopefully the method is overloaded and don't need the "WithOptions"

Its not but it should be. Add overloads for WinRT and .NET

MikeHillberg · 2021-09-14T03:58:24Z

+            String ProfileName, Boolean InPrivateModeEnabled);
+
+        Windows.Foundation.IAsyncOperation<CoreWebView2Controller>
+        CreateCoreWebView2ControllerWithOptionsAsync(


Add [overload("CreateCoreWebView2ControllerAsync")]

Yes please add for .NET and WinRT

MikeHillberg · 2021-09-14T03:59:35Z

+            CoreWebView2ControllerOptions options);
+
+        Windows.Foundation.IAsyncOperation<CoreWebView2CompositionController>
+        CreateCoreWebView2CompositionControllerWithOptionsAsync(


Add [overload("CreateCoreWebView2CompositionController")]

Yes please add for .NET and WinRT

MikeHillberg · 2021-09-14T04:01:39Z

+        // ...
+
+        CoreWebView2ControllerOptions CreateCoreWebView2ControllerOptions(
+            String ProfileName, Boolean InPrivateModeEnabled);


camelCase parameter names ("profileName" and "inPrivateModeEnabled"). (Same in methods below.)

These parameters were removed in a previous comment above.

Done. Thanks.

oldnewthing · 2021-09-14T17:30:57Z

+Although you can make your WebView2s use different user data directories to achieve data separation,
+in such way you'll have to be running multiple browser instances (each including a browser process
+and a bunch of child processes), which means much more consumption for system resources including
+memory, CPU footprint, disk space etc. so it is not desirable.


From the parenthetical, I think the difference between separate user data folders and separate profiles within the same user data folder is that with separate user data folders, there is no browser process sharing at all. With shared user data but separate profiles, there is process sharing, but no state sharing. This allows reduced CPU and memory usage, since multiple profiles share the same process.

Not clear what the disk space reduction benefit is, though. Do profile share caches?

Profiles share some state but I'm not sure what state is shared. Please investigate and document.

I think having multiple user data folders consume more disk space than having just one user data folder containing multiple profile folders. The disk space reduction benefit comes from here. Profiles do not share caches, but we can observe that there're a bunch of sub folders other than profile sub folders under the user data directory.

Please add to the documentation some examples of content that is shared.

Thanks! I've added to the documentation. There're some other files and folders in user data directory besides profile folders, such as GrShaderCache(handles caching compiled shaders for a Skia GrContext, about 10MB in my sample), SmartScreen(help keep user's safe while browse the web, about 500KB in my sample) and etc. If we can use multiple profiles sharing one user data directory, we can share these files and folders, thus will save some disk space.

oldnewthing · 2021-09-14T17:32:52Z

+  ///    and '#', '@', '$', '(', ')', '+', '-', '_', '~', '.', ' ' (space).
+  /// Note: the text must not end with a period '.' or ' ' (space). And, although upper case letters are
+  /// allowed, they're treated just as lower case couterparts because the profile name will be mapped to
+  /// the real profile directory path on disk and Windows file system handles path names in a case-insensitive way.


Presumably that means that leading space is also disallowed. Also, I can't name a profile nul or com1.

Document the additional restriction that they must be valid filenames (which also means no leading spaces, and nul and com* are disallowed) and then have 'For example: ' and link to what is valid file name document.

Something like:

/// The `ProfileName` property specifies the profile's name. It has a maximum length of 64 /// characters excluding the null terminator and must be a valid file name. See [Naming Files, Paths, and Namespaces](https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file) /// for more information on file names. It must contain only the following ASCII characters: /// * alphabet characters: a-z and A-Z /// * digit characters: 0-9 /// * and '#', '@', '$', '(', ')', '+', '-', '_', '~', '.', ' ' (space). /// /// Note: the text must not end with a period '.' or ' ' (space) nor start with a ' ' (space). And, although upper case letters are /// allowed, they're treated the same as their lower case counterparts because the profile name will be mapped to /// the real profile directory path on disk and Windows file system handles path names in a case-insensitive way.

Added. Thanks.

oldnewthing · 2021-09-14T17:34:30Z

+        m_webviewOption.profile.c_str(), m_webviewOption.isInPrivate, options.GetAddressOf());
+    if (hr == E_INVALIDARG)
+    {
+        ShowFailure(hr, L"Unable to create WebView2 due to an invalid profile name.");


Should the sample do its own profile name validation, rather than allowing invalid parameters to be passed through?

Yes. Apps should either be using hardcoded names (or otherwise computed/generated values) or taking user input. In the first case presumably we don't need to validate the values. In the second case, for user input the app should validate that the input is allowed before calling CreateCoreWebView2ControllerOptions

The sample uses the user input as ProfileName and will validate ProfileName when calling put_ProfileName function. When calling CreateCoreWebView2ControllerOptions, put_ProfileName will also be raised in it but the default empty string will be passed as the parameter.
For this sample, I think it tells the developer that CreateCoreWebView2ControllerOptions and put_ProfileName will validate the input parameter and will return E_INVALIDARG if passing invalid parameter. And the limitation of ProfileName is on documentation that developers can get easily. If add validation code in sample, it would be almost the same as code in function put_ProfileName. So I'm not sure should I still need to add its own validation code in sample?

oldnewthing · 2021-09-14T17:37:15Z

+    }
+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(


Since the ProfileName and IsInPrivateModeEnabled are both settable properties, do we even need these extra constructor parameters? Can they just default to L"Default" and false? That would align with any future Options properties we add later. For example, maybe you want to use the default profile in InPrivate mode. Or we add some future property like "UseRemoteHolographic" and you want to create a default profile, but with remote holographic support.

Yes let's have no in parameters to HRESULT CreateCoreWebView2ControllerOptions(ICoreWebView2ControllerOptions**) and just use the settable properties to make this consistent with future properties we add to the profile. The value for profile name should be the empty string, and the default value for IsInPrivate should be false.

Good idea! I changed the interface to accept only ICoreWebView2ControllerOptions parameter. Please help check. Thanks.

oldnewthing · 2021-09-14T17:51:31Z

+        String ProfilePath { get; };
+    }
+}
+```


There appears to be no way to enumerate profiles or delete unwanted ones. Is the only way to delete a profile to delete the entire user data folder (which deletes all the profiles)?

We will consider adding based on customer feedback.

oldnewthing · 2021-09-14T17:55:28Z

+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(
+        m_webviewOption.profile.c_str(), m_webviewOption.isInPrivate, options.GetAddressOf());


From what I can tell, this is the only way to create an InPrivate WebView: To create a profile in InPrivate mode. Is that right?

Correct. Document better how inPrivate interacts with profiles.

As WebView2 is built on top of Edge browser, it follows Edge's behavior pattern. To create an InPrivate WebView, we gets an off-the-record profile (an InPrivate profile) from a regular profile, then create the WebView with the off-the-record profile.

oldnewthing · 2021-09-14T17:58:25Z

+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(
+        m_webviewOption.profile.c_str(), m_webviewOption.isInPrivate, options.GetAddressOf());


What happens if I create a Webview2Controller with { Profile="Shopping", InPrivate = false } and then later create another one with { Profile = "Shopping", InPrivate = true }? Is that legal?

What happens if I create a Webview2Controller with { Profile="Shopping", InPrivate = true } and never create a non-InPrivate profile. When the WebView closes, is the profile destroyed? Or does the profile still hang around, with nothing in it?

Must investigate and document this.

In the first case, we create two controllers using the same profile but one with InPrivate set and one not. Are both allowed to run at the same time? I assume so because it works like that for the browser. Please investigate and document it in the reference documentation for CreateCoreWebView2ControllerWithOptions.

In the second case, what happens if you create a profile with InPrivate set and that is the first time the profile gets created? Will the profile exist on disk once the corresponding controller is closed? Please investigate and document in the reference documentation for CreateCoreWebView2ControllerWithOptions.

Yes in the first case, they are both allowed to run at the same time.

In the second case, the profile will be created on disk at the first time, and it will be released in memory when the corresponding controller is closed, but will still remain on disk.

jonwis · 2021-09-14T19:57:26Z

+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(
+        m_webviewOption.profile.c_str(), m_webviewOption.isInPrivate, options.GetAddressOf());


Is there a way to tell the WV2 to put the profile in a directory under the app's control? That way - especially for Desktop Bridge apps - the app/system can remove the profile directory during it uninstall phase.

Great question: yes indirectly. Profiles are sub folders under the CoreWebView2's user data folder. The host app optionally provides the path for the user data folder when creating the CoreWebView2 environment. WinUI doesn't provide a way for the end dev to set the user data folder right now but there's a scenario for it on them.

The profile is a subdirectory of the user data directory, and the app got to choose its user data directory location.

jonwis · 2021-09-14T19:58:53Z

+        m_profileDirName = str.substr(str.find_last_of(L'\\') + 1);
+
+        // update window title with m_profileDirName
+        UpdateAppTitle();


I guess the sample could show the uninstall making one last profile so it can find and delete the content?

jonwis · 2021-09-14T20:00:55Z

+interface ICoreWebView2Profile;
+
+[uuid(C2669A3A-03A9-45E9-97EA-03CD55E5DC03), object, pointer_default(unique)]
+interface ICoreWebView2ControllerOptions : IUnknown {


Is there a point at which we'll convert the interface to MIDL3, so you don't have to dual-author the C++ and C# interop types? We'll have Microsoft.Windows.UI.Core.WindowId at some point so you don't have to work around HWND missing in WinRT metadata.

TLDR: In the long term yes I'd like to make our IDL3 the source of truth and from it generate our .NET & COM APIs or replace those with just WinRT

This is our COM IDL(1) style IDL file to define our COM APIs usable down to Win7. Because it goes to Win7 where we don't have various aspects of WinRT available the conventions of the COM API are similar to but are not the same as the WinRT ABI. We later added WinRT support in part by generating a MIDL3 from our COM IDL in a tool that understands how to translate between our conventions and WinRT. In the future I'd like to make MIDL3 the source of truth from which we generate our COM IDL and then after that if usage drops far enough deprecating in favor of COM ABI WinRT.

david-risney · 2021-09-14T20:06:46Z

+Although you can make your WebView2s use different user data directories to achieve data separation,
+in such way you'll have to be running multiple browser instances (each including a browser process
+and a bunch of child processes), which means much more consumption for system resources including
+memory, CPU footprint, disk space etc. so it is not desirable.


Profiles share some state but I'm not sure what state is shared. Please investigate and document.

david-risney · 2021-09-14T20:07:55Z

+    }
+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(


Yes let's have no in parameters to HRESULT CreateCoreWebView2ControllerOptions(ICoreWebView2ControllerOptions**) and just use the settable properties to make this consistent with future properties we add to the profile. The value for profile name should be the empty string, and the default value for IsInPrivate should be false.

david-risney · 2021-09-14T20:09:23Z

+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(
+        m_webviewOption.profile.c_str(), m_webviewOption.isInPrivate, options.GetAddressOf());


Correct. Document better how inPrivate interacts with profiles.

david-risney · 2021-09-14T20:23:56Z

+        // ...
+
+        CoreWebView2ControllerOptions CreateCoreWebView2ControllerOptions(
+            String ProfileName, Boolean InPrivateModeEnabled);


These parameters were removed in a previous comment above.

david-risney · 2021-09-14T20:24:14Z

+            String ProfileName, Boolean InPrivateModeEnabled);
+
+        Windows.Foundation.IAsyncOperation<CoreWebView2Controller>
+        CreateCoreWebView2ControllerWithOptionsAsync(


Yes please add for .NET and WinRT

david-risney · 2021-09-14T20:24:18Z

+            CoreWebView2ControllerOptions options);
+
+        Windows.Foundation.IAsyncOperation<CoreWebView2CompositionController>
+        CreateCoreWebView2CompositionControllerWithOptionsAsync(


Yes please add for .NET and WinRT

david-risney · 2021-09-14T20:26:07Z

+        String ProfilePath { get; };
+    }
+}
+```


We will consider adding based on customer feedback.

david-risney · 2021-09-14T20:36:41Z

+
+    Microsoft::WRL::ComPtr<ICoreWebView2ControllerOptions> options;
+    HRESULT hr = webViewEnvironment4->CreateCoreWebView2ControllerOptions(
+        m_webviewOption.profile.c_str(), m_webviewOption.isInPrivate, options.GetAddressOf());


Great question: yes indirectly. Profiles are sub folders under the CoreWebView2's user data folder. The host app optionally provides the path for the user data folder when creating the CoreWebView2 environment. WinUI doesn't provide a way for the end dev to set the user data folder right now but there's a scenario for it on them.

david-risney · 2021-09-14T21:06:49Z

+interface ICoreWebView2Profile;
+
+[uuid(C2669A3A-03A9-45E9-97EA-03CD55E5DC03), object, pointer_default(unique)]
+interface ICoreWebView2ControllerOptions : IUnknown {


TLDR: In the long term yes I'd like to make our IDL3 the source of truth and from it generate our .NET & COM APIs or replace those with just WinRT

This is our COM IDL(1) style IDL file to define our COM APIs usable down to Win7. Because it goes to Win7 where we don't have various aspects of WinRT available the conventions of the COM API are similar to but are not the same as the WinRT ABI. We later added WinRT support in part by generating a MIDL3 from our COM IDL in a tool that understands how to translate between our conventions and WinRT. In the future I'd like to make MIDL3 the source of truth from which we generate our COM IDL and then after that if usage drops far enough deprecating in favor of COM ABI WinRT.

Co-authored-by: David Risney <dave@deletethis.net> Co-authored-by: Raymond Chen <oldnewthing@users.noreply.github.com>

leonhsl and others added 23 commits August 17, 2021 15:58

Create MultiProfile.md

f9b83cd

Update MultiProfile.md

abc08f4

Update specs/MultiProfile.md

f06eeff

Co-authored-by: David Risney <dave@deletethis.net>

Update specs/MultiProfile.md

a38eba6

Co-authored-by: David Risney <dave@deletethis.net>

Update MultiProfile.md

9288f89

Update MultiProfile.md

27c375b

Update MultiProfile.md

4341fb8

Apply suggestions from code review

25e1789

Co-authored-by: David Risney <dave@deletethis.net>

Update MultiProfile.md

065b3cf

Update MultiProfile.md

f46122e

Update MultiProfile.md

1ebcad2

Update specs/MultiProfile.md

e441785

Co-authored-by: David Risney <dave@deletethis.net>

Update MultiProfile.md

d4934af

Update MultiProfile.md

7fcaa4a

Update MultiProfile.md

7186917

Update MultiProfile.md

621b94a

Update MultiProfile.md

5608db3

Update MultiProfile.md

ca72063

Update MultiProfile.md

33a7ec0

Update specs/MultiProfile.md

8fc5c5b

Co-authored-by: David Risney <dave@deletethis.net>

Update MultiProfile.md

466ef32

Update MultiProfile.md

a51e651

Merge pull request #1654 from MicrosoftEdge/api-multiprofile-draft

a2fa44b

Add spec for MultiProfile.md

leonhsl requested a review from david-risney September 8, 2021 03:40

leonhsl added the API Proposal Review WebView2 API Proposal for review. label Sep 8, 2021

leonhsl self-assigned this Sep 8, 2021

david-risney reviewed Sep 8, 2021

View reviewed changes

Comment thread specs/MultiProfile.md

Update MultiProfile.md

e48e00d

This was referenced Sep 10, 2021

API Review: multiple profile support MicrosoftEdge/WebView2Announcements#30

Closed

Exception with --incognito and --profile-directory options specified #1385

Closed

MikeHillberg reviewed Sep 13, 2021

View reviewed changes

Comment thread specs/MultiProfile.md Outdated

Comment thread specs/MultiProfile.md Outdated

Comment thread specs/MultiProfile.md

MikeHillberg reviewed Sep 14, 2021

View reviewed changes

oldnewthing reviewed Sep 14, 2021

View reviewed changes

Comment thread specs/MultiProfile.md

jonwis reviewed Sep 14, 2021

View reviewed changes

david-risney requested changes Sep 14, 2021

View reviewed changes

leonhsl and others added 11 commits September 15, 2021 15:47

Apply suggestions from code review

6c53f26

Co-authored-by: David Risney <dave@deletethis.net> Co-authored-by: Raymond Chen <oldnewthing@users.noreply.github.com>

Update MultiProfile.md

e7c2a27

Update MultiProfile.md

2a2f937

some comments fix

c5ca396

Update MultiProfile.md

f8e3784

MultiProfile.md

d23c6a1

Update MultiProfile.md

16328cd

MultiProfile.md

17aa785

Update MultiProfile,md

5ceb019

Merge branch 'api-multiprofile-llc' into api-multiprofile

7edf2cb

add example about disk saving

6779e00

david-risney approved these changes Sep 28, 2021

View reviewed changes

david-risney added the review completed WebView2 API Proposal that's been reviewed and now needs final update and push label Sep 28, 2021

add overload methods for .NET & WinRT

c23c19c

lichen63 merged commit b6da5fb into master Oct 8, 2021

lichen63 deleted the api-multiprofile branch October 8, 2021 06:47

lichen63 restored the api-multiprofile branch October 18, 2021 02:12

Conversation

leonhsl commented Sep 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-risney Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-risney Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-risney Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-risney Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lichen63 Sep 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oldnewthing Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-risney Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-risney Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

david-risney Sep 14, 2021 •

edited

Loading

david-risney Sep 14, 2021 •

edited

Loading

david-risney Sep 14, 2021 •

edited

Loading

david-risney Sep 14, 2021 •

edited

Loading

lichen63 Sep 17, 2021 •

edited

Loading

oldnewthing Sep 14, 2021 •

edited

Loading

david-risney Sep 14, 2021 •

edited

Loading

david-risney Sep 14, 2021 •

edited

Loading

david-risney Sep 14, 2021 •

edited

Loading