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

[Jiri Vanek]

On 9/10/18 4:26 PM, Laurent Bourgès wrote:
> Jiri & Mario,
> 
> Here is an updated patch:
> http://cr.openjdk.java.net/~lbourges/itw/patch_EDT.log.2

Thank you very much., Double thanx for keeping old file to allow me a diff.
> 
> Mario, I tried webrev.ksh but it did not work with ITW repo. Do you have a specific script for IcedTea ?
> (I also uploaded the previous patch so you can see incremental diffs)
> 
> I fixed EDT reentrance in SwingUtils.invoke methods to fix the CertificatePane issue.

Great!
> 
...
>      >     +            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....
>      >
>      >
>      > I prefer having these dialogs (with my Aspro2 app) showing a title.
>      > Note: other titles are missing for few DialogType (later ?)
> 
>     Feel free to include them now or later
>     Still, those would be nice to separate to another changeset (all the "new" titles.)
> 
> 
> Removed.

Thank you. looking forward for next patches.
> 
> 
>      >
>      >     Unless you have more opinion around " MESAGE_ALL?" part, or   you/me confirm  that
...>
> 
> I fixed the reentrance in SwingUtils.invokeAndWait() when called from EDT.
> It was already in my (next) patch.
> 
> 
>     Please note http://icedtea.classpath.org/hg/icedtea-web/rev/5290684409aa it will interact with the
>     patch for bleeding edge.  Maybe it is worthy to backport it to 1.7 and to adapt your patch on
>     top of it?
> 
> 
> Please backport to 1.7.
done. Plus few more.
Head and 1.7 are now very very similar. All except rust launchers.

> I merged so the patch should not have these changes after backport:

So your patch now do not apply correctly, but the two rejected hunks can be happily ignored/removed. 
Feel free to provide updated version, but not necessary.
> 
> @@ -741,8 +742,10 @@
>               }
>               if (!headless) {
>                   try {
> -                    new JWindow().getOwner();
> -                } catch (Exception ex) {
> +                    if (GraphicsEnvironment.isHeadless()) {
> +                        throw new HeadlessException();
> +                    }
> +                } catch (HeadlessException ex) {
> 
> 
>      >
>      >
>      >     I will start testing asap, and the minor update for "MESAGE_ALL?" can go in meantime
>      >
>      > Definitely, later.
> 
>     Well we are makeing a turn. Feel free to adapt it. I woudl vote to inclussion to stderr via
>     ERROR_all, and I would vote to separate the titles hunk to separate changeset.
> 
> 
> Agreed.
> 
>      >
>      >
>      >     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.
>      >
>      >
>      > I think this property is mainly for debugging purposes (internals); so I do not think it
>     should be
>      > advertised.
>     ok.
> 
>     Sorry for troubles.
>        I was not investigating the freeze of certificates details, can you please look into it?
> 
> 
> Done.
> 
> I tested again and both 'javaws -viewer' and itw-settings dialogs are OK.
> 

Nothing more on my nitpicking list. Unless you or Mario will bring some news tomorrow, I will apply 
it to 1.7 and will look forward for patch for head, and other patches.
Note, that I'm on vacation 13-23.9, and I will be a bit delayed with replies. Lets guess a week 
delayed :)


I will write an changelog on your behalf. If you wish to write it on your own, feel free.


J.

J.