[rfc] [icedtea-web] Fix EDT hanging on OpenJDK 11

[Jiri Vanek]

On 09/06/2018 11:36 PM, Laurent Bourgès wrote:
> Jiri,
> Here is an updated patch, fixing all your concerns and I made more cleanups and fixed code formatting.

Thank you.
> 
...

>      > +            OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, "Loading
>     class: " + mainName);
>      > +
> 
>     Is this really necessary as MESAGE_ALL?
> 
> 
> Moved in logs, but I liked having it in my console as my app is starting slowly (from home @ DSL) 
> due to lots of http queries ... latency is then important and I like knowing what's going on, nevermind.

Then you can runit in --verbose, and you will see.
is "nevermind" ok, or "well, if you insits"
If it is so crucial, than it may be discussed to move it to default output.
> 
> 
>      >               Class<?> mainClass = app.getClassLoader().loadClass(mainName);
>      >
>      >               Method main = mainClass.getMethod("main", new Class<?>[] { String[].class });
>      > @@ -567,6 +569,7 @@
>      >
>      >               main.setAccessible(true);
>      >
>      > +            OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, "Starting
>     application ...");
>     Same.
> 
> 
> Fixed.
same.
...

Looking second times to the splashscreen and headless detections changes - are they still working?-)


@@ -284,6 +292,11 @@
              dialogTitle = "Security Warning";
          else if (dtype == DialogType.AUTHENTICATION)
              dialogTitle = "Authentication Required";
+        else if (dtype == DialogType.MISSING_ALACA) {
+            dialogTitle = "Security Warning";
+        } else if (dtype == DialogType.MATCHING_ALACA) {
+            dialogTitle = "Security Warning";
+        }

I overlooked this in first reading. How had you come over it? As it is only cosmetic change....

> 
>      > +public final class SwingUtils {
>      > +
>      > +    private static final boolean INFO_DIALOG = false;
>      > +    private static final boolean DEBUG_EDT = false;
>      > +    private static final boolean TRACE_EDT = false;
> 
>     Does any one f those sense to been enabled by ITW verbose mode? Does anyone have sense to be
>     enabled
>     via -Djava.awt.itw.whatever ?
> 
> 
> Only DEBUG_EDT may be helpful, so I fixed:
> DEBUG_EDT = System.getProperty("icedtea-web.edt.debug", "false").equalsIgnoreCase("true");
cool

> 
> 
> 
>     You are using sout here. This is violating ITW logging bottle neck. However ITW have logging -
>     unless disabled in itw settngs - bound to console, which is awt. So it can be contra productive.
> 
> 
> I wrapped such calls in trace(String) and explained that it is only for TRACING/DEBUG EDT (ITW 
> internals) so logging / console may interfere as it is using also EDT (chicken-egg problem) !

Yah. expected resolution.
> 
> 
>     I do not have preference for those two classes. I have never been so deep in AWT debugging to need
>     this, But I'm happy to learn the approach.  And tehy can go in.
> 
> 


Unless you have more opinion around " MESAGE_ALL?" part, or   you/me confirm  that 
splash/headless/sandbox  stopped working, I'm ok with this to be pushed to 1.7, with promises that 
push to future 1.8 will follow asap.

I will now start the integration suite of ITW (alas! have not made applets reasonably working, so 
javaws only), with hope to catch sandboxissue, if any. (will take long, as I need to create some 
base "before the patch")
after that, I will do some manual testing
after that I will push
and then I will hope for 1.8 version, which I will test in same way as current with 1.7.

Does it sound good to you?

I will start testing asap, and the minor update for "MESAGE_ALL?" can go in meantime


Btw (realized now)  there is scary part which generate documentation for ITW, and 
icedtea-web.edt.debug should be mentioned. Howeve, I'm afraid there is no precedence for proeprty.
I will look into it later.


-- 
Jiri Vanek
Senior QE engineer, OpenJDK QE lead, Mgr.
Red Hat Czech
jvanek at redhat.com    M: +420775390109