[rfc][icedtea-web] make console aware of plugin messages

[Pavel Tisnovsky]

Hi Jiri,

this patch looks good in overall (mainly the synchronization during pushing/popping strings to a pipe!).
I have just minor objections:

netx/net/sourceforge/jnlp/util/logging/JavaConsole.java:

the code:
   while (true) {
       try{
       String s = br.readLine();
       if (s == null) {
           break;
       }
       processPluginMessage(s);

might be rewritten as:
   while ((String s = br.readLine()) != null) {
       processPluginMessage(s);
   }

OutputController.getLogger().log("Ended processing of plugin-debug-to-conosle " + file.getAbsolutePath());
                                                                      ^^^^^^
OutputController.getLogger().log("Started processing of plugin-debug-to-conosle " + file.getAbsolutePath());
                                                                        ^^^^^^
netx/net/sourceforge/jnlp/util/logging/LogConfig.java	Fri Nov 29 17:30:50 2013 +0100
+    * logging stuff fmay be interested in used config
^^^

That code smells a bit :-) but now I know why U did it - to use the same method in a console with
selectable output format:
+    public String toString() {
+        return toString(true, true, true, true, true, true, true);
+    }


plugin/icedteanp/IcedTeaNPPlugin.cc:

IMHO it's better to explicitly initialize those variables to NULL, even if it's not scrictly necessary:
+// Applet viewer debug pipe name.
+gchar* debug_pipe_name;
+

I'm not sure if it's better to flush the pipe before reconecting the C and Java part:
+  if (plugin_debug_to_console){
+    //jvm_up is now false
+    if (g_io_channel_shutdown (debug_to_appletviewer,


plugin/icedteanp/IcedTeaNPPlugin.h
+  nanosleep(&ts ,0); it might be time consuming, do you have any profiler
                      output showing if there is/is not a perf. problem here?


+        snprintf(ldebug_message, MESSAGE_SIZE, "%s%s", ldebug_header, ldebug_body); \
+        char ldebug_channel_message[MESSAGE_SIZE+50];                               \
+        struct timeval current_time;   \
+        gettimeofday (&current_time, NULL);\
+        if (jvm_up) {                  \
+          sprintf(ldebug_channel_message,"%s %ld %s", "plugindebug", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message);   \
            ^^ should be snprintf(ldebug_message, MESSAGE_SIZE+50 or you migth create new constant (which is better)
+          push_pre_init_messages(ldebug_channel_message);                           \
+        } else {                       \
+          sprintf(ldebug_channel_message,"%s %ld %s", "preinit_plugindebug", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message);   \
            ^^ should be snprintf(ldebug_message, MESSAGE_SIZE+50 or you migth create new constant (which is better)
+          push_pre_init_messages(ldebug_channel_message);                            \

+      struct timeval current_time;     \
+      gettimeofday (&current_time, NULL);\
+        if (jvm_up) {                  \
+          sprintf(ldebug_channel_message,"%s %ld %s", "pluginerror", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message);   \
+          push_pre_init_messages(ldebug_channel_message);                         \
+        } else {                       \
+          sprintf(ldebug_channel_message,"%s %ld %s", "preinit_pluginerror", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message);   \
+          push_pre_init_messages(ldebug_channel_message);                         \
dtto, a good candidate for refactoring :)

+                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, "Warning, althoug console is on, plugin debug conenctin do not exists. No plugn information will be dysplayed in console (only java ones).");
althoug <- typo
dysplayed <- typo

Cheers,
Pavel

----- Jiri Vanek <jvanek at redhat.com> wrote:
> hi!
> 
> So another part of "big console rewrote", now making the plugin console aware of C messages.
> Design is simple. New write pipe from plugin to java is created, and console then read messgaes from it, and merge to outputs.
> Right now it is not ideal, but with new console (which wil come later) it made much more sense.
> There is small refactoring which made header as object (pluginheader and javaheader classes) , and change to string when needed.
> The new pipe had best performance from tested files, current pipes and sockets
> two nits:
>   you will see my synchronisatoion by 1s sleep is not ideal in C part. any better idea?
> Also you can see hat imput is char*, and also char* is written to pipe, however queue is using string -  I was not able to make it work via with char* :( so there is nasty char* -> string -> char* retyping ;( but I do not wont to malock and free the memory manually :P
> 
> Again a patch is much more simple (600lines is headers refactoring!)
> 
> Thanx for review!
> 
> J.