-
Notifications
You must be signed in to change notification settings - Fork 668
DYN-5245-DynamoRevit-CreateCustom UserDataFolder #13293
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
DYN-5245-DynamoRevit-CreateCustom UserDataFolder #13293
Conversation
Added code for create the WebView2 cache folder in the AppData location because when tries to create it inside C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit is crashing due to permissions.
|
@QilongTang related to this question, the initialization in constructor was since the beginning I'm just creating a method named InitializeBrowserAsync() and call it in the constructor, do you think it should not be initialized in the constructor? |
No, I did not see the previous call. I think @mjkkirschner could be right that previously everything was async so the view tests could finish before WebView2 was required |
| }; | ||
| } | ||
| notificationUIPopup.webView.CoreWebView2InitializationCompleted += WebView_CoreWebView2InitializationCompleted; | ||
| await notificationUIPopup.webView.EnsureCoreWebView2Async(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This await should not be required right? If our theory is correct, I think we can stick to the previous way of initilizing WebView2 async and apply the View closing fix as in your other PR. I will do some experiments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QilongTang well not sure if await is required or not but I knew that we should be awaiting async calls to finish otherwise we cannot control the flow of calls (if we need to use WebView2 later), for example in this case there is a call to RequestNotifications(); method before (in the original code was after), then not sure which problems could produce in the future but as you said let's test the behavior.
I will proceed to make the changes as you are suggesting and later we can do some experiments.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobertGlobant20 I made the change in #13295 and running tests. Because I see this as one of the big difference before/after in your previous PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: commit: 9dcf602
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobertGlobant20 For the RequestNotifications() call it should not affect WebView2 initialization, because the actual passing of information is bind to the event NavigationCompleted. So not until WebView2 was launched and navigated to the html, there would be no trial of passing info
Not using await for initializing WebView2
|
Tests passed merging. @RobertGlobant20 Please file the second PR based on this which would fix the crash |
* DYN-5245-Revit-NotificationCenter-Crashing Added code for create the WebView2 cache folder in the AppData location because when tries to create it inside C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit is crashing due to permissions. * DYN-5245-Revit-NotificationCenter-Crashing Code Review Not using await for initializing WebView2
* DYN-5245-DynamoRevit-CreateCustom UserDataFolder (#13293) * DYN-5245-Revit-NotificationCenter-Crashing Added code for create the WebView2 cache folder in the AppData location because when tries to create it inside C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit is crashing due to permissions. * DYN-5245-Revit-NotificationCenter-Crashing Code Review Not using await for initializing WebView2 * DYN-5245-DynamoRevit-Crash-WhenExiting (#13299) * DYN-5245-Revit-NotificationCenter-Crashing I added some code for stopping the WebView2 instance when Dynamo is being closed because otherwise is crashing and also not loading the notifications. Also the Dispose method was deleted due that we are doing the same in the new method added SuspendCoreWebviewAsync. * DYN-5245-Revit-NotificationCenter-Crashing Removing call to TrySuspendAsync(); Co-authored-by: Roberto T <61755417+RobertGlobant20@users.noreply.github.com>


Purpose
Added code for creating the WebView2 cache folder in the AppData location because when tries to create it inside C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit is crashing due to permissions then now will be created in the AppData location.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Added code for creating the WebView2 cache folder in the AppData location
Reviewers
@QilongTang
FYIs
@mjkkirschner