Mac: Add windowed rendering support

[GoogleCodeExporter]

What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?
Due OSR with manual message-loop handling is too slow, I've added the 
possibility to use On-Screen-Rendering for OS X systems.

What version of the product are you using? On what operating system?

Please provide any additional information below.

Therefore the Chromium API is told to be CALayer based because a CALayer has to 
be used within Java 7 to embed external content into Java.

In consequence of this, I've changed the behavior of CefContext.initialize, 
which must not be called out of windowOpened() because it will block
in non OSR mode. Reason for that is, that "CefRunMessageLoop" is called within 
it (using the main thread of the app). 
For details, please see the source code changes.

Benefits of Non-OSR:
Much Much faster (a good performance test: 
http://ie.microsoft.com/testdrive/performance/fishbowl/)
The Issues #15, #16, #22, #23 ,#24, #26 doesn't occur

Current pitfall:
The app crashes on exit (I think this is a "called on wrong thread" issue or 
something like that)

Original issue reported on code.google.com by k...@censhare.de on 15 Jan 2014 at 10:34

Attachments:

• 2014-01-15_04-added_windowed_rendering_for_mac.patch

[GoogleCodeExporter]

Thanks for the patch. Some comments:

1. line 46:

+  private boolean osr_;

Perhaps name this variable something more informative like |osr_enabled_|.

2. line 339:

+    // start application with argument "--osr" for offscreen-rendering

Same here, call it "--osr-enabled".

3. line 65:

+    if( osr_ ) {

The style guide would have you write this as "if (osr_) {". Same in other 
places that use if, while, etc.

4. line 287:

+  public long getWindowHandleOfCanvas(Canvas canvas) {
+    long result = 0;
+      Component comp = canvas;

Wrong indent.

5. line 388:

+    // On Mac OS X the message loop is performed by
+    // calling initialize, which will block the main process
+    // until the message loop will be quit
+    if  (!osr) {
+      CefContext.initialize("",osr);
+    }

For future reference on why this works: 
http://stackoverflow.com/questions/5642802/termination-of-program-on-main-thread
-exit

6. line 428:

+static bool g_use_osr = false;

We may want to support both windowed and osr use cases in the same application, 
in which case we shouldn't use a global variable.

7. line 454:

+      command_line->AppendSwitch("enable-speech-input");
+      command_line->AppendSwitch("enable-media-stream");

Instead of hard-coding arbitrary flags we should consider forwarding |args| 
from main().

8. line 559:

+// Determines whether the current thread is already attached to the VM,
+// and tells the caller if it needs to later DetachCurrentThread

What are the performance characteristics of attaching/detaching the same thread 
multiple times? Would it be better to attach each CEF thread one time and then 
detach before application exit?

9. line 615:

+#define BEGIN_ENV(e) \
+JNIEnv *e = NULL; \
+bool __shouldDetach = false; \
+if (GetJNIEnv(&e, &__shouldDetach) == JNI_OK && e != NULL) { \
+
+#define END_ENV(e) \
+DetachFromThread(&__shouldDetach); \

Indent the 2nd+ lines of macros 4 spaces.

10. line 703:

-
+  

Remove extra whitespace. Lots of this in util_mac.mm as well.

11. line 808:

+@end // BrowserStub

Need 2 spaces before "//".

Original comment by magreenb...@gmail.com on 22 Jan 2014 at 4:27

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Thanks for reviewing the patch.
Regarding your questions and comments:

1.) I've changed osr_ to osr_enabled_

2.) To correspond your example application ceflient, 
I've changed the param to "--off-screen-rendering-enabled"

3.-4. and 9.-11.) 
Sorry for that. I've corrected (hopefully all) that style-guide glitches.
Some of them came from the SDK, some of them I've missed to fix and some
of them came from copy-pasta.

5.) I've added your URL-reference to the in-line comment

6.) That's true, but it's currently hard to realize. I think this should happen
during the refactoring/extension of JCEF I've discussed with you (see my 
document I've sent to you).
Therefore I've added an TODO(jcef) remark to the code. 

7.) Yes, you're right. I've missed to remove that hard coded values. I've done 
it 
now. But according your suggestion, I've added an "forward params" mechanism 
into
Java/JNI which I've put into the separate Issue #33. 
(see: https://code.google.com/p/javachromiumembedded/issues/detail?id=33) 

8.) I never made any performance tests according this, but we're using these 
code
snippets (BEGIN_ENV, END_ENV) for years within our product and we never had any
performance issues. Beside this, currently the code is only used within the 
Obj-C
method "createBrowser" and therefore I haven't any misgivings about that.  

See attached patch-file for my changes.

Original comment by k...@censhare.de on 24 Jan 2014 at 11:07

Attachments:

• 2014-01-23_04-added_windowed_rendering_for_mac.patch

[GoogleCodeExporter]

Added in revision 21 with minor style and documentation changes.

Original comment by magreenb...@gmail.com on 30 Jan 2014 at 6:27

• Changed state: Fixed

[GoogleCodeExporter]

Missing jni_util_mac.mm added in revision 25.

Original comment by magreenb...@gmail.com on 31 Jan 2014 at 3:07