swtich registration of js object to async version #8777
Conversation
…ss to methods here, and they return promises.
| this.browser = browser; | ||
| sidebarGrid.Children.Add(view); | ||
| browser.RegisterJsObject("controller", this); | ||
| browser.RegisterAsyncJsObject("controller", this); |
There was a problem hiding this comment.
what if anyother software uses a different version of cefsharp that requires RegisterJsObject? I believe it should not be the case. Is there a possible way to check if this object is registered and window["controller will return the right object?
There was a problem hiding this comment.
these functions return void on the c# side so we can't check anything there. We could check in javascript but that wouldn't tell us much more than we knew before as we'll just avoid an exception only to have no functionality, I'd rather just see the exception.
Calling RegisterJsObject or RegisterAsyncJSobject again will throw an exception in CEF... :(
they are supported in version 51 - 63 according to the api docs... which only go back to version 51.
There was a problem hiding this comment.
yeah I understand that part..is it possible to add exception in JS? like if window["controller] is null, then throw (ex)..some clear message ?
|
@mjkkirschner Can you attach this PR to the Jira task, we may want to inform Revit team about this issue as well |
|
@QilongTang attached. |
|
@mjkkirschner Let's just buy the mcafee license so we can put it on your machine. It is only $45. Also, if we have a branch build, we can get it out to him for testing. |
|
@Racel it's not clear to me that the mcafee subscription will get us the same corporate deployment that is causing the problem. They have around 20 different products. |
|
@mjkkirschner so is the best course of action to get the customer to test this? Can we get an installer? |
|
@Racel I think so yes, we can get an installer when we get this merged and then get a dev build or @smangarole enables daily builds for master. @ramramps sure, I can do that but recall that window["controller"] wasn't exactly null - it was just an empty object, I'll try to check for that or check if a method we need is missing and throw a clear message. |
|
@ramramps PTAL - added a small check to the html. |
| //Get hold of object registered from C# | ||
| let csharpController = window["controller"]; | ||
| //check if one of our methods does not exist | ||
| if (csharpController.closeNodeTooltip == null) { |
There was a problem hiding this comment.
can you also check csharpController == null as well? Windows returns an empty object, and trying to access an property on the empty object will throw exception...
review comments
|
@mjkkirschner @Racel @QilongTang @smangarole @ramramps An fyi from the black team team: we suspect that this issue also occurs with Refinery 0.3.12 and the Avast anti-virus, so using Dynamo 2.0.1 with Avast running may result in a blank library. |
Purpose
to address an issue we assume is similar to:
cefsharp/CefSharp#1611
#8757
we switch the
registerJsObjectmethod toregisterAsyncJSobject- apparently this is recommended for our CEFSharp version, and is faster than the other registration method. The work involved is minimal (appears so) because we only use methods on our cross language object anyway and we do not use the results of those method calls.When an object is registered this way its methods return promises, but since we do not use the results of calls to the registered object on the js side, we should be able to just call them like before. In initial testing behavior seems the same if not a small bit faster. (not profiled).
I can not verify that this PR actually fixes the user's issue - I have tried installing Heimdal v2.2.2 and cannot reproduce the failure.
I do not yet have a copy of mcafee anti virus to test with.
I do not know how to get heimdall client to run - it may be a corporate only/ enterprise wide component.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@QilongTang @ramramps
FYIs
@Racel @jnealb @smangarole