roadlabs / cefpython

Automatically exported from code.google.com/p/cefpython
0 stars 0 forks source link

Fix issues related to embedding multiple browsers in tabs #97

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There are many issues in CEF when embedding multiple browsers in tabs. It seems 
that it is always the first browser created to react when calling methods on 
the different frame objects. This includes methods like LoadUrl(), 
LoadString(), ExecuteJavascript(). Both browser1.frame.loadUrl() and 
browser2.frame.loadUrl() cause loading of an url in the first browser.

The fix to these issues might be to instantiate a separate client handler for 
each browser, instead of using one global handler, this was suggested as a 
solution in this topic by thile:
http://www.magpcss.org/ceforum/viewtopic.php?p=18212#p18212

Original issue reported on code.google.com by czarek.t...@gmail.com on 13 Dec 2013 at 5:30

GoogleCodeExporter commented 9 years ago
Using a separate client handler for each browser does not fix the problem.

Original comment by czarek.t...@gmail.com on 13 Dec 2013 at 8:32

GoogleCodeExporter commented 9 years ago
Unit tests need to be created for this behavior in CEF, so that Marshall can 
debug it and fix it.

Original comment by czarek.t...@gmail.com on 13 Dec 2013 at 9:30

GoogleCodeExporter commented 9 years ago
I have attached a sample of the issue.  Basically, there are two frames on the 
same page, each one with a browser pointed at www.cnn.com.  When you click the 
button at the bottom, the second browser should move to www.yahoo.com, but 
instead the first browser does.  Not sure what you consider a unit test for 
this, but at least this will demonstrate the issue.

Original comment by chri...@hotmail.com on 3 Mar 2014 at 5:00

Attachments:

GoogleCodeExporter commented 9 years ago
A C++ unit test is required. See CEF C++ unit tests here:

https://code.google.com/p/chromiumembedded/source/browse/trunk%2Fcef3%2Ftests%2F
unittests

Original comment by czarek.t...@gmail.com on 3 Mar 2014 at 11:00

GoogleCodeExporter commented 9 years ago
Btw. my solution to this issue was to provide url when creating browser. 
Creating multiple browsers in such manner works fine. When I had to load a 
different url in browser, I got around the bug in question by destroying 
current browser and then embedding it again in the same frame, but with a new 
url provided during browser creation.

Original comment by czarek.t...@gmail.com on 3 Mar 2014 at 2:11

GoogleCodeExporter commented 9 years ago
I'm pretty sure I found the bug: in "frame.pyx", there's a dict g_pyFrames. As 
key you use the frame ID. However, for different Browsers, frame IDs are reused 
(Browser 1 has a Frame ID 1 and Browser 2 has a Frame ID 1, but they are 
different frames).

Original comment by 9ha...@gmail.com on 6 Apr 2014 at 4:09

GoogleCodeExporter commented 9 years ago
Here's a fix. Works for me.

Original comment by 9ha...@gmail.com on 6 Apr 2014 at 4:38

Attachments:

GoogleCodeExporter commented 9 years ago
@comment #6 9hatt2
See CefFrame.GetIdentifier() [1]:

  ///
  // Returns the globally unique identifier for this frame.
  ///
  /*--cef()--*/
  virtual int64 GetIdentifier() =0;

The problem is that this doesn't seem to be true. We could try to store frames 
using (browser id + frame id) as a key to see if that helps. If this is only a 
problem with the GetIdentifier() method then it may resolve the issue. In the 
first post in this topic [2] I've provided logs from when window.open() was 
used, the GetIdentifier() returned a unique id in that case.

[1] 
https://code.google.com/p/cefpython/source/browse/cefpython/cef3/include/cef_fra
me.h?r=de675a80db49#184
[2] http://www.magpcss.org/ceforum/viewtopic.php?p=18212

@comment #7 9hatt2

Can you provide details of what tests have you made? Which methods, scenarios?

Original comment by czarek.t...@gmail.com on 6 Apr 2014 at 4:44

GoogleCodeExporter commented 9 years ago
According to Marshall frame identifiers are unique per render process, so 
g_pyFrames dict keys definitely need to be fixed. Though, I'm still not so sure 
if this fixes all the issues.

Original comment by czarek.t...@gmail.com on 6 Apr 2014 at 4:51

GoogleCodeExporter commented 9 years ago
> Can you provide details of what tests have you made? Which methods, scenarios?
I have a kivy application, that creates two browsers. Before the fix, if I 
called Navigate() on the 2nd, it actually changed the location in the 1st. I 
realized that the problem was that GetMainFrame (in browser.pyx) returned the 
same object for both browsers. Now this is fixed.

If FrameIds are unique per render process, we should rather use (render process 
id + frame id) as key instead of (browser id + frame id), right?

Original comment by 9ha...@gmail.com on 6 Apr 2014 at 5:36

GoogleCodeExporter commented 9 years ago
@comment #10 9hatt2

Browser id is unique globally. There isn't such thing as render process id.

Original comment by czarek.t...@gmail.com on 6 Apr 2014 at 6:39

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
if there's no render process identifier and there's no browser with two render 
processes, my fix is just right.
I have done futher testing with kivy and every single problem I had before is 
fixed now.
What tests need to be done until you can merge it?

Original comment by 9ha...@gmail.com on 7 Apr 2014 at 9:39

GoogleCodeExporter commented 9 years ago
Thanks for testing. I will merge it soon.

Original comment by czarek.t...@gmail.com on 7 Apr 2014 at 9:44

GoogleCodeExporter commented 9 years ago

Original comment by czarek.t...@gmail.com on 9 Apr 2014 at 6:43

GoogleCodeExporter commented 9 years ago
Fixed in revision 8a1793938f30.

@9hatt2 Thank your for the patch. That was a hell of a bug, lots of time lost 
trying to invent workarounds.

Also, fixed crashes in Frame.`LoadUrl` when a redirect to a new origin (a 
different domain) was being made and when javascript bindings were set. See 
Issue 130 for details.

Original comment by czarek.t...@gmail.com on 30 Jul 2014 at 4:33