Changing CefClient to handle with 1..* browser instances and CefApp to handle with 1..* CefClient instances.

[GoogleCodeExporter]

This issue is an enhancement of JCEF (patch 9 of 10) 
----------------------------------------------------
Purpose: Changing CefClient to handle with 1..* browser instances
and CefApp to handle with 1..* CefClient instances.

This patch relies on the attached patch file of issue #52. 
If you want to try these code changes, add the patch of issue #52 first and 
afterwards add the patch file attached to this issue. Otherwise you'll get some 
patch-errors.

This patch moves the most complexity of using JCEF, into jcef itself.
That means there is no need to deal with CefMessageHandler or other 
things within the users application.

The native methods createBrowser and getWindowHandle are move to CefBrowser 
itself.
Beside this the window redered version and the OSR version of the browser
is split up into two independet classes (CefBrowserOsr and CefBrowserWr) which
inherit from CefBrower_N (now an abstract class instead of an interface).
An instance of one of those browsers is created by using CefBrowserFactory out 
of
CefClient (method createBrowser). The native part of the browser is created 
by using CefCreateBrowser instead of CefCreateBrowserSync. Consequently 
attaching
the native CefBrowser instance to the java instance is done in the native code 
by the CefLifeSpanHandler (method OnAfterCreated).

After applying this patch, you can use jcef for example on this simple way:
public static void main(String [] args) {
  CefApp app = CefApp.getInstance(args);
  CefClient client = app.createClient();

  // optional add handlers
  client.addDisplayHandler(new DisplayHandlerAdapter() {
    @Override
    //whatever you're intrested in
  });
  // add additional handlers, if required

  CefBrowser browser = client.createBrowser("http://some.where", osrEnabled, transparent);

  Component ui = browser.getUIComponent();
  // do whatever with the ui.
  // ...
}

Please see attached patch file for further informations.

Original issue reported on code.google.com by k...@censhare.de on 8 Mar 2014 at 8:01

• Blocking: #55, #54
• Blocked on: #52

Attachments:

• 09_2014-03-07_issue_XX_introduced_multiple_CefClients_with_multiple_CefBrowsers.patch

[GoogleCodeExporter]

Original comment by magreenb...@gmail.com on 12 Mar 2014 at 9:50

• Now blocking: #55

[GoogleCodeExporter]

Original comment by magreenb...@gmail.com on 12 Mar 2014 at 9:53

• Added labels: Type-Enhancement
• Removed labels: Type-Defect
• Now blocked on: #52

[GoogleCodeExporter]

Original comment by magreenb...@gmail.com on 12 Mar 2014 at 9:53

• Now blocking: #54

[GoogleCodeExporter]

This change appears not to work on Windows:

1. Using off-screen rendering the application crashes on exit with the 
following error:

[0328/140611:FATAL:context.cc(307)] Check failed: OnInitThread().

This indicates that CefShutdown is being called on the wrong thread (it needs 
to be called on the same thread as CefInitialize).

2. Using windowed rendering the browser window never loads (application hangs 
with wait cursor).

Original comment by magreenb...@gmail.com on 28 Mar 2014 at 1:10

[GoogleCodeExporter]

Original comment by magreenb...@gmail.com on 28 Mar 2014 at 1:14

• Changed state: Started

[GoogleCodeExporter]

Hi Marshall,

> 2. Using windowed rendering the browser window never loads
------------------------------------------------------------
The problem is caused by an AWT/Swing deadlock according AWTTreeLock.
See example on http://www.javaspecialists.eu/archive/Issue101.html for a similar
issue.

So I've changed the behavior of the methods
- CefApp.initialize()
- CefApp.doMessageLoopWork()
- CefApp.shutdown()
to be handled by SwingUtilities.invokeAndWait() respectively 
SwingUtilities.invokeLater().

On this way I make sure, that the JCEF initialization, the message loop as well 
as the shutdown are handled on the same thread. Creation of CefBrowser 
instances 
should already be done on that thread because it's triggered by the native 
method 
call CefBrowserHost::CreateBrowser.

Please see attached patch file for my solution (apply this attached patch file 
after you've applied the previous patch file 
"09_2013-03-07_issue_XX_introduced...").

> 1. Using off-screen rendering the application crashes on exit
---------------------------------------------------------------
I can't reproduce this problem on my test-system by using OSR mode, but I 
realized
that it occurs by using the windowed rendering mode.

Unfortunately I can't find the cause for that problem. 
Since I'm using invokeAndWait() and invokeLater() as well as 
CefBrowserHost::CreateBrowser(), all relevant calls should be performed on the 
same thread, shouldn't they? But the problem still occurs... 

Maybe I missed something? What's about "context.cc"? Why the error occurs there?
What's its job?

By the way: this issue occurs since the update to the CEF 1750 branch (revision 
28).
So if I go back to r27 I did' got such error messages. The first time I've 
recognized it on r28.

Do you have any suggestions?

Original comment by k...@censhare.de on 4 Apr 2014 at 8:37

Attachments:

• 09_2014-04-04_solved_swing_deadlock.patch

[GoogleCodeExporter]

@comment#6: Thanks for looking into the problem. The assertion in context.cc 
means that CefShutdown is being called on a different thread than 
CefInitialize. You can try adding System.out.println("shutdown on " + 
Thread.currentThread()); before calling N_Shutdown() and compare to the 
"initialize on ..." output to verify that it's the same thread.

Original comment by magreenb...@gmail.com on 4 Apr 2014 at 7:54

[GoogleCodeExporter]

@comment#7:
Hi Marshall,

I've managed to fix both issues. Please see attached patch file for my solution 
(apply this attached patch file after you've applied the previous patch file 
"09_2013-03-07_issue_XX_introduced...").

So the problem wasn't that shutdown() was called on a wrong thread but rather 
that it
wasn't called anywhere. Why? Because CefApp.finalize() wasn't called 
automatically
when the application exited. This caused CEF to call CefShutdown() after the 
dispatcher thread was terminated by java and that - in fact - caused the 
described 
crash on exit of the app.

To solve this problem, I've done the following:
- Removed the needless method CefApp.finalize()
- Added the new method CefApp.dispose() (explanation see below)
- Moved handling of initialize(), doMessageLoopWork(), shutdown() to the 
dispatcher thread (as written in comment #6)
- Performing "destroyAllBrowsers()" just before shutting down
- hook into "removeNotify()" of Canvas and forward it as 
"parentWindowWillClose()" to CEF
- calling "removeNotify()" on the Canvas each time the browser window is 
closed/destroyed
- Added parentWindowWillClose() from CEF to JCEF
- Removed runMessageLoop() due it isn't used any more

The new method "dispose()" in CefApp has to be called by MainFrame for an 
ordeded
shutdown of CEF. That means, that it 
1) closes all CefBrowser instances (and informs CEF by calling 
"ParentWindowWillClose()")
2) closes all CefClient instances
3) stops running the message look
and after all is cleaned up it
4) calls CefShutdown

Up to now, didn't find a way to trigger dispose() automatically because
- finalize() isn't called at any time due the singleton pattern of CefApp
- shutdown hooks can't be used because they are executed within the dispatcher 
thread
  but the dispatcher thread is required for shutting down CEF. That ends in a dead lock.

Btw: This patch fixes the crash which occured by terminating the app on 
Macintosh as well.

I think calling a "dispose()" method before App termination is a suitable way.

Original comment by k...@censhare.de on 11 Apr 2014 at 12:24

Attachments:

• 09_2014-04-11_solved_swing_deadlock_and_crash_on_exit.patch

[GoogleCodeExporter]

@comment#8: Thanks, I think adding a dispose method is fine. I've committed the 
combined patches as revision 40.

There's still one shutdown crash on Mac -- if you select "Quit MainFrame" from 
the top application menu without closing the window first. We can address that 
in issue #41.

Original comment by magreenb...@gmail.com on 11 Apr 2014 at 4:10

• Changed state: Fixed