Make JS bindings work with subprocesses hosting multiple browser instances #1562
Conversation
|
|
||
| static JavascriptRootObject^ Update(int key, JavascriptRootObject^ value) |
|
@cefsharp-ms Are you able to add a test case to one of the examples? |
|
Thinking about this a little more, are you sure this is the best solution? My original idea was that once the render process had been initialized the root objects could be reused. Are you using standard popups and having problems? |
|
I fixed the naming and leftover method, I'm sorry, I missed those.
Yep, I'll put together something.
I wasn't clear about the exact use case here. I'm using custom popups and I wanted to have separate objects bound to the popups. And this caused a few problems, hence my change. However it just hit me that for standard popups the original PR would have changed the previous behavior, so I modified it a bit to have the bound objects from the 'main' browser as default values for all popups. |
Thanks 👍
Much appreciated.
That makes more sense. Thanks for clarifying. Will take it for a spin when I get a few minutes (hopefully next week sometime). |
| _javascriptRootObjects->default[browser->GetIdentifier()] = syncRoot; | ||
|
|
||
| //let's store the non-popup browser's root objects to have default values for the popups | ||
| if (!browser->IsPopup()) |
There was a problem hiding this comment.
Would it be cleaner to move the assignment for the popup objects into the branch of the if statement then do a simple HasKey check above? Remove the need for https://github.com/cefsharp/CefSharp/pull/1562/files#diff-a27ca20dcc4f76e2dde0c785dd2abb9cR50
Overall I think we can make the intent a little cleaner to follow. Open to other suggestions
There was a problem hiding this comment.
Ideally I'd still like to simplify this.
cefsharp-ms
force-pushed
the
multitenantsubprocess
branch
2 times, most recently
from
411149d to
e46fd4d
Compare
|
Hi, Also it seems like something has changed the timing for some events since the upgrade to 49 so I needed to apply another approach to the binding, now it basically pauses the first resource load until the bindings are ready. Otherwise in case of the popups the context creation was too early, and the bound objects for the popups were not sent over before that. I know it's a pretty big change, but please review it and let's talk about it. |
I've had a quick look (might be quite some time before I can go over this properly). In my opinion the only way to justify making such drastic changes would be to continue the refactor and remove https://github.com/cefsharp/CefSharp/blob/cefsharp/49/CefSharp.Core/CefSettings.h#L38 |
cefsharp-ms
force-pushed
the
multitenantsubprocess
branch
2 times, most recently
from
b0d9cde to
843f762
Compare
|
So I have long overdue update on this one. Sorry about that. |
cefsharp-ms
force-pushed
the
multitenantsubprocess
branch
from
843f762 to
ae18a9f
Compare
cefsharp-ms
force-pushed
the
multitenantsubprocess
branch
from
ae18a9f to
8502e67
Compare
|
You can now bind objects by name in different web pages, should also apply for |
When the popup functionality is used, more than one browser instance can be hosted in the same subprocess. As the bound JS objects are not stored on a per-browser basis they won't work properly in this scenario. I modified the storage and binding mechanism a bit to solve this.