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

[ANNOUNCEMENT] User directory folder default handling updates coming. This is a potentially breaking change. #1410

Closed
DavidShoe opened this issue Jun 15, 2021 · 22 comments

Comments

@DavidShoe
Copy link
Contributor

The current implementation of WebView2 User Data Folder default handling is to create a directory alongside the WebView2 hosting application and use that for runtime content. This case is used when the developer has not specified the user directory folder. The intention of the default location is for developers to start prototyping WebView2 solutions with the least amount of resistance, but as a best practice (WebView2 development best practices - Microsoft Edge Development | Microsoft Docs), we recommend developers specify their own user data folder path and own the folder lifetime for production apps. However, with the current default location, we sometimes see the user data folder created alongside the application in locations with limited permissions such as Program Files, which results in malfunction of the control.

In an upcoming release we will be changing the way that the default folder is handled. Instead of just creating the folder next to the exe we will be creating the folder in the path returned by SHGetKnownFolderPath (FOLDERID_LocalAppData,…) MSDN Documentation. This is a windows standard process for getting a writable location to store application data.
There will be some additional attribution on the path to help reduce collisions in case there is more than one installed copy of the application.

To simplify finding out where your User Directory Folder has been created we are going to be introducing a new API that will report where exactly the User Directory Folder for this running WebView2 application process has been created. Developers can use this if they want to clean the folder for any reason.

Please note that this naming and implementation is preliminary and subject to change.
If your application is assuming the current default handling it could result a breaking change once the default is moved. For example if you are assuming the folder is created next to the exe and try to delete it, the user data folder won’t be there and your code could fail.

More details will be provided on the exact implementation and suggested handling of the new User Data Folder location when we are closer to releasing the new functionality.

@DavidShoe DavidShoe added the feature request feature request label Jun 15, 2021
@DavidShoe DavidShoe pinned this issue Jun 15, 2021
@champnic champnic changed the title User directory folder default handling updates coming. This is a potentially breaking change. [ANNOUNCEMENT] User directory folder default handling updates coming. This is a potentially breaking change. Jun 15, 2021
@champnic champnic removed the feature request feature request label Jun 15, 2021
@Symbai
Copy link

Symbai commented Jun 16, 2021

Instead of forcing a breaking change, it would be a LOT better if you make this change an OPT-IN for those who care. Because in most situations the executable folder is writable. Only in some business environments it might not. If the developer is not sure, he can opt-in.

Otherwise add some internal checks to see if user data is stored in executable and move it to the new folder yourself. Why should developers change tons of code just to deal with something that wasn't an issue before? I have several usages of WebView2 where I don't call EnsureCoreWebView2Async but point to the target site directly. In order to fix your breaking change I have to change all of these usages, calling EnsureCoreWebView2Async first (which btw. isnt that easy on non-async GUI because I also have to change the way this GUI is shown by blocking it until the async task returned).

@DavidShoe
Copy link
Contributor Author

@Symbai it is in a way, if the developer specifies the UDF this code will never execute. This is just for the defaulting case. Also it is more than just business environments, if a developer builds an installer that uses windows default install locations IE: "Program files" they will crash.

@champnic
Copy link
Member

@Symbai While this is a runtime change, we're currently looking into if we can tie it to a version of the SDK or higher. That way it wouldn't break existing apps, and would only effect apps that choose to adopt the newer SDK.

@pmaytak
Copy link

pmaytak commented Jun 30, 2021

Is there an expected timeline when this change will be released?

@DavidShoe
Copy link
Contributor Author

We are in active development right now, but not eta yet.

@FreddyDgh
Copy link

Hopefully I don't come across as insane, but I have strong opinions on this issue. I think that this is a really terrible idea for a default. I understand that one of the main goals is to allow libraries like MSAL to use WebView2 as a dependency, but not add WebView2-specific properties to its API surface. Great. Fantastic idea. However, this proposal doesn't actually do that.

When the default is to put the User Data Folder (UDF) in a folder next to the exe, the UDF gets cleaned up when the exe's bin folder is deleted, which is a reasonable default. If you put the default in LocalAppData, how is the UDF going to get cleaned up? If you have an installer for your app, then sure, you can have the installer clean it up. However, assuming that the app has a formal installer doesn't make any sense as a default. It will allow more situations to run correctly, but every app that doesn't have an installer will leak disk space by default.

You mention that you want to expose an API in WebView2 that you can manually call and would allow you to cleanup the folder. However, that defeats the entire purpose of changing the default! Let's say that MSAL wants to put a hello world sample app in their documentation. Nobody is going to create an installer for a MSAL hello world. That is an unreasonable default expectation. The choice for the MSAL team is to either:

  1. Not clean up the UDF folder ever (not appropriate for a default)
  2. Mention in the docs that after running the hello world sample, you will have to manually go into LocalAppData, find which folder the hello world app created, and delete it (a terribly annoying developer experience, and easy to overlook/forget)
  3. Put the WebView2-specific UDF cleanup API in the hello world code (the only right-way to do it, but again, this defeats the entire purpose, as the goal was not to require WebView2-specific APIs when only MSAL is being used)

Ultimately, this is a very common problem for libraries and is by no means WebView2-specific. I don't think it's even desirable to have the burden of this issue fall on WebView2. In fact, I think it would make a lot of sense to perhaps introduce an additional set of APIs for app storage management (perhaps under the Project Reunion banner?), although, I'm not sure how likely this is to happen. However, it would then allow libraries requiring disk storage to take hard dependencies on the storage management APIs rather than having different libraries come up with different implementations. The burden of finding a good default app folder on Windows would then fall on one single library, in one single place, and multiple libraries could then use a SINGLE implementation. In particular, the App Center SDK (which doesn't have anything to do with WebView2) comes to mind as having this same disk storage problem, with its own implementation to fix it, and the result is frustrating to use. A storage management API could also allow a developer to specify the app data folder information only once, instead of doing it for each component, which would actually be helpful for something like MSAL where they have a disk cache helper in their extensions library that could utilize the same API as well. Consider the experience of having different libraries with different patterns for determining storage locations:

private async Task SetupAppAsync()
{
    string appDataLocation = @"C:\CustomPath\AppData";

    var cacheHelper = await MsalCacheHelper.CreateAsync(new StorageCreationPropertiesBuilder("someFileName", appDataLocation).Build());
    var webView2Environment = await CoreWebView2Environment.CreateAsync(userDataFolder: appDataLocation);
}

versus having a common API that can handle finding the default app storage location, rather than delegating it to all the different libraries:

private async Task SetupAppAsync()
{
    string appDataLocation = @"C:\CustomPath\AppData";

    AppStorageManager.SetDefaultAppStorage(appDataLocation);

    var cacheHelper = await MsalCacheHelper.CreateAsync();
    var webView2Environment = await CoreWebView2Environment.CreateAsync();
}

I'll admit that I haven't necessarily fully thought it out, but it seems like creating such an API could be done relatively easily and would provide a ton of benefit to both first-party and third-party libraries. This issue is basically guaranteed to come up in every single Microsoft library repo that requires disk storage, and having each team spend time designing and coding multiple implementations seems absurd to me. Nevertheless, I can understand if Microsoft doesn't want to invest in developing a new API set for this.

If the final decision is that WebView2 must work out of the box in the Program Files directory with no additional code changes on the consumer end, I would strongly suggest that the LocalAppData option should be added AFTER the exe directory in the search order, not before. The search order for the UDF location should then be something like:

  1. User-specified directory, if set (MSAL would be hoping to avoid a WebView2-specific API for this, so this would be skipped from their point-of-view)
    • User is responsible for cleanup (via an uninstaller or any other means)
    • On failure, throw, don't try any other possible folder locations
  2. Isolated Storage (i.e. MSIX packaged app), if available
    • Uninstaller is reponsible for cleanup
    • On failure, try next possible location
  3. Same Directory as EXE
    • UDF will be cleaned up when EXE directory is removed
    • On failure (e.g. no write permission), try next possible location
  4. LocalAppData method
    • Unknown if cleanup is available - it might get cleaned up when the app is removed, but it might not (may leak system resources; libraries like MSAL would have to document this behavior)
    • On failure, throw

@Alex240
Copy link

Alex240 commented Jul 29, 2021

The possibility of choosing the path will be really welcome! This should simplify the developments. Could you indicate (approximately) when we can hope have this modification ?

@champnic
Copy link
Member

champnic commented Aug 2, 2021

@Alex240 You can already choose the user data folder path today when creating the CoreWebView2Environment:
https://docs.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/webview2-idl?view=webview2-1.0.902.49#createcorewebview2environmentwithoptions

@champnic
Copy link
Member

champnic commented Aug 2, 2021

@FreddyD-GH Sorry we didn't respond yet. I like the idea of a central Microsoft "AppStorage" API, I'll see if I can forward the suggestion on to the relevant team. It's unlikely that we would wait for that implementation to be approved, implemented, and shipped before we make a change here. In that vein, I also like the idea of not changing the search order if we don't have to, and instead default to the exe location unless it fails. @DavidShoe Can you take a look and see if that's something we can incorporate into our current design? Thanks!

@jasonstephen15 jasonstephen15 unpinned this issue Aug 13, 2021
@wilva
Copy link

wilva commented Aug 20, 2021

A silly question maybe, but is there any reason on not to use the same folder for the caching data as what Microsoft Edge is using?
Does that create issues with MS Edge or is this mostly for reasons of keeping sensitive data out of the normal caching folders?

@champnic
Copy link
Member

This is disallowed, and is indeed to keep data separate between the browser and the app. There are additional reasons around compatibility (a WebView2 app may be using an older runtime than the browser version, and they may have incompatible user data folder formats).

@wilva
Copy link

wilva commented Aug 21, 2021

That makes perfect sense.
Thank you for taking the time to answer my question.

@wilva
Copy link

wilva commented Aug 26, 2021

Any reason why not using a windows designed temp folder?

Eg. I'm hosting the control in Excel and thus by the original design for the user data folder it tries to create the folder like this:
'C:\Program Files (x86)\Microsoft Office\Root\Office16\EXCEL.EXE.WebView2'
Obviously that is in an area where by default the current user cannot write.

So instead I use the windows api GetTempPath to determine the user's temp folder and then apply the same name scheme as the WebView2 control already employs.
So in my particular scenario, the cache folder then becomes:
'C:\Users\Wil\AppData\Local\Temp\EXCEL.EXE.WebView2'

At least then the cache location is in an area where it is expected that temporary files are written.

@DavidShoe
Copy link
Contributor Author

There are scenarios where this makes perfect sense, if the data is truly transient or the application can function if that data is removed. But there are other cases where the application may want to persist data for longer than the temp directory is expected to keep it. If that path works for your application then by all means grab a temp dir and pass that into the webview2 when you create it. Just be aware you may not find any data persisting between executions due to system cleanup or other imposed rules on the temp directory.

@wilva
Copy link

wilva commented Aug 26, 2021

Great feedback, thanks!
I ask because it must have been considered and knowing why that wasn't chosen helps a lot.
This will probably work for most of my users, but I do intend to make it configurable anyways.

@wilva
Copy link

wilva commented Aug 27, 2021

Sorry for all the questions..

I found out already that WebView2 does not like it when you have different applications that try to use the same UserDataPath.
Eg. if both demo.exe and demo2.exe try to use "C:\UserDataFolder" then there will be an error the moment you start the second application when the first one is still running.
The error is "The group or resource is not in the correct state to perform the requested operation"
That is fine.

One of my customers now asks.
What happens if you start 2 instances of the same application?
If I try this, then there are no complaints from the WebView2 control.
But is it a safe thing to do?

@champnic
Copy link
Member

You can use the same user data folder (assuming both apps have access to it) as long as the command lines/environments used to start the WebView2 are the same in both cases. Otherwise you can get the error you mentioned. Lifetime management of the WebView2 does become more difficult in this scenario, as the apps will have to manage a shared browser process.

What happens if you start 2 instances of the same application?
If I try this, then there are no complaints from the WebView2 control.
But is it a safe thing to do?

A similar thing happens here. You will get the perf benefits of a shared browser and utility processes, but the content will remain isolated in separate renderer processes. You can see this behavior in Task Manager. This is generally safe to do, but can become difficult if you are trying to do things like shutdown all the process from one of the apps.

@wilva
Copy link

wilva commented Aug 27, 2021

Brilliant.
You are all so extremely helpful, thank you.
I was testing here and starting multiple processes works very well, even with things like IndexedDB which I was wondering if it could have issues. I had noticed that each process gets its own renderer processes, but wondered if sharing the files was problematic.

BTW, if there's a better place to ask these kind of questions, or if I missed an obvious "it is all documented there" place, then please do tell. I checked the doc of the .net control, but did not see the level of detail that is in this thread.

Thanks again.

@champnic
Copy link
Member

@wilva Any time Wil! Happy to help :)

Feel free to ask more questions as new Issues here in this GitHub repo. Besides our .NET reference docs, we also have Conceptual, How To, and Getting Started guides here: https://docs.microsoft.com/en-us/microsoft-edge/webview2/

Here's some info on sharing user data folders, for example:
https://docs.microsoft.com/en-us/microsoft-edge/webview2/concepts/user-data-folder#share-user-data-folders

Cheers!

@hunkydoryrepair
Copy link

You can use the same user data folder (assuming both apps have access to it) as long as the command lines/environments used to start the WebView2 are the same in both cases. Otherwise you can get the error you mentioned. Lifetime management of the WebView2 does become more difficult in this scenario, as the apps will have to manage a shared browser process.

What influences the command lines/environments being the same? I'm not sure what all goes into the command line.

I am getting an error in the scenario I have 2 instances, where the EXEs are in separate folders, but they are set to share the same working folder in the writable user area. They are the exact EXE, and set all the same settings, just run from different locations. The only argument passed to CoreWebView2Environment.CreateAsync is the userDir.

I'm not seeing the actual error, but my CoreWebView2InitializationCompleted event is getting triggered when CoreWebView2 is still null (WinForms).

I'm guessing it may be an issue with browserExecutableFolder being different?

@champnic
Copy link
Member

champnic commented Nov 2, 2021

Hey @hunkydoryrepair - Having the same user data folder implies using the same shared set of browser processes, which is why the command lines have to (nearly) match. Typically this means 1) not setting different flags in CoreWebView2EnvironmentOptions.AdditionalBrowserArgs, and 2) that the apps have the same DPI awareness. Besides the command line, it also means the runtime needs to match, so in this case the browserExectuableFolder's should be pointing to the same location (or kept as the default null to use the evergreen runtime loading logic).

You can try checking the IsSuccess and InitializationException properties of the event args when handling CoreWebView2InitializationCompleted:
https://docs.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.core.corewebview2initializationcompletedeventargs?view=webview2-dotnet-1.0.1056-prerelease

@DavidShoe
Copy link
Contributor Author

Following up on this. we have continued investigations of how the UDF is being used and how our developers want to move forward. At this time we are not going to make this change. We may revisit this issue in the future.

Use of the default handling is still discouraged as we feel the developers should be making a clear design decision for long term data storage. We have added a runtime check and a debug message if the default handling is detected, this will only be seen if running under a debugger.

"WebView2 Warning: Using default User Data Folder is not recommended, please see documentation. https://go.microsoft.com/fwlink/?linkid=2187341"

If you are using the default handling intentionally you can ignore this warning.

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

No branches or pull requests

8 participants