hawkerm / monaco-editor-uwp

A Windows Runtime Component wrapper around the web-based Monaco Editor.
MIT License
152 stars 36 forks source link

Moved WebView to out of process #48

Closed Sergio0694 closed 3 years ago

Sergio0694 commented 4 years ago

Moved the WebView to out of process for better performance.

Didn't actually test this yet as I don't have node.js installed 🙈

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

hawkerm commented 3 years ago

@Sergio0694 tried running the sample app but got:

image

System.Exception
  HResult=0x8001010E
  Message=The application called an interface that was marshalled for a different thread. (Exception from HRESULT: 0x8001010E (RPC_E_WRONG_THREAD))
  > Monaco.winmd!Monaco.CodeEditor.Options.get() Line 99    C#
    [Native to Managed Transition]  
    [Managed to Native Transition]  
    Monaco.winmd!Monaco.Helpers.ParentAccessor.GetJsonValue(string name) Line 144   C#
hawkerm commented 3 years ago

Rebased this on top of the 0.9 work thread I had with all the updates I've made recently. Also worked on making sure we could dispatch everything properly to marshal between the threads. Has been a bit of a pain, but almost have it working...

hawkerm commented 3 years ago

When we highlight/mark all the things, we should get this: image

We're currently with the out-of-proc WebView getting this: image

Which... is the same... it works! Yay!

I think the problem was that I was trying to test them when the language wasn't set to the right provider and for some reason there it wasn't working as expected. This may be related to the HasGlyphMargin property not being initialized at the beginning as well...

hawkerm commented 3 years ago

Surprisingly in Release Mode I still hit an exception with Decorations like I did in the 0.9 changes (see comment here)...

I would have hoped that was resolved by this move to async everything... That's going to require a deeper investigation then, which is concerning as it's a Release Mode only thing... 😭

hawkerm commented 3 years ago

Oh, this may be a different exception, the stack appears to be a ton of Newtonsoft Json bits:

MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::ExpressionValueProvider.GetValue(System::Object & target)   Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.CalculatePropertyValues($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonContainerContract & contract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonProperty & property, $7_Newtonsoft::Json::Serialization::JsonContract & & memberContract, System::Object & & memberValue)   Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.SerializeObject($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonObjectContract & contract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonContainerContract & collectionContract, $7_Newtonsoft::Json::Serialization::JsonProperty & containerProperty)  Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.SerializeValue($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonContract & valueContract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonContainerContract & containerContract, $7_Newtonsoft::Json::Serialization::JsonProperty & containerProperty) Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.SerializeObject($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonObjectContract & contract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonContainerContract & collectionContract, $7_Newtonsoft::Json::Serialization::JsonProperty & containerProperty)  Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.SerializeValue($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonContract & valueContract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonContainerContract & containerContract, $7_Newtonsoft::Json::Serialization::JsonProperty & containerProperty) Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.SerializeObject($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonObjectContract & contract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonContainerContract & collectionContract, $7_Newtonsoft::Json::Serialization::JsonProperty & containerProperty)  Unknown
    MonacoEditorTestApp.dll!$7_Newtonsoft::Json::Serialization::JsonSerializerInternalWriter.SerializeValue($7_Newtonsoft::Json::JsonWriter & writer, System::Object & value, $7_Newtonsoft::Json::Serialization::JsonContract & valueContract, $7_Newtonsoft::Json::Serialization::JsonProperty & member, $7_Newtonsoft::Json::Serialization::JsonContainerContract & containerContract, $7_Newtonsoft::Json::Serialization::JsonProperty & containerProperty) Unknown
... repeating until maxed out

Wish I knew where this was being called from exactly. Maybe this is an rd.xml thing I need to tweak? I should check if I updated the Newtonsoft version or not... (eventually should switch to System.Text.Json, but that's another can of worms).

hawkerm commented 3 years ago

Or maybe the same, was able to switch to x64 bits and get some breakpoints...

image

System.Exception
  HResult=0x8001010E
  Message=The application called an interface that was marshalled for a different thread. (Exception from HRESULT: 0x8001010E)
  Source=<Cannot evaluate the exception source>
  StackTrace:
<Cannot evaluate the exception stack trace>

Stepped a bit and got this:

Newtonsoft.Json.JsonSerializationException
  HResult=0x80131500
  Message=Error getting value from 'Color' on 'Windows.UI.Xaml.Media.SolidColorBrush'.
  Source=<Cannot evaluate the exception source>
  StackTrace:
<Cannot evaluate the exception stack trace>

Inner Exception 1:
Exception: The application called an interface that was marshalled for a different thread. (Exception from HRESULT: 0x8001010E)

Yeah, happening in the Decoration method: image

Really odd, as I don't think this would have changed from the last release?

hawkerm commented 3 years ago

This is all initiating from the UI Thread:

image

So I'm surprised there'd be an issue marshalling a UI object here...

I can try dispatching again, but that seems a bit silly. Otherwise, maybe it's best to have a converter for the object? Don't know if that'd run into the same problem...

hawkerm commented 3 years ago

Weird... it does look like the inner ContinueWith in the DeltaDecorationsHelperAsync method may be on a different thread:

image

hawkerm commented 3 years ago

Removed the ContinueWith to just use the AsyncInfo pattern to bridge as been doing everywhere else... still hitting an issue though... That seems to indicate that we're all on the same thread now, but still failing in the serialization call... though not being as explicit about what's going on.

hawkerm commented 3 years ago

Fixed #60 over here finally, just needed to clean-up a few things. Was unfortunately unresolved by threading everything for out-of-proc webview as I was hoping.

But in good news, it means out-of-proc WebView is basically working already. Just need to resolve the issue with the initial settings not getting set properly.

hawkerm commented 3 years ago

This should also help us with the move to WebView2 and it's addhostobjecttoscript interface later.

hawkerm commented 3 years ago

@Sergio0694 think I'm good to merge this in my beta branch. I then may try and build a test package to try out in a couple of other projects, merge that branch into main. Then just a couple of bugs to track down...?

Feel free to take a look at this PR and if you have any thoughts, know you can't test right now, but everything seems to be working well so far and think this should help with stuff later anyway for WebView2 too.

hez2010 commented 3 years ago

Fixed #60 over here finally

I proposed a better fix for this in #63. You don't have to add plenty of JsonIgnore and JsonConverter on properties, instead you can simply add them to the implementation class.