Skip to content
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

Update Chrome/CEF/Xilium version #55

Open
danakj opened this issue Jun 19, 2017 · 3 comments
Open

Update Chrome/CEF/Xilium version #55

danakj opened this issue Jun 19, 2017 · 3 comments

Comments

@danakj
Copy link

danakj commented Jun 19, 2017

Hello,

I tried to update the CEF version so that we can get access to newer web features but I ran into some problems. I thought I should document them here for you and others. The result was that there is a bug in CEF where it will hang on shutdown when using multi-threaded + single-process together, which this project does. So quitting ACT leaves it hung.

I was trying Xilium a655793 which is based on CEF 3.2987.1601.gf035232 (Chrome 57.0.2987.133).

The bug is here, which is marked as wontfix: https://bitbucket.org/chromiumembedded/cef/issues/1993/cef-2785-shutdown-hangs-in-single-process

I left a comment explaining in the bug how to fix CEF. Until that is done, it won't be possible to update to a newer CEF without disabling multi-thread (ie, pushing all CEF stuff off to another thread in the control of this project, but requires asynchronous access to all of the CEF interfaces), or disabling single-process, which is not ideal given how many browsers will be running.

I tried a lot to find a workaround that OverlayPlugin could do to avoid the problem. The best I could come up with was to call CefContentRendererClient::Get()->WillDestroyCurrentMessageLoop(). However that would need to be done from a C++ module that would need to have access to all the Cef and Chromium headers, which was not ideal. And along the way to trying that I found something was defining a macro "max()" which interfered with chromium headers and I had to #undef that in them.

So the best would be fixing this upstream in CEF and then updating to the new version from OverlayPlugin. I am hoping my response on that bug gets some attention.

Other than this problem, the update process was very smooth. There was one CEF function that got an extra parameter, so I had to add that to the OverlayPlugin override, but that was the only issue.

@hibiyasleep
Copy link

What will happen when we turn on multi-process mode with cefhelper executable?

@danakj
Copy link
Author

danakj commented Oct 3, 2017

I don't think the javascript injection will work anymore, it relies on a pointer to the renderer "process".

var frame = this.Overlay.Renderer.Browser.GetFrame(frameId);

Here the frame would not be in the same process, unless Xilium and/or Cef provides a Frame abstraction in the browser process in multi-process mode. If not, it would require some IPC to ship the javascript text over and run it in the renderer process. I'm not sure what other uses of pointers would become invalid, this one I know from seeing it before. I think there's also a callback from the renderer to the overlay plugin that would need IPC help.

It would also increase the overhead of running multiple overlays a little, as you'd now have another process per overlay. I don't think this would be problematic though.

It'd be nice if the CEF bug would be fixed, I didn't go to the trouble of trying to submit a patch but if you're motivated that might be worth trying.

@nolanhu
Copy link

nolanhu commented Feb 15, 2019

What will happen when we turn on multi-process mode with cefhelper executable?

cannot debug at least

quisquous added a commit to quisquous/OverlayPlugin that referenced this issue Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants