Non osr mode - reparent window when necessary (when AWT/Swing deletes Canvas peer) - possible Windows fix about java-cef HOT 19 CLOSED

Comment 1. originally posted by magreenblatt on 2014-06-17T18:21:55.000Z:

Your approach seems reasonable. Please submit your changes as a patch file against the current trunk revision.

from java-cef.

Original comment by Anonymous.

Comment 2. originally posted by christophe.cornu on 2014-06-18T00:42:50.000Z:

Will have to look how to break some of the code out of my wrapper and back into CefBrowser and CefClient.

Note I haven't investigated yet the Mac equivalent. Maybe similar approach with a Canvas and shifting CALayer around will work.

from java-cef.

Original comment by Anonymous.

Comment 3. originally posted by christophe.cornu on 2014-06-25T15:59:23.000Z:

About to give it a try. I'll probably add a "reparent" test action to the MainFrame sample, to show how the current instance can be moved to a different parent / frame. Though a better example is a multi-tab app, since this also test the Z-order part.

from java-cef.

Original comment by Anonymous.

Comment 4. originally posted by christophe.cornu on 2014-06-25T20:18:44.000Z:

Here's the patch, over latest r92.

To try it out:
Recompile native lib, recompile java.
Start Detailed MainFrame > Tests > Reparent.
A new frame appears with a button "Reparent <". Click on button. Magically, the current browser moves into that frame. Click on the button again - the browser moves back and forth between the 2 frames.

You can bring a third frame (Tests > Reparent) and make it move between these 2 frames as long as you bring it back into the original frame first.

I've done minimal changes to the sample.

Note 1. In CefBrowserWr.java, removeNotify for the Canvas was previously calling parentWindowWillClose();
Of course with reparenting, we don't want to automatically close the browser in such case. We move it to a hidden frame until further notice - addNotify. The developer has to call cefBrowser.close() if they know they really don't need it anymore - e otherwise these will keep accumulate in the hidden frame until the final CefApp.shutdown closes them all.
E.g. in my case, I use it in multi-tabbing scenarios, and I know when the tab is meant to be closed I explicitly call cefBrowser.close() along with any other cleanup for that tab. But when the tab is reordered, we let the reparenting do the work (the actual canvas peer gets destroyed by AWT when tabs are moved around).

Note 2. During exit, frames get disposed and the removeNotify of Canvas'es get invoked. Once CefApp.shutdown is started, there's no need for Canvas.removeNotify in CefBrowserWr to do further reparenting. I found it useful to have a global variable to not mess up with removeNotify if we have started CefApp.shutdown. I did not expose this variable in this patch.

It'd be nice if similar approach can be done for the Mac with CALayer's.

HTH let me know what you think @ magreen

from java-cef.

Original comment by Anonymous.

Comment 5. originally posted by christophe.cornu on 2014-06-25T20:25:53.000Z:

screenshot of modified Detailed app

from java-cef.

Original comment by Anonymous.

Comment 6. originally posted by [email protected] on 2014-07-07T05:55:36.000Z:

Hi Christoph,
thanks for that patch. Some comments:

1. Please correct your indentations:
   * Use 2 spaces instead of tabs
   * Remove extra whitespaces (e.g. CefBrowserWr.java between addNotify() and removeNotify()
   * Add one space between "if" and "(" (e.g. CefApp.java, Hunk @ @ -255,6 +268,11 @ @ and others)
   * Avoid importing Java classes which aren't required (e.g. you're importing CefApp or CefClient in MenuBar.java but you never use them)
   * MenuBar,java: Hunk @ @ -339,11 +349,56 @ @ :
   - dlg.setVisible(true);^M
   + dlg.setVisible(true); ^M
   -> Remove white spaces after setVisible(true); then this line isn't changed

2. @ Note1: parentWindowWillClose() isn't used any more if you'll update to CEF version 3.1916.1749 (issue comment 94.)

3. Why are you using one global JFrame and per CefBrowser one additional Canvas? So in case of reparenting you'll get: "JFrame -> Canvas -> Canvas"? Wouldn't it be enough to set a new parent to the first canvas and avoid calling "super.removeNotify()"?

4. I don't think CefApp is the right place for JFrame. This breaks the architecture of JCEF because CefApp is more the glue to CEF (start/stop/message loop/...) instead of an UI thing. And beside that one CefApp keeps 1..n CefClients where one CefClient keeps 1…n CefBrowser instances. So you will get one JFrame might keep many CefBrowser instances from many CefClient instances.
   For me it would make more sense to keep one JFrame per CefBrowser or at lease one per CefClient. So you don't mix up CefBrowser instances from different CefClient instances.

5. Please make void N_SetParent(Canvas parent) private and create a protected wrapper method "setParent" within CefBrowser_N. Use try{…} catch(UnsatisfiedLinkError ule) { …} like the other protected methods within CefBrowser_N.

Regards, Kai

from java-cef.

Original comment by Anonymous.

Comment 7. originally posted by christophe.cornu on 2014-07-07T14:52:10.000Z:

Hi Kai:

Thanks for the feedback. Will work on it.

1. ok
2. ok. I did not introduce a new call to parentWindowWillClose(), it was there before. I just avoid calling it on platforms where we support reparenting. Unless CEF version 3.1916.1749 brings some hidden free behavior, things should be fine without it.
3. There certainly is more than one way to do this reparenting dance. Your suggestion is also interesting.

With my patch, this is what a complete typical reparenting cycle look like:
stage 1: SunAwtFrame1 > SunAwtCanvas1 > CefBrowserWindow1
The browser handle is under a Canvas under the Application's frame.

User initiates an action that drags the Canvas/browser to a different area (reorder tabs for example. This takes us to the transient stage 2.

stage 2: SunAwtFrame2(Hidden) > SunAwtCanvas2 > CefBrowserWindow1
In stage 2 we have temporarily parked CefBrowserWindow1 under a hidden frame. Its former canvas parent died its natural death. We gave it a new temporary canvas2 parent.

The application now created the drop destination (e.g. a new tab or new editor) and readds the CefBrowser to this new destination. This takes us to the last stage.

stage 3: SunAwtFrame1 > SunAwtCanvas3 > CefBrowserWindow1

The rational for this approach is to limit messing up with AWT since we are not AWT committers. We don't fight the natural handling of Canvas - we don't reparent the Canvas peer itself. We only reparent the CefBrowserWindow1 itself, which is a handle created by CEF and not AWT. So this gives us more chances not to be broken by future Java releases.
CefBrowserWindow1 is always parented to a Canvas. In case it ever has some assumption its parent is a Canvas, then that assumption is still true through the 3 stages of reparenting.

4. I understand the argument. The best solution over time will be the one that ports well to the other platforms and doesn't have side effect for the host application. It's nice to limit the number of hidden top windows. These top windows show up under MSFT Spy++, there's always a risk they can trigger some focus bug or break some internal AWT assumption. That's why at the moment I got good results with a single global hidden JFrame. I'd love to see reparenting done on the Mac as well to be more confident about what the ideal solution should be like.

Note in theory, we could skip creating a hidden parent JFrame/Canvas all together and just create a transient parent window in native code. But we've been through the pain of making CEF work with JFrame/Canvas so it's nice to keep the reparenting through that known working path.

These are just thoughts, I'd be happy to have others evolve this patch and make it work on Mac as well...

5. ok

from java-cef.

Original comment by aravind raghavan (Bitbucket: aQlQa).

hi Guys, Any update on the current status of this issue? I am interested in using this. I tried the hidden frame method suggested here. However the 'removeNotify' method causes JVM crashes when shutting down the JCEF. Any help/ideas?

Thanks.

from java-cef.

Attached win_reparent_20180612.patch is an updated version of the patch against current JCEF master. This also fixes a crash during shutdown after reparenting the browser. Support still needs to be added for non-Windows platforms.

from java-cef.

Original comment by Johann Scheiterbauer (Bitbucket: Phylanx, GitHub: Phylanx).

Hi!

We need exactly this patch.
See http://magpcss.org/ceforum/viewtopic.php?f=17&t=15983.

Will this usecase be submitted?
If needed we can test this patch on our usecases (windows only).

Can this patch be applied to 3325 (I'm currently upgrading to this version) or maybe also 2704 (our current productive version).
This would be great, our customers currently have 2704 in productive and would have a huge benefit of reparenting JCEF components...

from java-cef.

Original comment by Johann Scheiterbauer (Bitbucket: Phylanx, GitHub: Phylanx).

First Tests with version 3325 are promising.
Also with 2704, we just have a JVM crash when terminating. But that should be a implementation detail in our classes.

from java-cef.

addNotify/removeNotify are not being called so I suspect this patch is not actually working as intended.

from java-cef.

addNotify/removeNotify are not being called so I suspect this patch is not actually working as intended.

Nevermind, it was something wrong with my local build.

from java-cef.

Mac and Windows support for reparenting windowed browsers added in master revision 58a0028 (bb). This implementation uses a native temp window as the parent to reduce complexity in the Java code.

from java-cef.

• set state to "open"

from java-cef.

Original changes by Anonymous.

• set attachment to "win_reparent.diff"

from java-cef.

Original changes by Anonymous.

• set attachment to "reparent.png"

from java-cef.

• set attachment to "win_reparent_20180612.patch"
• edited description

from java-cef.

• changed state from "new" to "resolved"

from java-cef.