DYN-5208-DynamoRevit-Crash-Fix #13245

RobertGlobant20 · 2022-08-25T18:58:17Z

Purpose

Fixing crash in Dynamo for Revit when initializing WebView2.
DocumentationBrowser was crashing in Dynamo for Revit due that WebView2 was trying to create the cache folder in the path "C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit" due to permissions so in this fix I'm changing the folder to use the AppData for Revit "C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Revit\2.16".
The same medicin was applied for the WebView2 used in Guided Tours, when using the ResourceUtilities.LoadWebBrowser() method it will using the cache folder passed as parameter.

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated

Release Notes

Fixing crash in Dynamo for Revit when initializing WebView2.

Reviewers

@QilongTang

FYIs

@avidit

commit c81f3fe Merge: 98ff9c0 7240196 Author: t-tellro <[email protected]> Date: Thu Aug 25 12:46:28 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit 98ff9c0 Author: t-tellro <[email protected]> Date: Thu Aug 25 12:32:33 2022 -0500 DYN-5106-WebView2-DocumentationBrowser DocumentationBrowser was crashing in Dynamo for Revit due that WebView2 was trying to create the cache folder in the path "C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit" due to permissions so in this fix I'm changing the folder to use the AppData for Revit "C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Revit\2.16". The same medicin was applied for the WebView2 used in Guided Tours, when using the ResourceUtilities.LoadWebBrowser() method it will using the cache folder passed as parameter. commit 715626f Merge: 979f0fc 3491e96 Author: t-tellro <[email protected]> Date: Tue Aug 16 13:12:47 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit 979f0fc Author: t-tellro <[email protected]> Date: Mon Aug 15 22:46:10 2022 -0500 DYN-5106-WebView2-DocumentationBrowser Code Review Fixing CanCreateNodeDocumenationHtmlFromNodeAnnotationEventArgsWithPackageNodeWithAddtionalDocumentation test due that it was using and old format of the html <img> tab, then I updated the image to use the virtual folder and http. commit 284eb7a Merge: ab4326d b95ef11 Author: Roberto T <[email protected]> Date: Mon Aug 15 13:48:44 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit ab4326d Merge: dc993a0 fc26f34 Author: t-tellro <[email protected]> Date: Mon Aug 15 13:43:15 2022 -0500 Merge branch 'DYN-5106-WebView2-DocumentationBrowser' of https://github.com/RobertGlobant20/Dynamo into DYN-5106-WebView2-DocumentationBrowser commit dc993a0 Author: t-tellro <[email protected]> Date: Mon Aug 15 13:42:26 2022 -0500 DYN-5106-WebView2-DocumentationBrowser Code Review - changing the visibility for the property FallbackDirectoryName - renaming coreDir by docsDir commit fc26f34 Merge: b0526ab 933f16c Author: Aaron (Qilong) <[email protected]> Date: Mon Aug 15 10:00:24 2022 -0700 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit b0526ab Author: t-tellro <[email protected]> Date: Fri Aug 12 18:52:22 2022 -0500 DYN-5106-WebView2-DocumentationBrowser Code Review Due that the fallback_doc directory path can change depending if Dynamo is executed as Sandbox or if is over Revit then the path can be different so I added a code that will create the virtual directory depending of the correct fallback_doc directory path. Also I modified the Md2Html class so now it will be inserting in the image source (<img src) the prefix "http://appassets" so it can be loaded from a virtual directory created by the DocumentationBrowser. commit 417069a Merge: 0c12dff bef463e Author: t-tellro <[email protected]> Date: Thu Aug 11 18:11:39 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit 0c12dff Author: t-tellro <[email protected]> Date: Thu Aug 11 18:01:50 2022 -0500 DYN-5106-WebView2-DocumentationBrowser I replaced the WebBrowser component by WebView2 in the file DocumentationBrowserView.xaml, also I added some functions for initializing WebView2, The method ShouldAllowNavigation was modified due that WebView2 doesn't have the Navigating event and also the event handler receive different parameters then modified the function to implement the same behavior than the previous one. Also I noticed that WebView2 CORS was blocking to load the jpg images that are shown for several nodes, then I had to create a mapped virtual directory in this way the html will be able to load the images and show them in the web page. Finally I removed a tag in all the html that was making the web page compatible for IE based browsers (due that now we are using WebView2 a Chromium based browser).

Removing comment.

RobertGlobant20 · 2022-08-25T18:59:29Z

This GIF is showing the behavior of Dynamo in Revit.

mjkkirschner · 2022-08-25T19:00:31Z

src/DynamoCoreWpf/Views/GuidedTour/PopupWindow.xaml.cs

@@ -31,6 +31,11 @@ public partial class PopupWindow : Popup
        //Assembly path to the Resources folder
        private const string resourcesPath = "Dynamo.Wpf.Views.GuidedTour.HtmlPages.Resources";

+        /// <summary>
+        /// This property will be hold the path of the WebView2 cache folder, the value will change based in if DynamoSandbox is executed or Dynamo from Revit is executed


consider this generally, it's not just DynamoRevit, what about DynamoForCivil or Dynamo Formit etc?

Comment updated: 45e845b

QilongTang · 2022-08-26T01:37:08Z

src/DocumentationBrowserViewExtension/DocumentationBrowserViewExtension.cs

-                var docsDir = new DirectoryInfo(Path.Combine(pathManager.HostApplicationDirectory, FALLBACK_DOC_DIRECTORY_NAME));
+                //when running over Revit HostApplicationDirectory = C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit\Revit
+                //Then we need to remove the Revit folder from the path so we can find the fallback_docs directory.
+                var hostAppDirectory = Directory.GetParent(pathManager.HostApplicationDirectory).FullName;


We may want to double check on this if there is a better way. I will get back to you

So for Sandbox, this pathManager.HostApplicationDirectory would be null? I think this only means this is not an in process integration, for case like FormIt, where it is out of process but still delivered in programe files, this may still not work

@QilongTang
For other products like FormIt or Civil not sure which is the path of the Dynamo "fallback_doc" folder, I followed the implementation that I found in PackageDocumentationManager.cs (check link below) then if the fallback_doc folder location is different for each host (Revit, Civil, Formit) then the implementation done in PackageDocumentationManager is not working or hostDynamoFallbackDocPath = null or there is another process creating the folder and moving all the md/jpg files to the HostApplicationDirectory path.

Should I install FormIt and Civil so I can test if images are shown in DocumentationBrowser?

Dynamo/src/DocumentationBrowserViewExtension/PackageDocumentationManager.cs

Line 66 in 79ebac3

if (!string.IsNullOrEmpty(pathManager.HostApplicationDirectory))

@QilongTang now the WebView2 cache folder will be always created in AppData, see the next commit:
e101e8a

@RobertGlobant20 Is the comment above still accurate?

@QilongTang fixed comment about Revit in the next commit:
2729c08

Updating comment

Fix that will create the WebView2 cache folder always in the AppData path. e.g. Revit C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Revit\2.16 Sandbox C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Core\2.16

QilongTang · 2022-08-29T14:51:12Z

src/DynamoCoreWpf/UI/GuidedTour/GuidesManager.cs

+                    if(popupWindow != null)
+                    {
+                        popupWindow.WebBrowserUserDataFolder = userDataFolder != null ? userDataFolder.FullName : string.Empty;
+                    }


Why do we need this code here? Shouldn't this be set one time only at the Welcome step? Is it because all of our Welcome steps not leveraging WebView2?

@QilongTang
1st question:

we need to set the WebBrowserUserDataFolder before calling the WebView2.EnsureCoreWebView2Async() method so WebView2 can set the cache folder in the right location, I put it here because in GuidesManager we have access to DynamoViewModel.Model.PathManager.

2nd question.

I did a change that will be set only for popups that use html, check the commit below.

3rd question.

Not all the Welcome steps use html then it will be set only for the ones using html (and by consequence WebView2).

commit:
b37dcc1

@RobertGlobant20 are there any MSDN docs on how these webview2 startup options affect other instances of WebView2 in the same process?

@mjkkirschner MS documentation doesn't talk specifically about that but I think the startup options are specific for each WebView2 instance.
If you question is because guided tours and documentationbrowser will be using the same cache - user data folder (UDF) folder, MS Documentation says "It's possible for your host app to overlap them" but doesn't say anything about the consequences, I just opened the "User Interface" tour that opens the DocumentationBrowser at the same time than a Popup opened but I didn't see any problem (the Popup is not using html for this case), then once Notification Center is completed we will need to see what happen if we open the Notifications Center and the DocumentationBrowser at the same time.
https://docs.microsoft.com/en-us/microsoft-edge/webview2/concepts/user-data-folder?tabs=win32

@RobertGlobant20 thanks- I'm also curious because inside of Hosts like Revit, they will use WebView2 and so will the other addins running in the Host process. It's the wild west!

@mjkkirschner well, we asked Revit about it but for my understanding they are still in the process of adopting WebView2 then they haven't deal with this feature yet so if Revit team decide to use another cache folder in AppData for WebView2 we can evaluate a change in Dynamo to use the same than them, another thing is that we need to ask if this feature of enable/disable cache for WebView2 will be enabled in the future (since I didn't find anything about it).
Also I checked with AutoCAD team and they said they create the folder in the user profile (consider that Autocad is using C++).
Thanks

QilongTang

LGTM with some comments

Fix that will set the WebView2.UserDataFolder only when the popup is using an html page.

QilongTang

Please see one of my comments about updating the comments about revit, I dont think it is up to date. Otherwise, LGTM

Updating comment due that was specific to Revit.

QilongTang · 2022-08-29T22:38:40Z

@RobertGlobant20 Please cherry-pick this PR

* Squashed commit of the following: commit c81f3fe Merge: 98ff9c0 7240196 Author: t-tellro <[email protected]> Date: Thu Aug 25 12:46:28 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit 98ff9c0 Author: t-tellro <[email protected]> Date: Thu Aug 25 12:32:33 2022 -0500 DYN-5106-WebView2-DocumentationBrowser DocumentationBrowser was crashing in Dynamo for Revit due that WebView2 was trying to create the cache folder in the path "C:\Program Files\Autodesk\Revit 2023\AddIns\DynamoForRevit" due to permissions so in this fix I'm changing the folder to use the AppData for Revit "C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Revit\2.16". The same medicin was applied for the WebView2 used in Guided Tours, when using the ResourceUtilities.LoadWebBrowser() method it will using the cache folder passed as parameter. commit 715626f Merge: 979f0fc 3491e96 Author: t-tellro <[email protected]> Date: Tue Aug 16 13:12:47 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit 979f0fc Author: t-tellro <[email protected]> Date: Mon Aug 15 22:46:10 2022 -0500 DYN-5106-WebView2-DocumentationBrowser Code Review Fixing CanCreateNodeDocumenationHtmlFromNodeAnnotationEventArgsWithPackageNodeWithAddtionalDocumentation test due that it was using and old format of the html <img> tab, then I updated the image to use the virtual folder and http. commit 284eb7a Merge: ab4326d b95ef11 Author: Roberto T <[email protected]> Date: Mon Aug 15 13:48:44 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit ab4326d Merge: dc993a0 fc26f34 Author: t-tellro <[email protected]> Date: Mon Aug 15 13:43:15 2022 -0500 Merge branch 'DYN-5106-WebView2-DocumentationBrowser' of https://github.com/RobertGlobant20/Dynamo into DYN-5106-WebView2-DocumentationBrowser commit dc993a0 Author: t-tellro <[email protected]> Date: Mon Aug 15 13:42:26 2022 -0500 DYN-5106-WebView2-DocumentationBrowser Code Review - changing the visibility for the property FallbackDirectoryName - renaming coreDir by docsDir commit fc26f34 Merge: b0526ab 933f16c Author: Aaron (Qilong) <[email protected]> Date: Mon Aug 15 10:00:24 2022 -0700 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit b0526ab Author: t-tellro <[email protected]> Date: Fri Aug 12 18:52:22 2022 -0500 DYN-5106-WebView2-DocumentationBrowser Code Review Due that the fallback_doc directory path can change depending if Dynamo is executed as Sandbox or if is over Revit then the path can be different so I added a code that will create the virtual directory depending of the correct fallback_doc directory path. Also I modified the Md2Html class so now it will be inserting in the image source (<img src) the prefix "http://appassets" so it can be loaded from a virtual directory created by the DocumentationBrowser. commit 417069a Merge: 0c12dff bef463e Author: t-tellro <[email protected]> Date: Thu Aug 11 18:11:39 2022 -0500 Merge branch 'master' into DYN-5106-WebView2-DocumentationBrowser commit 0c12dff Author: t-tellro <[email protected]> Date: Thu Aug 11 18:01:50 2022 -0500 DYN-5106-WebView2-DocumentationBrowser I replaced the WebBrowser component by WebView2 in the file DocumentationBrowserView.xaml, also I added some functions for initializing WebView2, The method ShouldAllowNavigation was modified due that WebView2 doesn't have the Navigating event and also the event handler receive different parameters then modified the function to implement the same behavior than the previous one. Also I noticed that WebView2 CORS was blocking to load the jpg images that are shown for several nodes, then I had to create a mapped virtual directory in this way the html will be able to load the images and show them in the web page. Finally I removed a tag in all the html that was making the web page compatible for IE based browsers (due that now we are using WebView2 a Chromium based browser). * DYN-5208-DynamoRevit-Crash-Fix Removing comment. * DYN-5208-DynamoRevit-Crash-Fix Updating comment * DYN-5208-DynamoRevit-Crash-Fix Code Review Fix that will create the WebView2 cache folder always in the AppData path. e.g. Revit C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Revit\2.16 Sandbox C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Core\2.16 * DYN-5208-DynamoRevit-Crash-Fix Code Review 2 Fix that will set the WebView2.UserDataFolder only when the popup is using an html page. * DYN-5208-DynamoRevit-Crash-Fix Code Review 2 Updating comment due that was specific to Revit.

RobertGlobant20 added 2 commits August 25, 2022 13:43

DYN-5208-DynamoRevit-Crash-Fix

f6f8697

Removing comment.

RobertGlobant20 requested a review from QilongTang August 25, 2022 18:58

mjkkirschner reviewed Aug 25, 2022

View reviewed changes

QilongTang reviewed Aug 26, 2022

View reviewed changes

RobertGlobant20 added 2 commits August 26, 2022 12:57

DYN-5208-DynamoRevit-Crash-Fix

45e845b

Updating comment

DYN-5208-DynamoRevit-Crash-Fix Code Review

e101e8a

Fix that will create the WebView2 cache folder always in the AppData path. e.g. Revit C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Revit\2.16 Sandbox C:\Users\roberto.tellez\AppData\Roaming\Dynamo\Dynamo Core\2.16

QilongTang reviewed Aug 29, 2022

View reviewed changes

DYN-5208-DynamoRevit-Crash-Fix Code Review 2

b37dcc1

Fix that will set the WebView2.UserDataFolder only when the popup is using an html page.

QilongTang approved these changes Aug 29, 2022

View reviewed changes

DYN-5208-DynamoRevit-Crash-Fix Code Review 2

2729c08

Updating comment due that was specific to Revit.

QilongTang merged commit 2632111 into DynamoDS:master Aug 29, 2022

RobertGlobant20 mentioned this pull request Aug 29, 2022

Cherry-Pick DYN-5208-DynamoRevit-Crash-Fix (#13245) #13264

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DYN-5208-DynamoRevit-Crash-Fix #13245

DYN-5208-DynamoRevit-Crash-Fix #13245

RobertGlobant20 commented Aug 25, 2022

RobertGlobant20 commented Aug 25, 2022

mjkkirschner Aug 25, 2022

RobertGlobant20 Aug 26, 2022

QilongTang Aug 26, 2022

QilongTang Aug 26, 2022

RobertGlobant20 Aug 26, 2022

RobertGlobant20 Aug 26, 2022

QilongTang Aug 29, 2022

RobertGlobant20 Aug 29, 2022

QilongTang Aug 29, 2022 •

edited

Loading

RobertGlobant20 Aug 29, 2022 •

edited

Loading

mjkkirschner Aug 29, 2022 •

edited

Loading

RobertGlobant20 Aug 29, 2022

mjkkirschner Aug 29, 2022

RobertGlobant20 Aug 29, 2022

QilongTang left a comment

QilongTang left a comment

QilongTang commented Aug 29, 2022

DYN-5208-DynamoRevit-Crash-Fix #13245

DYN-5208-DynamoRevit-Crash-Fix #13245

Conversation

RobertGlobant20 commented Aug 25, 2022

Purpose

Declarations

Release Notes

Reviewers

FYIs

RobertGlobant20 commented Aug 25, 2022

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

QilongTang Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

RobertGlobant20 Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

mjkkirschner Aug 29, 2022 • edited Loading

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

QilongTang left a comment

Choose a reason for hiding this comment

QilongTang left a comment

Choose a reason for hiding this comment

QilongTang commented Aug 29, 2022

QilongTang Aug 29, 2022 •

edited

Loading

RobertGlobant20 Aug 29, 2022 •

edited

Loading

mjkkirschner Aug 29, 2022 •

edited

Loading