Review commits by sidreesshah who updated cefpython to the latest CEF #195

cztomczak · 2016-01-26T10:12:06Z

Syed's repository is available on bitbucket:
https://bitbucket.org/idrissha/cefpython

His work is tested only on Linux (Fedora 21). Some features may have been removed. Some functions were removed and that breaks BC - eg. ParentWindowWillClose() should stay and do just nothing.

Review his code: ask questions and add comments on bitbucket.

sidreesshah · 2016-01-26T18:15:23Z

Hi Czarek,
I removed the ParentWindowWillClose() function, because it was removed from CefBrowserHost. Although i did get crashes while closing the window in Debug mode. For that i removed some debug checks in CEF code. So it now works fine for me.

How will an empty function prevents the crash?

sidreesshah · 2016-01-26T18:19:10Z

Also i have updated my repo to support fedora 23 recently.

cztomczak · 2016-01-26T20:03:52Z

Hi Syed,

The idea is to keep the API backwards compatible. Python is a dynamic language and API changes can't be easily detected, as there is no compilation process. I just don't want user's code to break after updating - whenever possible. Regarding that specific function - ParentWindowWillClose() - never did anything on some of the OS'es, don't remember exactly but it probably only affected one OS.

Regarding the crash. What kind of checks? Does it only occur in Debug mode?

How are you running the Debug mode?
Edit: I was wrong, I recall using Debug builds of CEF, but only on Linux.

sidreesshah · 2016-01-26T22:41:08Z

Ok. Got your point about keeping that function. Thanks for that.

Here are the checks i have to disable in CEF:
diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc
index 83c7265..baa5056 100644
--- a/libcef/browser/browser_info.cc
+++ b/libcef/browser/browser_info.cc
@@ -79,9 +79,9 @@ void CefBrowserInfo::RenderIDManager::remove_render_id(RenderIdSet* id_set,
if (id_set->empty())
return;

bool erased = id_set->erase(

 std::make_pair(render_process_id, render_routing_id)) != 0;

DCHECK(erased);
/*bool erased = */id_set->erase(

 std::make_pair(render_process_id, render_routing_id))/\* != 0*/;

// DCHECK(erased);
}

bool CefBrowserInfo::RenderIDManager::is_render_id_match(
diff --git a/libcef/browser/browser_main.cc b/libcef/browser/browser_main.cc
index ded5cc9..d46525c 100644
--- a/libcef/browser/browser_main.cc
+++ b/libcef/browser/browser_main.cc
@@ -184,7 +184,7 @@ void CefBrowserMainParts::PostMainMessageLoopRun() {

#ifndef NDEBUG
// No CefBrowserContext instances should exist at this point.

DCHECK_EQ(0, CefBrowserContext::DebugObjCt);
//DCHECK_EQ(0, CefBrowserContext::DebugObjCt);
#endif
}

@@ -198,6 +198,6 @@ void CefBrowserMainParts::PostDestroyThreads() {

#ifndef NDEBUG
// No CefURLRequestContext instances should exist at this point.

DCHECK_EQ(0, CefURLRequestContext::DebugObjCt);
//DCHECK_EQ(0, CefURLRequestContext::DebugObjCt);
#endif
}

Yes, it occurs only in Debug mode. I just compile the CEF in Debug/Release mode and stick it into CefPython using the steps you have mentioned on your sources. Since you are using only CEF Release builds, you may not see them.

cztomczak · 2016-01-27T10:54:18Z

These reports seem to be related:

Looks like an issue in upstream CEF.

sidreesshah · 2016-01-27T17:08:41Z

To me it seems more like cefpython or my application not releasing the references. I could not reproduce the crashes with cefclient. How do we release the reference in Python?

cztomczak · 2016-01-27T17:45:42Z

What was the earliest branch of CEF you've been testing? In which branch did the crash start happening? Does the crash occur in one of official cefpython examples?

See the wxpython.py example > OnClose function for an example on how to release CEF references (use the del operator on a CEF object):

cefpython/cefpython/cef3/windows/binaries_32bit/wxpython.py

Line 209 in c05fcf2

def OnClose(self, event):

Browser is not the only object that needs to be released. This applies also to request objects and probably others. In the wxpython-response.py example we keep strong references to the ResourceHandler class which keeps CEF references to request objects and others - these need to be released when work is done. See:

cefpython/cefpython/cef3/linux/binaries_64bit/wxpython-response.py

Line 101 in c05fcf2

# A strong reference to ResourceHandler must be kept

sidreesshah · 2016-01-27T18:11:31Z

It was branch 2272 and i started seeing crashes very early on. I think i did not see any crash in pygtk_.py at that time.

Thanks for the reference links, i am not taking these steps for sure onClose. Will include them definitely.

There is one more thing that i did not pay attention to is that pygtk_.py started crashing after CEF release 2454, i guess. Luckily my own application was and is working fine, so i did not pay any attention to the pygtk_.py crash. Here is the crash what i am getting with pygtk_.py:
[pygtk_.py] GTK version: 2.24.29
Using Chromium 47.0.2526
[CEF Python] Initialize() called
[CEF Python] CefExecuteProcess(): exitCode = -1
[CEF Python] CefInitialize()
[CEF Python] App_OnBeforeCommandLineProcessing_BrowserProcess()
[CEF Python] Command line string for the browser process: /home/idrissha/work/cefpython/cefpython/cef3/linux/binaries_64bit/pygtk_.py --no-sandbox --disable-gpu --process-per-site --browser-subprocess-path=/home/idrissha/work/cefpython/cefpython/cef3/linux/binaries_64bit/subprocess --lang=en-US --log-file=/home/idrissha/work/cefpython/cefpython/cef3/linux/binaries_64bit/debug.log --log-severity=info --resources-dir-path=/home/idrissha/work/cefpython/cefpython/cef3/linux/binaries_64bit --locales-dir-path=/home/idrissha/work/cefpython/cefpython/cef3/linux/binaries_64bit/locales --remote-debugging-port=54067
[0127/101015:ERROR:browser_main_loop.cc(203)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on.
[CEF Python] BrowserProcessHandler_OnBeforeChildProcessLaunch()
[CEF Python] WindowUtils::IsWindowHandle() not implemented (always True)
[CEF Python] CreateBrowserSync() called
[CEF Python] navigateUrl: http://google.com
[CEF Python] CefBrowser::CreateBrowserSync()
[0127/101015:ERROR:browser_main_loop.cc(253)] Gdk: The program 'pygtk_.py' received an X Window System error.
This probably reflects a bug in the program.
The error was 'BadMatch (invalid parameter attributes)'.
(Details: serial 307 error_code 8 request_code 42 minor_code 0)
(Note to programmers: normally, X errors are reported asynchronously;
that is, you will receive the error a while after causing it.
To debug your program, run it with the --sync command line
option to change this behavior. You can then get a meaningful
backtrace from your debugger if you break on the gdk_x_error() function.)

cztomczak · 2016-01-27T18:34:44Z

What is the stack trace in GDB?

cztomczak · 2016-01-27T18:46:35Z

Maybe this is a non-fatal error that should be ignored? See xlib error handler in cefclient: https://bitbucket.org/chromiumembedded/cef/src/c3d80a5658b8636964981da23b455c3c85b3259e/tests/cefclient/cefclient_gtk.cc?at=master&fileviewer=file-view-default#cefclient_gtk.cc-104

sidreesshah · 2016-01-27T19:52:52Z

I think you are right. I am using xlib in my application. And just removing the calls to that started crashing my application(although with different stack). Here is the backtrace for pygtk_.py anyhow:

#0  0x00007fffe37b981b in g_logv () at /lib64/libglib-2.0.so.0
#1  0x00007fffe37b998f in g_log () at /lib64/libglib-2.0.so.0
#2  0x00007fffd58e0a50 in gdk_x_error () at /lib64/libgdk-x11-2.0.so.0
#3  0x00007fffe23d339d in _XError () at /lib64/libX11.so.6
#4  0x00007fffe23d0227 in handle_error () at /lib64/libX11.so.6
#5  0x00007fffe23d02e5 in handle_response () at /lib64/libX11.so.6
#6  0x00007fffe23d0e3d in _XReadEvents () at /lib64/libX11.so.6
#7  0x00007fffe23cf2b0 in XWindowEvent () at /lib64/libX11.so.6
#8  0x00007fffe9ae9340 in ui::X11EventSource::BlockUntilWindowMapped(unsigned long) (this=0x555555d55bd0, window=54525953) at ../../ui/events/platform/x11/x11_event_source.cc:115
#9  0x00007fffe5026025 in CefWindowX11::Show() (this=0x555555ef73a0) at ../../cef/libcef/browser/window_x11.cc:195
#10 0x00007fffe5020204 in CefBrowserHostImpl::PlatformCreateWindow() (this=0x555555e951b0) at ../../cef/libcef/browser/browser_host_impl_linux.cc:192
#11 0x00007fffe4dd3f44 in CefBrowserHostImpl::CreateInternal(CefWindowInfo const&, CefStructBase<CefBrowserSettingsTraits> const&, CefRefPtr<CefClient>, content::WebContents*, scoped_refptr<CefBrowserInfo>, unsigned long, CefRefPtr<CefRequestContext>) (window_info=..., settings=..., client=..., web_contents=0x555555e88520, browser_info=..., opener=0, request_context=...)
    at ../../cef/libcef/browser/browser_host_impl.cc:542
#12 0x00007fffe4dd36a3 in CefBrowserHostImpl::Create(CefWindowInfo const&, CefRefPtr<CefClient>, CefStringBase<CefStringTraitsUTF16> const&, CefStructBase<CefBrowserSettingsTraits> const&, unsigned long, bool, CefRefPtr<CefRequestContext>) (windowInfo=..., client=..., url=..., settings=..., opener=0, is_popup=false, request_context=...)
    at ../../cef/libcef/browser/browser_host_impl.cc:488
#13 0x00007fffe4dd3337 in CefBrowserHost::CreateBrowserSync(CefWindowInfo const&, CefRefPtr<CefClient>, CefStringBase<CefStringTraitsUTF16> const&, CefStructBase<CefBrowserSettingsTraits> const&, CefRefPtr<CefRequestContext>) (windowInfo=..., client=..., url=..., settings=..., request_context=...) at ../../cef/libcef/browser/browser_host_impl.cc:458
#14 0x00007fffe4d0f5c0 in cef_browser_host_create_browser_sync(cef_window_info_t const*, _cef_client_t*, cef_string_t const*, _cef_browser_settings_t const*, _cef_request_context_t*) (windowInfo=0x7fffffffd028, client=0x555555e52270, url=0x555555e51d20, settings=0x7fffffffd068, request_context=0x0) at ../../cef/libcef_dll/cpptoc/browser_host_cpptoc.cc:89
#15 0x00007fffd6374c4c in CefBrowserHost::CreateBrowserSync(CefWindowInfo const&, CefRefPtr<CefClient>, CefStringBase<CefStringTraitsUTF16> const&, CefStructBase<CefBrowserSettingsTraits> const&, CefRefPtr<CefRequestContext>) (windowInfo=..., client=..., url=..., settings=..., request_context=...) at ../../cef/libcef_dll/ctocpp/browser_host_ctocpp.cc:55
#16 0x00007fffd6357b08 in __pyx_pf_14cefpython_py27_22CreateBrowserSync(PyObject*, PyObject*, PyObject*, PyObject*, PyObject*) (__pyx_v_windowInfo=0x7fffd363ce30, __pyx_v_browserSettings=<optimized out>, __pyx_v_navigateUrl=0x7ffff7e89ab0, __pyx_v_requestContext=<optimized out>, __pyx_self=<optimized out>) at cefpython.cpp:114395
#17 0x00007fffd635886b in __pyx_pw_14cefpython_py27_23CreateBrowserSync(PyObject*, PyObject*, PyObject*) (__pyx_self=<optimized out>, __pyx_args=0x7fffd31bb350, __pyx_kwds=<optimized out>)
    at cefpython.cpp:114196
#18 0x00007ffff7af5572 in PyEval_EvalFrameEx () at /lib64/libpython2.7.so.1.0
#19 0x00007ffff7af66b4 in PyEval_EvalCodeEx () at /lib64/libpython2.7.so.1.0
#20 0x00007ffff7a825ac in function_call () at /lib64/libpython2.7.so.1.0
#21 0x00007ffff7a5db03 in PyObject_Call () at /lib64/libpython2.7.so.1.0
#22 0x00007ffff7a6c95c in instancemethod_call () at /lib64/libpython2.7.so.1.0
#23 0x00007ffff7a5db03 in PyObject_Call () at /lib64/libpython2.7.so.1.0
#24 0x00007ffff7aefac7 in PyEval_CallObjectWithKeywords () at /lib64/libpython2.7.so.1.0
#25 0x00007ffff7a6d660 in PyInstance_New () at /lib64/libpython2.7.so.1.0
#26 0x00007ffff7a5db03 in PyObject_Call () at /lib64/libpython2.7.so.1.0
#27 0x00007ffff7af3a68 in PyEval_EvalFrameEx () at /lib64/libpython2.7.so.1.0
#28 0x00007ffff7af66b4 in PyEval_EvalCodeEx () at /lib64/libpython2.7.so.1.0
#29 0x00007ffff7af67d9 in PyEval_EvalCode () at /lib64/libpython2.7.so.1.0
#30 0x00007ffff7b0fbdf in run_mod () at /lib64/libpython2.7.so.1.0
#31 0x00007ffff7b10db2 in PyRun_FileExFlags () at /lib64/libpython2.7.so.1.0
#32 0x00007ffff7b11fc7 in PyRun_SimpleFileExFlags () at /lib64/libpython2.7.so.1.0
#33 0x00007ffff7b241e1 in Py_Main () at /lib64/libpython2.7.so.1.0
#34 0x00007ffff6d4c580 in __libc_start_main () at /lib64/libc.so.6
#35 0x0000555555554839 in _start ()

cztomczak · 2016-01-27T21:35:11Z

You referenced many issues with your #xx numbering (a bit of spam appeared in those issues) - I've edited your comment and embraced code with three backticks.

cztomczak · 2016-01-27T21:37:47Z

Not the greatest idea by GitHub to treat every hash followed by a number as an issue..

sidreesshah · 2016-01-27T21:48:37Z

Ohhh...i idid not realize it :-) Thanks for fixing the comments. So you are merging my changes with some of your updates?

cztomczak · 2016-01-28T09:04:24Z

Like I said earlier, I will add comments and ask some questions on bitbucket. I haven't yet reviewed it.

sidreesshah · 2016-01-28T18:11:02Z

Sorry i did not understand your previous comment about "embraced code with three backticks". My english :-). Its clear now.

allestuetsmerweh · 2016-02-10T16:44:38Z

I tried to test your code on ubuntu, 64bit.

I had to remove a semicolon in cefpython/cython_includes/cef_urlrequest_cef3.pxd:

diff --git a/cefpython/cython_includes/cef_urlrequest_cef3.pxd b/cefpython/cython_includes/cef_urlrequest_cef3.pxd
index 3af4d90..7c6a804 100644
--- a/cefpython/cython_includes/cef_urlrequest_cef3.pxd
+++ b/cefpython/cython_includes/cef_urlrequest_cef3.pxd
@@ -14,7 +14,7 @@ cdef extern from "include/cef_urlrequest.h":
             "CefURLRequest::Create"( \
                     CefRefPtr[CefRequest] request, \
                     CefRefPtr[CefURLRequestClient] client, \
-                    CefRefPtr[CefRequestContext] request_context);
+                    CefRefPtr[CefRequestContext] request_context)

     cdef cppclass CefURLRequest(CefBase):
         CefRefPtr[CefRequest] GetRequest()

The compilation succeeded (the cefpython_py27.so was created), but when running the wxpython example, the following error occurred:

Traceback (most recent call last):
  File "pygtk_.py", line 18, in <module>
    import cefpython_py27 as cefpython
ImportError: /home/rentouch/Developer/cefpython47/cefpython/cef3/linux/binaries_64bit/cefpython_py27.so: undefined symbol: _ZN3cef7logging10LogMessageC1EPKcii

I used the prebuilt CEF binaries from branch 2526.

Any ideas?

cztomczak · 2016-02-11T14:57:24Z

I started reviewing the code. Added a bunch of comments to the fd7f932 commit.

@allestuetsmerweh You won't be happy about this - Idris removed OSR support ;) See window_info_cef3.pyx.

cztomczak · 2016-02-11T17:09:32Z

@sidreesshah
I've reviewed your commits and added about 30 comments and questions. There are several issues with your code. Apart from updating to latest CEF, I see you've also added many new features and it would be nice to have them incorporated in cefpython (getChromiumVersion, custom context menu, print handler [gtk only?], download handler, new callbacks in display handler [favicon, fullscreen, audio, video]).

What is missing and I think is important is example usage for many of the new functions.

I see you've updated to Cython 0.23.4 - didn't you have any issues with that update? Last time I've checked there were incompatibilities - see Issue #110.

cztomczak · 2016-02-11T17:17:10Z

@sidreesshah I haven't found xlib error handler in pygtk_.py and from what you've said earlier it's important to set it to avoid crashes. Are you using c++ code or do you make the calls using python, in such case can you share it? Were there any other issues of importance?

cztomczak · 2016-02-16T11:43:04Z

There is an ongoing big refactoring, so hold on with any pull requests. See Issue #208 - "Big code cleanup and refactoring with the upcoming Chrome 47 release".

sidreesshah · 2016-02-16T17:41:35Z

Sorry man, just saw your comments now. I was too busy for last week. I will look at your comments today and not push anything new till you are done with the upgrade. I will be as active as i can on #208 to help the upgrade.

As for as your previous queries, i did not experience any major issue with upgrade of Cython.

My application uses python but it also uses C calls to X functions via Cython. Its not my own product, but i will see if i can reduce it to bare minimum open source and publish it.

cztomczak · 2016-02-16T18:42:51Z

Regarding X11 error handler - I will expose cefpython.x11.InstallErrorHandlers() using the cefclient code that sets the error handlers. That's best I think.

Issues with Cython may not be obvious - those are subtle bugs that may reveal in some of the functions.

cztomczak · 2016-02-16T19:14:07Z

Btw. thanks for sharing code. It gave me an overall look what needs to be done and how.

sidreesshah · 2016-02-18T05:52:40Z

you are wellcome.:-) So far we have not hit any issue with Cython but there
may be cases we have not bumped into yet.

On Tue, Feb 16, 2016 at 11:14 AM, Czarek Tomczak notifications@github.com
wrote:

Btw. thanks for sharing code. It gave me an overall look what needs to be
done and how.

—
Reply to this email directly or view it on GitHub
#195 (comment)
.

cztomczak · 2016-02-18T11:56:44Z

In Issue #216 I've outlined modifications and new features added in your fork. Also listed several changes in upstream cefclient that need action.

cztomczak added this to the Chrome 47 milestone Jan 26, 2016

cztomczak added the work in progress label Feb 11, 2016

cztomczak removed the work in progress label Feb 11, 2016

cztomczak closed this as completed Feb 16, 2016

cztomczak mentioned this issue Jun 25, 2016

Error when importing the CEF Python 47 module: undefined symbol: cef::logging::LogMessage #230

Closed

cztomczak changed the title ~~Chrome 47: review commits by sidreesshah who updated cefpython to the latest CEF~~ Review commits by sidreesshah who updated cefpython to the latest CEF Feb 24, 2017

cztomczak modified the milestones: v47, Chrome 50+ Feb 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Review commits by sidreesshah who updated cefpython to the latest CEF #195

Review commits by sidreesshah who updated cefpython to the latest CEF #195

cztomczak commented Jan 26, 2016

sidreesshah commented Jan 26, 2016

sidreesshah commented Jan 26, 2016

cztomczak commented Jan 26, 2016

sidreesshah commented Jan 26, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 27, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 27, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 28, 2016

sidreesshah commented Jan 28, 2016

allestuetsmerweh commented Feb 10, 2016

cztomczak commented Feb 11, 2016

cztomczak commented Feb 11, 2016

cztomczak commented Feb 11, 2016

cztomczak commented Feb 16, 2016

sidreesshah commented Feb 16, 2016

cztomczak commented Feb 16, 2016

cztomczak commented Feb 16, 2016

sidreesshah commented Feb 18, 2016

cztomczak commented Feb 18, 2016

Review commits by sidreesshah who updated cefpython to the latest CEF #195

Review commits by sidreesshah who updated cefpython to the latest CEF #195

Comments

cztomczak commented Jan 26, 2016

sidreesshah commented Jan 26, 2016

sidreesshah commented Jan 26, 2016

cztomczak commented Jan 26, 2016

sidreesshah commented Jan 26, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 27, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 27, 2016

cztomczak commented Jan 27, 2016

sidreesshah commented Jan 27, 2016

cztomczak commented Jan 28, 2016

sidreesshah commented Jan 28, 2016

allestuetsmerweh commented Feb 10, 2016

cztomczak commented Feb 11, 2016

cztomczak commented Feb 11, 2016

cztomczak commented Feb 11, 2016

cztomczak commented Feb 16, 2016

sidreesshah commented Feb 16, 2016

cztomczak commented Feb 16, 2016

cztomczak commented Feb 16, 2016

sidreesshah commented Feb 18, 2016

cztomczak commented Feb 18, 2016