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

Expose CefOverridePath to allow overriding DIR_EXE/DIR_MODULE paths (Issue 1936) #4

Closed
wants to merge 1 commit into from

Conversation

cztomczak
Copy link
Contributor

@cztomczak cztomczak commented Apr 26, 2017

This is to fix Issue 1936 "Override paths: DIR_EXE = DIR_MODULE = <libcef.so.path> on Linux":
https://bitbucket.org/chromiumembedded/cef/issues/1936/override-paths-dir_exe-dir_module-on-linux

This is an alternative pull request to PR 66 on bitbucket:
https://bitbucket.org/chromiumembedded/cef/pull-requests/66/override-paths-dir_exe-dir_module-on-linux/diff

Overriding DIR_EXE / DIR_MODULE works fine when done before calling CefInitialize. I have been using this patch for about 6 months with CEF v54..v58 and it works good, this fixed problems described in Issue 1936 in CEF Python project. Similar issues are also having users of java-cef project, they are forced to put binaries in jre/bin.

Code that fixes issue:

if LINUX:
    CefOverridePath(PK_DIR_EXE, cef_module_dir)
    CefOverridePath(PK_DIR_MODULE, cef_module_dir)
CefInitialize()

…36).

This is an alternative pull request to PR 66 on bitbucket:
https://bitbucket.org/chromiumembedded/cef/pull-requests/66/override-paths-dir_exe-dir_module-on-linux/diff

Overriding DIR_EXE / DIR_MODULE works fine when done before
calling CefInitialize. I have been using this patch for about
6 months with CEF v54..v58 and it works good, this fixed problems
described in Issue 1936 in CEF Python project. Similar issues are
also having users of java-cef project, they are forced to put
binaries in /usr/bin.

Code that fixes issue:
> CefOverridePath(PK_DIR_EXE, cef_module_dir)
> CefOverridePath(PK_DIR_MODULE, cef_module_dir)
> CefInitialize()

Bitbucket issue: https://bitbucket.org/chromiumembedded/cef/issues/1936/override-paths-dir_exe-dir_module-on-linux
@cztomczak
Copy link
Contributor Author

cztomczak commented Apr 26, 2017

On Mac issues with being forced to put binaries to /usr/bin can be solved by setting CefSettings.framework_dir_path.

@cztomczak
Copy link
Contributor Author

cztomczak commented Apr 26, 2017

In CEF Python library I do these CefOverridePath calls implicitilly, so users don't have to know anything about this fix. I believe a similar approach can be done in java-cef wrapper.

@cztomczak
Copy link
Contributor Author

I am thinking of a more severe warning in doc comments, that this function is for use only for advanced users that understand Chromium/CEF internals and that no support will be provided in case of issues. I can see your viewpoint of possible issues when people start using it. It doesn't matter for me really if this gets accepted or PR 66 or modified PR 66 with additional option like CefSettings.module_path. However I'm not convinced "module_path" is a good name, however we could make it more clear, explain in doc comments exactly what it does. It doesn't matter for me what solution is chosen as long as this gets fixed, because I would love to be able to use Spotify builds on Linux in CEF Python. If you would like a new PR with CefSettings.module_path option let me know and I will send.

@dmitry-azaraev
Copy link
Contributor

dmitry-azaraev commented Apr 27, 2017

I'm prefer CefOverridePath, mainly for API completeness (it matches to CefGetPath). And whenever i'm meniton PathService, i'm always imagine similar method. And this is completely not depends on other possible resolutions (as for me).

module_path on other side can work good, in that case CEF will be responsible to setup all path keys relative to module path.

And probably it is possible to make a step forward and use dladdr to determine own module path directly in libcef.so (on Linux)? (Section "bugs" is funny, and basically shows why explicit path manipulation mechanics are useful.)

@cztomczak
Copy link
Contributor Author

cztomczak commented Apr 27, 2017

In PR 66 dladdr is used: https://bitbucket.org/chromiumembedded/cef/pull-requests/66/override-paths-dir_exe-dir_module-on-linux/diff#Llibcef/browser/context.ccT306

So the CefSettings option could be a Bool, because we can already get know the correct path. But I'm not sure what name should be for the option in such case (CefSettings.override_dir_exe_module? override_module_path?)

@dmitry-azaraev
Copy link
Contributor

@cztomczak oh, i miss it, thanks.

I'm not sure about CefSettings and naming.

break;
#endif
case PK_USER_DATA:
pref_key = chrome::DIR_USER_DATA;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently allow customization of DIR_USER_DATA via CefSettings.user_data_path (in CefMainDelegate::PreSandboxStartup). If we add this method then we should delete CefSettings.user_data_path.

@chromiumembedded
Copy link
Collaborator

I wonder if we should implement CefOverridePath somewhat differently to keep users from shooting themselves. For example, CefOverridePath could cache values instead of calling PathService::Override immediately. Then, in a safe place during startup (some CefMainDelegate method) it could call PathService::Override one time for each value set. Any calls to CefOverridePath after that time would safely be ignored.

@gzunino
Copy link

gzunino commented May 3, 2017

This fix is fundamental for any integration of CEF with VM runtimes like java or python, to provide a seamless user experience that doesn't require copying files. Any ETA on merging it ?

@cztomczak
Copy link
Contributor Author

Marshall raised some valid concerns that need to be addressed before merging. I will update PR soon.

@bparadisayuda
Copy link

bparadisayuda commented Jul 24, 2017

Any idea when you might have time to make the modifications?

Any chance we could ge the concerns added here? Edit: I am guessing it just refers to the post from chromiumembedded commented on Apr 28

@cztomczak
Copy link
Contributor Author

@bparadisayuda There is no known schedule at this moment. I might take on this again when I prepare new CEF Python release.

Yes, the concerns written by Marshall here.

ClarkWang-2013 added a commit to ClarkWang-2013/cef that referenced this pull request Aug 29, 2017
@guusdk
Copy link

guusdk commented Sep 2, 2017

I'm interested in this too, as it will help me generate useful distributions for JCEF: http://magpcss.org/ceforum/viewtopic.php?f=17&t=15307

If there's anything that I can help, please let me know.

@cztomczak
Copy link
Contributor Author

@guusdk I see that you are integrating JCEF with AppVeyor. I have similar plans to integrate appveyor with cefpython : cztomczak/cefpython#202 . But I don't have much experience. If you would be willing to help me with that, send PR to add support, I could repay you with completing this PR :)

@guusdk
Copy link

guusdk commented Sep 11, 2017

@cztomczak : I'd love to give it a try, but you should be warned that even with JCEF, I mostly had no idea what I was doing. :) The build process for JCEF is well documented and scripted, which allowed me to reproduce those steps in AppVeyor, while remaining blissfully unaware of what is being done to build the project(s) itself. Don't get your hopes up!

@guusdk
Copy link

guusdk commented Sep 11, 2017

I'm giving it a go here: https://ci.appveyor.com/project/guusdk/cefpython

@cztomczak
Copy link
Contributor Author

@guusdk Discussing it here is off-topic. I pointed you earlier to a cefpython issue.

@guusdk
Copy link

guusdk commented Sep 19, 2017

@cztomczak Although I miserably failed, I'm hopeful that my brave attempt gave me just enough credits for this PR to be completed. ;) (I'm kind of blocked by this). Dare I ask for an ETA?

@cztomczak
Copy link
Contributor Author

The patch provided here by me already works and resolves the issue, you only have to build CEF from sources and apply the patch.

This is not the right place for discussions that are not strictly related to implementation details for this issue.

@speedy01
Copy link

speedy01 commented Sep 27, 2017

@chromiumembedded
On May 3rd @cztomczak stated the following

Marshall raised some valid concerns that need to be addressed before merging. I will update PR

soon.

Do those concerns still need to be addressed before this can be merged? It appears that @cztomczak doesn't have an ETA on the followup. Would it be possible to get this merged "as is" in order to address some immediate needs?

@magreenblatt
Copy link
Collaborator

@speedy01 Sorry, no. You can build CEF yourself if you need this change before it has passed review.

jiri-janousek added a commit to tiliado/cef that referenced this pull request Dec 20, 2017
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Dec 21, 2017
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
@speedy01
Copy link

@cztomczak @chromiumembedded
I'm not a developer, but participle in an open source project that would like to use jcef; Our devs are not familiar enough with the code to implement the changes asked for by @chromiumembedded . This seems to be a stopper for us. What needs to happen to get this across the finish line, and merged?

@cztomczak
Copy link
Contributor Author

@speedy01 See Marshall's comment in this issue and code comment - both from April 28th, 2017.

@speedy01
Copy link

@cztomczak @chromiumembedded
I did see that, but I also noticed on May 3 and Aug 19 it sounded like you were working on addressing Marshall's comments. I was hoping that was still the case. If not, I hope you will reconsider picking it back up. It appears that you are most familiar with what needs to be done to get this over the finish line. Let me know if there is anything I can do to help with this.

deepin-gerrit pushed a commit to martyr-deepin/qcef-chromium that referenced this pull request Mar 6, 2018
Project: cef dev-3112 48f0926c60c854604d1d5a92ac46d6307514ae37

Remove unused gn argument


Merge branch 'dev-3112' of github.com:LiuLang/cef into dev-3112


Support atomic operation on mips64el


Add more gn arguments


Support overriding paths

Merge patch from chromiumembedded/cef#4
Issue link is cztomczak/cefpython#231


Disable kerberos


Add more gn options


Support IME on linux


Added default gn args for linux platform


Windows: Fix crash during window creation (see https://crbug.com/761389)


Windows: Wait for WM_NCDESTROY before calling OnBeforeClose (issue #2248)


Update to Chromium version 60.0.3112.113


Windows: Fix crash during window creation (see https://crbug.com/761389)


Windows: Wait for WM_NCDESTROY before calling OnBeforeClose (issue #2248)


Support atomic operation on mips64el


Add more gn arguments


Update to Chromium version 60.0.3112.113


Support overriding paths

Merge patch from chromiumembedded/cef#4
Issue link is cztomczak/cefpython#231


Disable kerberos


Add more gn options


Support IME on linux


Added default gn args for linux platform


Fix Widevine DRM loading


Update to Chromium version 60.0.3112.90


views: Fix LabelButton size calculation


Linux: Work around gold linker bug for 32-bit code.


Fix encoding of linux_build.patch


Fix encoding of linux_build.patch


Linux: Fix build error related to GTK dependency version.


Update to Chromium version 60.0.3112.78


Linux/Mac: Fix handling of command-line arguments (issue #2208)


Windows: Build cef_sandbox.lib with different GN args for official binary distributions (issue #2220)


Add support for loading certificate revocation lists (issue #2213)


Fix incorrect OSR browser display during navigation (issue #2209)


Fix OSR PDF mouse events after keyboard input (issue #2078)


Update to Chromium version 60.0.3112.40


Fix OSR PDF mouse wheel scrolling (issue #2078)


Windows: cefclient: Fix ATL-related build errors (issue #2200)


Linux: Don't pass NULL CefBrowser to PrintHandler::GetPdfPaperSize (issue #2199)


Linux: Pass CefBrowser to CefPrintHandler callbacks (issue #2196)


Fix gtest path (issue #2188)


macOS: Fix error: unknown type name 'size_t'


Linux: Update to debian jessie sysroots


Linux: Fix error: use of undeclared identifier 'GetPreferredSize'


Update to Chromium version 60.0.3112.10
jiri-janousek added a commit to tiliado/cef that referenced this pull request Mar 11, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Mar 12, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Mar 13, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Mar 18, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request May 13, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Jun 1, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Jun 9, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Nov 2, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Nov 2, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Nov 2, 2018
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
@lorenzodallavecchia
Copy link

It has been 4 years of not being able to integrate the binary distribution of CEF in Linux.
Is there an ETA for this patch?
Thanks!

@jiri-janousek
Copy link

As you can see from the comments, nobody is working on this patch, so there is no ETA.

jiri-janousek added a commit to tiliado/cef that referenced this pull request Jan 3, 2019
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Feb 23, 2019
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
@magreenblatt
Copy link
Collaborator

Declining this PR in favor of https://bitbucket.org/chromiumembedded/cef/pull-requests/205

jiri-janousek added a commit to tiliado/cef that referenced this pull request May 6, 2019
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
jiri-janousek added a commit to tiliado/cef that referenced this pull request Sep 1, 2019
Patch by Czarek Tomczak

chromiumembedded/cef#4

Signed-off-by: Jiří Janoušek <[email protected]>
TwentyPast4 added a commit to TroniusGaming/cef that referenced this pull request Oct 29, 2024
TwentyPast4 added a commit to TroniusGaming/cef that referenced this pull request Dec 16, 2024
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

Successfully merging this pull request may close these issues.

10 participants