RainbowMage / OverlayPlugin

Other
133 stars 182 forks source link

Update Chrome/CEF/Xilium version #55

Open danakj opened 7 years ago

danakj commented 7 years ago

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 commented 6 years ago

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

danakj commented 6 years ago

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

https://github.com/RainbowMage/OverlayPlugin/blob/96552e5603d520be6116d52b6d685355dd5e7a24/OverlayPlugin.Core/OverlayBase.cs#L370

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 commented 5 years ago

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

cannot debug at least