Skip to content

Commit a010e68

Browse files
cztomczakcztomczak
committed
Fix errors due to NULL frames sent from Renderer process (cztomczak#431).
Remove Python callback references properly. Fix PyCharm code inspection warnings in main_test.py.
1 parent cf0761e commit a010e68

File tree

7 files changed

+81
-79
lines changed

7 files changed

+81
-79
lines changed

‎src/client_handler/client_handler.cpp‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ bool ClientHandler::OnProcessMessageReceived(
3030
CefProcessId source_process,
3131
CefRefPtr<CefProcessMessage> message)
3232
{
33+
// Return true if message was handled.
3334
if (source_process != PID_RENDERER) {
3435
return false;
3536
}
@@ -42,6 +43,12 @@ bool ClientHandler::OnProcessMessageReceived(
4243
if (arguments->GetSize() == 1 && arguments->GetType(0) == VTYPE_INT) {
4344
int64 frameId = arguments->GetInt(0);
4445
CefRefPtr<CefFrame> frame = browser->GetFrame(frameId);
46+
if (!frame.get()) {
47+
// Frame was already destroyed while IPC messaging was
48+
// executing. Issue #431. User callback will not be
49+
// executed in such case.
50+
return true;
51+
}
4552
V8ContextHandler_OnContextCreated(browser, frame);
4653
return true;
4754
} else {
@@ -56,6 +63,11 @@ bool ClientHandler::OnProcessMessageReceived(
5663
&& arguments->GetType(1) == VTYPE_INT) {
5764
int browserId = arguments->GetInt(0);
5865
int64 frameId = arguments->GetInt(1);
66+
// Even if frame was alrady destroyed (Issue #431) you still
67+
// want to call V8ContextHandler_OnContextReleased as it releases
68+
// some resources. Thus passing IDs instead of actual
69+
// objects. Cython code in V8ContextHandler_OnContextReleased
70+
// will handle a case when frame is already destroyed.
5971
V8ContextHandler_OnContextReleased(browserId, frameId);
6072
return true;
6173
} else {
@@ -75,8 +87,13 @@ bool ClientHandler::OnProcessMessageReceived(
7587
int64 frameId = arguments->GetInt(0);
7688
CefString functionName = arguments->GetString(1);
7789
CefRefPtr<CefListValue> functionArguments = arguments->GetList(2);
78-
CefRefPtr<CefFrame> frame = browser->GetFrame(frameId);
79-
V8FunctionHandler_Execute(browser, frame, functionName,
90+
// Even if frame was already destroyed (Issue #431) you still
91+
// want to call V8FunctionHandler_Execute, as it can run
92+
// Python code without issues and doesn't require an actual
93+
// frame. Thus passing IDs instead of actual objects. Cython
94+
// code in V8FunctionHandler_Execute will handle a case when
95+
// frame is already destroyed.
96+
V8FunctionHandler_Execute(browser, frameId, functionName,
8097
functionArguments);
8198
return true;
8299
} else {
@@ -100,18 +117,6 @@ bool ClientHandler::OnProcessMessageReceived(
100117
" messageName=ExecutePythonCallback";
101118
return false;
102119
}
103-
} else if (messageName == "RemovePythonCallbacksForFrame") {
104-
CefRefPtr<CefListValue> arguments = message->GetArgumentList();
105-
if (arguments->GetSize() == 1 && arguments->GetType(0) == VTYPE_INT) {
106-
int frameId = arguments->GetInt(0);
107-
RemovePythonCallbacksForFrame(frameId);
108-
return true;
109-
} else {
110-
LOG(ERROR) << "[Browser process] OnProcessMessageReceived():"
111-
" invalid arguments,"
112-
" messageName=ExecutePythonCallback";
113-
return false;
114-
}
115120
}
116121
return false;
117122
}

‎src/frame.pyx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ cdef void RemovePyFrame(int browserId, object frameId) except *:
9393
del pyFrame
9494
del g_pyFrames[uniqueFrameId]
9595
g_unreferenced_frames.append(uniqueFrameId)
96+
RemovePythonCallbacksForFrame(frameId)
9697
else:
9798
Debug("RemovePyFrame() FAILED: uniqueFrameId = %s" % uniqueFrameId)
9899

@@ -112,6 +113,8 @@ cdef void RemovePyFramesForBrowser(int browserId) except *:
112113
del pyFrame
113114
del g_pyFrames[uniqueFrameId]
114115
g_unreferenced_frames.append(uniqueFrameId)
116+
# RemovePythonCallbacksForBrowser already called
117+
# in LifespanHandler_OnBeforeClose.
115118

116119
cdef class PyFrame:
117120
cdef CefRefPtr[CefFrame] cefFrame

‎src/handlers/v8context_handler.pyx‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
include "../cefpython.pyx"
1212
include "../browser.pyx"
13+
include "../frame.pyx"
1314

1415
cdef public void V8ContextHandler_OnContextCreated(
1516
CefRefPtr[CefBrowser] cefBrowser,
@@ -23,7 +24,7 @@ cdef public void V8ContextHandler_OnContextCreated(
2324
pyBrowser = GetPyBrowser(cefBrowser, "OnContextCreated")
2425
pyBrowser.SetUserData("__v8ContextCreated", True)
2526
pyFrame = GetPyFrame(cefFrame)
26-
# User defined callback.
27+
# User defined callback
2728
clientCallback = pyBrowser.GetClientCallback("OnContextCreated")
2829
if clientCallback:
2930
clientCallback(browser=pyBrowser, frame=pyFrame)
@@ -52,19 +53,15 @@ cdef public void V8ContextHandler_OnContextReleased(
5253
if not pyBrowser:
5354
Debug("OnContextReleased: Browser doesn't exist anymore, id={id}"
5455
.format(id=str(browserId)))
56+
RemovePyFrame(browserId, frameId)
5557
return
5658
pyFrame = GetPyFrameById(browserId, frameId)
57-
if pyBrowser and pyFrame:
59+
# Frame may already be destroyed while IPC messaging was executing
60+
# (Issue #431).
61+
if pyFrame:
5862
clientCallback = pyBrowser.GetClientCallback("OnContextReleased")
5963
if clientCallback:
6064
clientCallback(browser=pyBrowser, frame=pyFrame)
61-
else:
62-
if not pyBrowser:
63-
Debug("V8ContextHandler_OnContextReleased() WARNING: "
64-
"PyBrowser not found")
65-
if not pyFrame:
66-
Debug("V8ContextHandler_OnContextReleased() WARNING: "
67-
"PyFrame not found")
6865
RemovePyFrame(browserId, frameId)
6966
except:
7067
(exc_type, exc_value, exc_trace) = sys.exc_info()

‎src/handlers/v8function_handler.pyx‎

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,48 @@
44

55
include "../cefpython.pyx"
66
include "../browser.pyx"
7+
include "../frame.pyx"
78

89
cdef public void V8FunctionHandler_Execute(
910
CefRefPtr[CefBrowser] cefBrowser,
10-
CefRefPtr[CefFrame] cefFrame,
11-
CefString& cefFunctionName,
12-
CefRefPtr[CefListValue] cefFunctionArguments
11+
int64 frameId,
12+
CefString& cefFuncName,
13+
CefRefPtr[CefListValue] cefFuncArgs
1314
) except * with gil:
1415
cdef PyBrowser pyBrowser
15-
cdef PyFrame pyFrame
16-
cdef py_string functionName
17-
cdef object function
18-
cdef list functionArguments
16+
cdef CefRefPtr[CefFrame] cefFrame
17+
cdef PyFrame pyFrame # may be None
18+
cdef py_string funcName
19+
cdef object func
20+
cdef list funcArgs
1921
cdef object returnValue
20-
cdef py_string jsErrorMessage
22+
cdef py_string errorMessage
2123
try:
2224
pyBrowser = GetPyBrowser(cefBrowser, "V8FunctionHandler_Execute")
23-
pyFrame = GetPyFrame(cefFrame)
24-
functionName = CefToPyString(cefFunctionName)
25-
Debug("V8FunctionHandler_Execute(): functionName=%s" % functionName)
25+
cefFrame = cefBrowser.get().GetFrame(frameId)
26+
if cefFrame.get():
27+
pyFrame = GetPyFrame(cefFrame)
28+
else:
29+
pyFrame = None
30+
funcName = CefToPyString(cefFuncName)
31+
Debug("V8FunctionHandler_Execute(): funcName=%s" % funcName)
2632
jsBindings = pyBrowser.GetJavascriptBindings()
27-
function = jsBindings.GetFunctionOrMethod(functionName)
28-
if not function:
33+
func = jsBindings.GetFunctionOrMethod(funcName)
34+
if not func:
2935
# The Renderer process already checks whether function
3036
# name is valid before calling V8FunctionHandler_Execute(),
3137
# but it is possible for the javascript bindings to change
3238
# during execution, so it's possible for the Browser/Renderer
3339
# bindings to be out of sync due to delay in process messaging.
34-
jsErrorMessage = "V8FunctionHandler_Execute() FAILED: " \
35-
"python function not found: %s" % functionName
36-
Debug(jsErrorMessage)
37-
# Raise a javascript exception in that frame.
38-
pyFrame.ExecuteJavascript("throw '%s';" % jsErrorMessage)
40+
errorMessage = "V8FunctionHandler_Execute() FAILED: " \
41+
"python function not found: %s" % funcName
42+
NonCriticalError(errorMessage)
43+
# Raise a javascript exception in that frame if it still exists
44+
if pyFrame:
45+
pyFrame.ExecuteJavascript("throw '%s';" % errorMessage)
3946
return
40-
functionArguments = CefListValueToPyList(cefBrowser,
41-
cefFunctionArguments)
42-
returnValue = function(*functionArguments)
43-
if returnValue is not None:
44-
Debug("V8FunctionHandler_Execute() WARNING: function returned" \
45-
"value, but returning values to javascript is not " \
46-
"supported, functionName=%s" % functionName)
47+
funcArgs = CefListValueToPyList(cefBrowser, cefFuncArgs)
48+
func(*funcArgs)
4749
except:
4850
(exc_type, exc_value, exc_trace) = sys.exc_info()
4951
sys.excepthook(exc_type, exc_value, exc_trace)

‎src/python_callback.pyx‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,20 @@
55
include "cefpython.pyx"
66

77
cdef int g_pythonCallbackMaxId = 0
8+
# [callbackId] = (browserId, frameId, func)
89
cdef dict g_pythonCallbacks = {}
910

1011
# TODO: send callbackId using CefBinaryNamedValue, see:
11-
# http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=10881
12+
# http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=10881
13+
1214
cdef struct PythonCallback:
1315
int callbackId
1416
char uniqueCefBinaryValueSize[16]
1517

1618
cdef CefRefPtr[CefBinaryValue] PutPythonCallback(
1719
object browserId,
1820
object frameId,
19-
object function
21+
object func
2022
) except *:
2123
global g_pythonCallbacks
2224
global g_pythonCallbackMaxId
@@ -29,12 +31,11 @@ cdef CefRefPtr[CefBinaryValue] PutPythonCallback(
2931
pyCallback.callbackId = g_pythonCallbackMaxId
3032
cdef CefRefPtr[CefBinaryValue] binaryValue = CefBinaryValue_Create(
3133
&pyCallback, sizeof(pyCallback))
32-
# [0] browserId, [1] frameId, [2] function.
33-
g_pythonCallbacks[g_pythonCallbackMaxId] = (browserId, frameId, function)
34+
g_pythonCallbacks[g_pythonCallbackMaxId] = (browserId, frameId, func)
3435
return binaryValue
3536

3637
cdef public void RemovePythonCallbacksForFrame(
37-
int frameId
38+
object frameId
3839
) except * with gil:
3940
# Cannot remove elements from g_pythonCallbacks (dict) while iterating.
4041
cdef list toRemove = []
@@ -46,8 +47,8 @@ cdef public void RemovePythonCallbacksForFrame(
4647
for callbackId in toRemove:
4748
del g_pythonCallbacks[callbackId]
4849
Debug("RemovePythonCallbacksForFrame(): " \
49-
"removed python callback, callbackId = %s" \
50-
% callbackId)
50+
"removed python callback, callbackId = %s" \
51+
% callbackId)
5152
except:
5253
(exc_type, exc_value, exc_trace) = sys.exc_info()
5354
sys.excepthook(exc_type, exc_value, exc_trace)
@@ -62,33 +63,28 @@ cdef void RemovePythonCallbacksForBrowser(
6263
for callbackId in toRemove:
6364
del g_pythonCallbacks[callbackId]
6465
Debug("RemovePythonCallbacksForBrowser(): " \
65-
"removed python callback, callbackId = %s" \
66-
% callbackId)
66+
"removed python callback, callbackId = %s" \
67+
% callbackId)
6768

6869
cdef public cpp_bool ExecutePythonCallback(
6970
CefRefPtr[CefBrowser] cefBrowser,
7071
int callbackId,
71-
CefRefPtr[CefListValue] cefFunctionArguments,
72+
CefRefPtr[CefListValue] cefFuncArgs,
7273
) except * with gil:
73-
cdef object function
74-
cdef list functionArguments
74+
cdef object func
75+
cdef list funcArgs
7576
cdef object returnValue
7677
try:
7778
global g_pythonCallbacks
7879
if callbackId in g_pythonCallbacks:
79-
# [0] browserId, [1] frameId, [2] function.
80-
function = g_pythonCallbacks[callbackId][2]
81-
functionArguments = CefListValueToPyList(
82-
cefBrowser, cefFunctionArguments)
83-
returnValue = function(*functionArguments)
84-
if returnValue is not None:
85-
Debug("ExecutePythonCallback() WARNING: function returned" \
86-
"value, but returning values to javascript is not " \
87-
"supported, function name = %s" % function.__name__)
80+
func = g_pythonCallbacks[callbackId][2]
81+
funcArgs = CefListValueToPyList(
82+
cefBrowser, cefFuncArgs)
83+
func(*funcArgs)
8884
return True
8985
else:
9086
Debug("ExecutePythonCallback() FAILED: callback not found, " \
91-
"callbackId = %s" % callbackId)
87+
"callbackId = %s" % callbackId)
9288
return False
9389
except:
9490
(exc_type, exc_value, exc_trace) = sys.exc_info()

‎src/subprocess/cefpython_app.cpp‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -337,15 +337,10 @@ void CefPythonApp::OnContextReleased(CefRefPtr<CefBrowser> browser,
337337
// ------------------------------------------------------------------------
338338
// 2. Remove python callbacks for a frame.
339339
// ------------------------------------------------------------------------
340-
// If this is the main frame then the message won't arrive
341-
// to the browser process, as browser is being destroyed,
342-
// but it doesn't matter because in LifespanHandler_BeforeClose()
340+
// This is already done via RemovePyFrame called from
341+
// V8ContextHandler_OnContextReleased.
342+
// If this is the main frame then in LifespanHandler_BeforeClose()
343343
// we're calling RemovePythonCallbacksForBrowser().
344-
message = CefProcessMessage::Create("RemovePythonCallbacksForFrame");
345-
arguments = message->GetArgumentList();
346-
// TODO: int64 precision lost
347-
arguments->SetInt(0, (int)(frame->GetIdentifier()));
348-
browser->SendProcessMessage(PID_BROWSER, message);
349344
// ------------------------------------------------------------------------
350345
// 3. Clear javascript callbacks.
351346
// ------------------------------------------------------------------------

‎unittests/main_test.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,15 @@ def test_main(self):
154154
# supports passing functions as callbacks when called from
155155
# javascript, and as a side effect any value and in this case
156156
# a property can also be a function.
157-
bindings.SetProperty("test_property3_function", external.test_property3_function)
157+
bindings.SetProperty("test_property3_function",
158+
external.test_property3_function)
158159
bindings.SetProperty("cefpython_version", cef.GetVersion())
159160
bindings.SetObject("external", external)
160161
browser.SetJavascriptBindings(bindings)
161162
subtest_message("browser.SetJavascriptBindings() ok")
162163

163164
# Test Request.SetPostData(list)
165+
# noinspection PyArgumentList
164166
req = cef.Request.CreateRequest()
165167
req_file = os.path.dirname(os.path.abspath(__file__))
166168
req_file = os.path.join(req_file, "main_test.py")
@@ -173,6 +175,7 @@ def test_main(self):
173175
subtest_message("cef.Request.SetPostData(list) ok")
174176

175177
# Test Request.SetPostData(dict)
178+
# noinspection PyArgumentList
176179
req = cef.Request.CreateRequest()
177180
req_data = {b"key": b"value"}
178181
req.SetMethod("POST")
@@ -199,6 +202,7 @@ def test_main(self):
199202
time.sleep(0.01)
200203

201204
# Automatic check of asserts in handlers and in external
205+
# noinspection PyTypeChecker
202206
for obj in [] + client_handlers + [global_handler, external]:
203207
test_for_True = False # Test whether asserts are working correctly
204208
for key, value in obj.__dict__.items():

0 commit comments

Comments
 (0)