Skip to content

Commit d7d69fa

Browse files
CzarekCzarek
Czarek
authored and
Czarek
committed
CEF 3: freeing PyBrowser and PyFrame objects in the OnBrowserDestroyed
and OnContextReleased events. This was causing errors when sending process messages to do the javascript bindings to non-existent browsers. Not freeing these objects was also causing minor memory leaks.
1 parent 55ce020 commit d7d69fa

7 files changed

Lines changed: 167 additions & 44 deletions

File tree

‎cefpython/browser.pyx‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ IF CEF_VERSION == 1:
1818

1919
cdef dict g_pyBrowsers = {}
2020

21+
cdef PyBrowser GetPyBrowserById(int browserId):
22+
if g_pyBrowsers.has_key(browserId):
23+
return g_pyBrowsers[browserId]
24+
return None
25+
2126
cdef PyBrowser GetPyBrowser(CefRefPtr[CefBrowser] cefBrowser):
2227
global g_pyBrowsers
2328
if <void*>cefBrowser == NULL or not cefBrowser.get():
@@ -74,6 +79,30 @@ cdef PyBrowser GetPyBrowser(CefRefPtr[CefBrowser] cefBrowser):
7479
pyBrowser.SetJavascriptBindings(javascriptBindings)
7580
return pyBrowser
7681

82+
cdef public void ProcessMessage_OnBrowserDestroyed(
83+
int browserId
84+
) except * with gil:
85+
# CefRenderProcessHandler::OnBrowserDestroyed() sends a process
86+
# message to the browser process. The browser is probably already
87+
# destroyed in the renderer process when this message arrives to
88+
# the browser process.
89+
# --
90+
# Due to multi-process architecture in CEF 3, this function won't
91+
# get called for the main browser, to send a message from the
92+
# renderer a parent browser is used, as you can't send a process
93+
# message to a browser that is being destroyed.
94+
try:
95+
RemovePyBrowser(browserId)
96+
except:
97+
(exc_type, exc_value, exc_trace) = sys.exc_info()
98+
sys.excepthook(exc_type, exc_value, exc_trace)
99+
100+
cdef void RemovePyBrowser(int browserId) except *:
101+
# Called from ProcessMessage_OnBrowserDestroyed().
102+
# TODO: call this function also in CEF 1, currently called only in CEF 3.
103+
Debug("del g_pyBrowsers[%s]" % browserId)
104+
del g_pyBrowsers[browserId]
105+
77106
cpdef PyBrowser GetBrowserByWindowHandle(WindowHandle windowHandle):
78107
cdef PyBrowser pyBrowser
79108
for browserId in g_pyBrowsers:

‎cefpython/cef3/client_handler/client_handler.cpp‎

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,46 +17,61 @@ bool ClientHandler::OnProcessMessageReceived(CefRefPtr<CefBrowser> browser,
1717
logMessage.append(messageName.c_str());
1818
DebugLog(logMessage.c_str());
1919
if (messageName == "OnContextCreated") {
20-
CefRefPtr<CefListValue> args = message->GetArgumentList();
21-
if (args->GetSize() == 1 && args->GetType(0) == VTYPE_INT) {
22-
int64 frameId = args->GetInt(0);
20+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
21+
if (arguments->GetSize() == 1 && arguments->GetType(0) == VTYPE_INT) {
22+
int64 frameId = arguments->GetInt(0);
2323
CefRefPtr<CefFrame> frame = browser->GetFrame(frameId);
2424
V8ContextHandler_OnContextCreated(browser, frame);
2525
return true;
2626
} else {
27-
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments," \
28-
" messageName=OnContextCreated");
27+
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments" \
28+
", messageName = OnContextCreated");
2929
return false;
3030
}
3131
} else if (messageName == "OnContextReleased") {
32-
CefRefPtr<CefListValue> args = message->GetArgumentList();
33-
if (args->GetSize() == 1 && args->GetType(0) == VTYPE_INT) {
34-
int64 frameId = args->GetInt(0);
35-
CefRefPtr<CefFrame> frame = browser->GetFrame(frameId);
36-
V8ContextHandler_OnContextReleased(browser, frame);
32+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
33+
if (arguments->GetSize() == 2 \
34+
&& arguments->GetType(0) == VTYPE_INT \
35+
&& arguments->GetType(1) == VTYPE_INT) {
36+
int browserId = arguments->GetInt(0);
37+
int64 frameId = arguments->GetInt(1);
38+
V8ContextHandler_OnContextReleased(browserId, frameId);
3739
return true;
3840
} else {
39-
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments," \
40-
" messageName=OnContextReleased");
41+
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments" \
42+
", messageName = OnContextReleased");
4143
return false;
4244
}
4345
} else if (messageName == "V8FunctionHandler::Execute") {
44-
CefRefPtr<CefListValue> args = message->GetArgumentList();
45-
if (args->GetSize() == 3
46-
&& args->GetType(0) == VTYPE_INT // frameId
47-
&& args->GetType(1) == VTYPE_STRING // funcName
48-
&& args->GetType(2) == VTYPE_LIST) { // funcArguments
49-
int64 frameId = args->GetInt(0);
50-
CefString funcName = args->GetString(1);
51-
CefRefPtr<CefListValue> funcArguments = args->GetList(2);
46+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
47+
if (arguments->GetSize() == 3
48+
&& arguments->GetType(0) == VTYPE_INT // frameId
49+
&& arguments->GetType(1) == VTYPE_STRING // funcName
50+
&& arguments->GetType(2) == VTYPE_LIST) { // funcArguments
51+
int64 frameId = arguments->GetInt(0);
52+
CefString funcName = arguments->GetString(1);
53+
CefRefPtr<CefListValue> funcArguments = arguments->GetList(2);
5254
CefRefPtr<CefFrame> frame = browser->GetFrame(frameId);
5355
V8FunctionHandler_Execute(browser, frame, funcName, funcArguments);
5456
return true;
5557
} else {
56-
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments," \
57-
" messageName=V8FunctionHandler::Execute");
58+
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments" \
59+
", messageName = V8FunctionHandler::Execute");
60+
return false;
61+
}
62+
} else if (messageName == "OnBrowserDestroyed") {
63+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
64+
if (arguments->GetSize() == 1 && arguments->GetType(0) == VTYPE_INT) {
65+
int browserId = arguments->GetInt(0);
66+
// NOT browser->GetIdentifier()!
67+
ProcessMessage_OnBrowserDestroyed(browserId);
68+
return true;
69+
} else {
70+
DebugLog("Browser: OnProcessMessageReceived(): invalid arguments" \
71+
", messageName = OnBrowserDestroyed");
5872
return false;
5973
}
74+
6075
}
6176
return false;
6277
}

‎cefpython/cef3/linux/setup/cefpython.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
#endif
1313
#endif
1414

15+
__PYX_EXTERN_C DL_IMPORT(void) ProcessMessage_OnBrowserDestroyed(int);
1516
__PYX_EXTERN_C DL_IMPORT(void) V8ContextHandler_OnContextCreated(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame>);
16-
__PYX_EXTERN_C DL_IMPORT(void) V8ContextHandler_OnContextReleased(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame>);
17+
__PYX_EXTERN_C DL_IMPORT(void) V8ContextHandler_OnContextReleased(int, int64);
1718
__PYX_EXTERN_C DL_IMPORT(void) V8FunctionHandler_Execute(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame>, CefString &, CefRefPtr<CefListValue>);
1819

1920
#endif /* !__PYX_HAVE_API__cefpython_py27 */

‎cefpython/cef3/subprocess/cefpython_app.cpp‎

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
#include "javascript_callback.h"
1212
#include "python_callback.h"
1313

14+
CefRefPtr<CefBrowser> g_mainBrowser;
15+
1416
// -----------------------------------------------------------------------------
1517
// CefApp
1618
// -----------------------------------------------------------------------------
@@ -83,10 +85,45 @@ void CefPythonApp::OnWebKitInitialized() {
8385
}
8486

8587
void CefPythonApp::OnBrowserCreated(CefRefPtr<CefBrowser> browser) {
88+
// TODO: keep a list of all browsers, for sending the message
89+
// about browser being destroyed use the first browser
90+
// from that list. See this topic:
91+
// http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=10893
92+
// --
93+
// The g_mainBrowser is not guaranteed to be the "main" browser,
94+
// it is just the first browser being created, that in most cases
95+
// should be the main browser. We will need it later to send
96+
// a process message about other browsers being destroyed.
97+
if (!g_mainBrowser.get()) {
98+
g_mainBrowser = browser;
99+
}
86100
}
87101

88102
void CefPythonApp::OnBrowserDestroyed(CefRefPtr<CefBrowser> browser) {
103+
DebugLog("Renderer: OnBrowserDestroyed()");
89104
RemoveJavascriptBindings(browser);
105+
// TODO: keep a list of all browsers, for sending the message
106+
// about browser being destroyed use the first browser
107+
// from that list. See this topic:
108+
// http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=10893
109+
if (g_mainBrowser.get() && g_mainBrowser->IsSame(browser)) {
110+
g_mainBrowser = NULL;
111+
} else {
112+
// OnBrowserDestroyed process message is not sent for
113+
// the main browser.
114+
if (g_mainBrowser.get()) {
115+
CefRefPtr<CefProcessMessage> message = CefProcessMessage::Create(
116+
"OnBrowserDestroyed");
117+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
118+
arguments->SetInt(0, browser->GetIdentifier());
119+
g_mainBrowser->SendProcessMessage(PID_BROWSER, message);
120+
} else {
121+
DebugLog("Renderer: OnBrowserDestroyed(): ERROR: main browser not " \
122+
"found, cannot send process message to the browser process " \
123+
"about browser being destroyed. This will cause memory leaks" \
124+
" and possibly other errors that could crash application.");
125+
}
126+
}
90127
}
91128

92129
bool CefPythonApp::OnBeforeNavigation(CefRefPtr<CefBrowser> browser,
@@ -103,7 +140,7 @@ void CefPythonApp::OnContextCreated(CefRefPtr<CefBrowser> browser,
103140
DebugLog("Renderer: OnContextCreated()");
104141
CefRefPtr<CefProcessMessage> message = CefProcessMessage::Create(
105142
"OnContextCreated");
106-
CefRefPtr<CefListValue> args = message->GetArgumentList();
143+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
107144
/*
108145
Sending int64 type using process messaging would require
109146
converting it to a string or a binary, or you could send
@@ -125,7 +162,7 @@ void CefPythonApp::OnContextCreated(CefRefPtr<CefBrowser> browser,
125162
// from string to int64. But it is rather unlikely
126163
// that number of frames will exceed int range, so
127164
// casting it to int for now.
128-
args->SetInt(0, (int)(frame->GetIdentifier()));
165+
arguments->SetInt(0, (int)(frame->GetIdentifier()));
129166
browser->SendProcessMessage(PID_BROWSER, message);
130167
CefRefPtr<CefDictionaryValue> jsBindings = GetJavascriptBindings(browser);
131168
if (jsBindings.get()) {
@@ -149,16 +186,27 @@ void CefPythonApp::OnContextReleased(CefRefPtr<CefBrowser> browser,
149186
CefRefPtr<CefFrame> frame,
150187
CefRefPtr<CefV8Context> context) {
151188
DebugLog("Renderer: OnContextReleased()");
152-
CefRefPtr<CefProcessMessage> message = CefProcessMessage::Create(
153-
"OnContextReleased");
154-
CefRefPtr<CefListValue> args = message->GetArgumentList();
155-
// TODO: losing int64 precision, the solution is to convert
156-
// it to string and then in the Browser process back
157-
// from string to int64. But it is rather unlikely
158-
// that number of frames will exceed int range, so
159-
// casting it to int for now.
160-
args->SetInt(0, (int)(frame->GetIdentifier()));
161-
browser->SendProcessMessage(PID_BROWSER, message);
189+
if (g_mainBrowser.get()) {
190+
CefRefPtr<CefProcessMessage> message = CefProcessMessage::Create(
191+
"OnContextReleased");
192+
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
193+
arguments->SetInt(0, browser->GetIdentifier());
194+
// TODO: losing int64 precision, the solution is to convert
195+
// it to string and then in the Browser process back
196+
// from string to int64. But it is rather unlikely
197+
// that number of frames will exceed int range, so
198+
// casting it to int for now.
199+
arguments->SetInt(1, (int)(frame->GetIdentifier()));
200+
// Should we send the message using current "browser"
201+
// when this is not the main frame? It could fail, so
202+
// it is more reliable to always use the main browser.
203+
g_mainBrowser->SendProcessMessage(PID_BROWSER, message);
204+
} else {
205+
DebugLog("Renderer: OnContextReleased(): ERROR: main browser not " \
206+
"found, cannot send process message to the browser process " \
207+
"about frame being destroyed. This will cause memory leaks" \
208+
" and possibly other errors that could crash application.");
209+
}
162210
// Clear javascript callbacks.
163211
RemoveJavascriptCallbacksForFrame(frame);
164212
RemovePythonCallbacksForFrame(frame);

‎cefpython/frame.pyx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44

55
cdef dict g_pyFrames = {}
66

7+
cdef PyFrame GetPyFrameById(object frameId):
8+
if g_pyFrames.has_key(frameId):
9+
return g_pyFrames[frameId]
10+
return None
11+
712
cdef PyFrame GetPyFrame(CefRefPtr[CefFrame] cefFrame):
813
global g_pyFrames
914

@@ -20,7 +25,8 @@ cdef PyFrame GetPyFrame(CefRefPtr[CefFrame] cefFrame):
2025

2126
for id, pyFrame in g_pyFrames.items():
2227
if not pyFrame.cefFrame.get():
23-
Debug("GetPyFrame(): removing an empty CefFrame reference, frameId=%s" % id)
28+
Debug("GetPyFrame(): removing an empty CefFrame reference, " \
29+
"frameId = %s" % id)
2430
del g_pyFrames[id]
2531

2632
# Debug("GetPyFrame(): creating new PyFrame, frameId=%s" % frameId)
@@ -29,6 +35,12 @@ cdef PyFrame GetPyFrame(CefRefPtr[CefFrame] cefFrame):
2935
g_pyFrames[frameId] = pyFrame
3036
return pyFrame
3137

38+
cdef void RemovePyFrame(int frameId) except *:
39+
# Called from V8ContextHandler_OnContextReleased().
40+
# TODO: call this function also in CEF 1, currently called only in CEF 3.
41+
Debug("del g_pyFrames[%s]" % frameId)
42+
del g_pyFrames[frameId]
43+
3244
cdef class PyFrame:
3345
cdef CefRefPtr[CefFrame] cefFrame
3446

‎cefpython/javascript_bindings.pyx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ cdef class JavascriptBindings:
131131
cdef dict methods
132132
global g_pyBrowsers
133133
for browserId, pyBrowser in g_pyBrowsers.iteritems():
134+
if pyBrowser.GetJavascriptBindings() != self:
135+
continue
134136
# Send to the Renderer process: functions, properties,
135137
# objects and its methods, bindToFrames.
136138
functions = {}

‎cefpython/v8context_handler_cef3.pyx‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,36 @@ cdef public void V8ContextHandler_OnContextCreated(
2929
sys.excepthook(exc_type, exc_value, exc_trace)
3030

3131
cdef public void V8ContextHandler_OnContextReleased(
32-
CefRefPtr[CefBrowser] cefBrowser,
33-
CefRefPtr[CefFrame] cefFrame
32+
int browserId,
33+
int64 frameId
3434
) except * with gil:
3535
cdef PyBrowser pyBrowser
3636
cdef PyFrame pyFrame
3737
cdef object clientCallback
3838
try:
39+
# Due to multi-process architecture in CEF 3, this function won't
40+
# get called for the main frame in main browser. To send a message
41+
# from the renderer process a parent browser is used. If this was
42+
# a main frame then this would mean that the browser is being
43+
# destroyed, thus we can't send a process message using this browser.
44+
# There is no guarantee that this will get called for frames in the
45+
# main browser, if the browser is destroyed shortly after the frames
46+
# were released.
3947
Debug("V8ContextHandler_OnContextReleased()")
40-
pyBrowser = GetPyBrowser(cefBrowser)
41-
pyFrame = GetPyFrame(cefFrame)
42-
# User defined callback.
43-
clientCallback = pyBrowser.GetClientCallback("OnContextReleased")
44-
if clientCallback:
45-
clientCallback(pyBrowser, pyFrame)
48+
pyBrowser = GetPyBrowserById(browserId)
49+
pyFrame = GetPyFrameById(frameId)
50+
if pyBrowser and pyFrame:
51+
clientCallback = pyBrowser.GetClientCallback("OnContextReleased")
52+
if clientCallback:
53+
clientCallback(pyBrowser, pyFrame)
54+
else:
55+
if not pyBrowser:
56+
Debug("V8ContextHandler_OnContextReleased() WARNING: " \
57+
"pyBrowser not found")
58+
if not pyFrame:
59+
Debug("V8ContextHandler_OnContextReleased() WARNING: " \
60+
"pyFrame not found")
61+
RemovePyFrame(frameId)
4662
except:
4763
(exc_type, exc_value, exc_trace) = sys.exc_info()
4864
sys.excepthook(exc_type, exc_value, exc_trace)

0 commit comments

Comments
 (0)