Re: [9] Review Request: 4419271 Provide support for scrolling-mechanisms of non-mouse input-devices

2016-12-05 Thread Sergey Malenkov
I approve

On Tue, Dec 6, 2016 at 4:21 AM, Sergey Bylokhov
 wrote:
> Hi, Sergey.
> Please take a look to the updated version (the names are updated):
> http://cr.openjdk.java.net/~serb/4419271/webrev.03
>
>> 28 нояб. 2016 г., в 7:01, Sergey Malenkov  написал(а):
>>
>> The fix looks good, but we realized that the direction is not always changed.
>> For example, Windows from VirtualBox on Mac.
>>
>> -jint scrollLines = 3;
>> +jint toScroll = 3;
>>
>> -UINT platformLines;
>> +UINT platformUnits;
>>
>> Also I prefer to use similar names for this variables as before.
>> For example, scrollUnits instead of toScroll
>>
>>
>>
>> On Thu, Nov 24, 2016 at 4:32 PM, Alexandr Scherbatiy
>>  wrote:
>>>
>>> The fix looks good to me.
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>>
>>> On 11/23/2016 5:20 PM, Sergey Bylokhov wrote:

 Hello.

 Please review the fix for jdk9.

 Support of WM_MOUSEHWHEEL for Swing was added (AWT components were not
 touched), which mostly the code symmetric to the WM_MOUSEWHEEL, except that
 I changed the direction to align it in Swing and OS. It seems that this
 functionality does not work on linux as well, at least I was not able to
 enable it. Will file a separate bug for jdk on linux.

 Bug: https://bugs.openjdk.java.net/browse/JDK-4419271
 Webrev can be found at: http://cr.openjdk.java.net/~serb/4419271/webrev.02

>>>
>>
>>
>>
>> --
>> Best regards,
>> Sergey A. Malenkov
>



-- 
Best regards,
Sergey A. Malenkov


Re: [9] Review request for 8166683: On macOS (Mac OS X) getting a ScreenMenuBar when not running "com.apple.laf.AquaLookAndFeel"

2016-12-05 Thread Alexander Zvegintsev
Actually there is no need in this property, this behavior can be 
disabled for


other L by setting apple.laf.useScreenMenuBar property to false.

http://cr.openjdk.java.net/~azvegint/jdk/9/8166683/03/

the fix is also reworked to remove mac specific stuff from shared code.

Thanks,
Alexander.

On 11/29/16 4:12 AM, Alexander Zvegintsev wrote:
I don't find any modern jdk9 prefix convention for such property, so 
I've named it "jdk.swing.disableForcedGlobalMenuBar"


http://cr.openjdk.java.net/~azvegint/jdk/9/8166683/02/


Thanks,
Alexander.

On 11/28/16 9:05 PM, Sergey Bylokhov wrote:

Looks fine, but here is some of my thoughts:
Since we tries to provide some kind of public API, I suggest to 
double check the solution again. In fact we tried to provide a 
support of the global menu on osx for all our L
 - Is it necessary to reference the Aqua from the shared code? in 
variables names and properties? Probably something like 
"globalMenuBar", etc? At least this will allow us to change 
implementation in any ways on other platforms w/o changing/adding the 
old/new properties.


On 15.11.16 17:39, Alexander Zvegintsev wrote:

Hi Sergey,

I've not found casting issues, but I've found the issue when previous
fix does not

treat dynamically changed "apple.laf.useScreenMenuBar" property
correctly. (e.g. ScreenMenuBarInputTwice test fails).

So please see the updated changeset:

http://cr.openjdk.java.net/~azvegint/jdk/9/8166683/01/

Thanks,
Alexander.

On 11/11/16 2:14 PM, Sergey Bylokhov wrote:

Hi, Alexander.
Did you run the tests on non-Aqua l? I assume that we can have a
places in other l where we try to cast the MenuBarUI to some
specific UI delegate.

On 09.11.16 16:58, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8166683/00/

for the issue

https://bugs.openjdk.java.net/browse/JDK-8166683

This fix adds support for ScreenMenuBar for L's other than Aqua.

With this fix it is enabled by default if apple.laf.useScreenMenuBar
property is true.

This behavior can be disabled by setting
apple.laf.disableForcedScreenMenuBar property to true.















Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding

2016-12-05 Thread Philip Race

I didn't eyeball what you changed but JPRT is now happy.
The build passes on all platforms...

-phil.

On 12/5/16, 2:47 PM, Lindenmaier, Goetz wrote:

Hi Phil,

sorry for that! I fixed it, there is an other occurrence, too.
I double checked all the casts, there was also a mp_flags<->  mp_sign wrong in 
mpi.c
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/
(Also rebased the change)

Best regards,
   Goetz


-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Monday, December 05, 2016 7:26 PM
To: Lindenmaier, Goetz; Sergey Bylokhov

Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
issues in awt coding

I tried it .. and just as well I did. It fails in the crypto code on Mac.

jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error:
expression which evaluates to zero treated as a null pointer constant of type
'mp_digit *' (aka 'unsigned long *') [-Werror,-Wnon-literal-null-conversion]
  k.dp = (mp_digit)0;
 ^~~
1 error generated.

-phil.



On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:

Hi Phil,

that would be great, unfortunately I don't have access to JPRT.

Thanks,
Goetz.


-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Friday, December 02, 2016 8:46 PM
To: Lindenmaier, Goetz; Sergey Bylokhov

Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
issues in awt coding

I had no other comments, except that it would be good to be sure
that this builds on all relevant platforms .. using the 'blessed' compilers.
If you like I can submit a JPRT job for you based on this patch.

-phil.

On 12/01/2016 11:58 PM, Lindenmaier, Goetz wrote:

Hi Phil,

I added the initialization to the other function.
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.04/

I missed that because Coverity didn't complain.
It's not a perfect tool, but it finds sufficient issues
to make it worthwhile.   Is the other awt code fine?

Best regards,
 Goetz.




-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Donnerstag, 1. Dezember 2016 22:31
To: Lindenmaier, Goetz; Sergey

Bylokhov

; Vincent Ryan



Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net>;

security-

d...@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor

issues

in awt coding

Sorry. it is
ops->GetRasInfo(env, ops, lockInfo);
that initialises it ..


That is still before the dereference
Anyway, what was the reason for updating one function, but not the

other.

I don't mind the change, but the inconsistency looks odd.

-phil.

On 12/01/2016 03:03 AM, Lindenmaier, Goetz wrote:

Hi Phil,


Did you actually compile this ? The variable is called "rasBase", not
"resBase".

Yes, I compiled it, and I fixed the error, but that was another repo
I use for the coverity checks.  Somehow it did not find its way into
the webrev.

I don't see where ops->Lock() initializes this field.
ops->GetRasInfo(env, ops, lockInfo); does so if it resolves
to BufImg_GetRasInfo().  I can't look in our code scan results
because the issue is gone after fixing it, that lists the path
of execution that leads to the issue.
If you are sure this is correct I will remove the initialization,
else I will also put it into the other method.

DBN_GetPixelPointer checks lockInfo->rasBase for NULL and
does what looks like the error case if it's NULL. Therefore NULL
seemed a good initialization to me.

Best regards,
  Goetz.




-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 20:59
To: Sergey Bylokhov; Lindenmaier,

Goetz

; Vincent Ryan



Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net>;

security-

d...@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix

minor

issues

in awt coding

Hi Goetz,


  DataBufferNative.c
Using uninitialized value lockInfo.rasBase when calling

DBN_GetPixelPointer.

   75 lockInfo.resBase = NULL;

Did you actually compile this ? The variable is called "rasBase", not
"resBase".

And strictly there is no problem since inside  DBN_GetPixelPointer
the code calls ops->Lock which should initialise this.
A "rasBase" of 0 isn't really any better than a random one ..

Also I don't see why there's a problem here and not in
the function immediately following since it is the exact same case.

-phil.

On 11/30/2016 10:00 AM, Sergey Bylokhov wrote:

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to 

Re: [9] Review Request: 4419271 Provide support for scrolling-mechanisms of non-mouse input-devices

2016-12-05 Thread Sergey Bylokhov
Hi, Sergey.
Please take a look to the updated version (the names are updated):
http://cr.openjdk.java.net/~serb/4419271/webrev.03

> 28 нояб. 2016 г., в 7:01, Sergey Malenkov  написал(а):
> 
> The fix looks good, but we realized that the direction is not always changed.
> For example, Windows from VirtualBox on Mac.
> 
> -jint scrollLines = 3;
> +jint toScroll = 3;
> 
> -UINT platformLines;
> +UINT platformUnits;
> 
> Also I prefer to use similar names for this variables as before.
> For example, scrollUnits instead of toScroll
> 
> 
> 
> On Thu, Nov 24, 2016 at 4:32 PM, Alexandr Scherbatiy
>  wrote:
>> 
>> The fix looks good to me.
>> 
>> Thanks,
>> Alexandr.
>> 
>> 
>> On 11/23/2016 5:20 PM, Sergey Bylokhov wrote:
>>> 
>>> Hello.
>>> 
>>> Please review the fix for jdk9.
>>> 
>>> Support of WM_MOUSEHWHEEL for Swing was added (AWT components were not
>>> touched), which mostly the code symmetric to the WM_MOUSEWHEEL, except that
>>> I changed the direction to align it in Swing and OS. It seems that this
>>> functionality does not work on linux as well, at least I was not able to
>>> enable it. Will file a separate bug for jdk on linux.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-4419271
>>> Webrev can be found at: http://cr.openjdk.java.net/~serb/4419271/webrev.02
>>> 
>> 
> 
> 
> 
> -- 
> Best regards,
> Sergey A. Malenkov



Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding

2016-12-05 Thread Lindenmaier, Goetz
Hi Phil, 

sorry for that! I fixed it, there is an other occurrence, too.
I double checked all the casts, there was also a mp_flags <-> mp_sign wrong in 
mpi.c
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/
(Also rebased the change)

Best regards,
  Goetz

> -Original Message-
> From: Phil Race [mailto:philip.r...@oracle.com]
> Sent: Monday, December 05, 2016 7:26 PM
> To: Lindenmaier, Goetz ; Sergey Bylokhov
> 
> Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> issues in awt coding
> 
> I tried it .. and just as well I did. It fails in the crypto code on Mac.
> 
> jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error:
> expression which evaluates to zero treated as a null pointer constant of type
> 'mp_digit *' (aka 'unsigned long *') [-Werror,-Wnon-literal-null-conversion]
>  k.dp = (mp_digit)0;
> ^~~
> 1 error generated.
> 
> -phil.
> 
> 
> 
> On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:
> > Hi Phil,
> >
> > that would be great, unfortunately I don't have access to JPRT.
> >
> > Thanks,
> >Goetz.
> >
> >> -Original Message-
> >> From: Phil Race [mailto:philip.r...@oracle.com]
> >> Sent: Friday, December 02, 2016 8:46 PM
> >> To: Lindenmaier, Goetz ; Sergey Bylokhov
> >> 
> >> Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>
> >> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> >> issues in awt coding
> >>
> >> I had no other comments, except that it would be good to be sure
> >> that this builds on all relevant platforms .. using the 'blessed' 
> >> compilers.
> >> If you like I can submit a JPRT job for you based on this patch.
> >>
> >> -phil.
> >>
> >> On 12/01/2016 11:58 PM, Lindenmaier, Goetz wrote:
> >>> Hi Phil,
> >>>
> >>> I added the initialization to the other function.
> >>> http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.04/
> >>>
> >>> I missed that because Coverity didn't complain.
> >>> It's not a perfect tool, but it finds sufficient issues
> >>> to make it worthwhile.   Is the other awt code fine?
> >>>
> >>> Best regards,
> >>> Goetz.
> >>>
> >>>
> >>>
>  -Original Message-
>  From: Phil Race [mailto:philip.r...@oracle.com]
>  Sent: Donnerstag, 1. Dezember 2016 22:31
>  To: Lindenmaier, Goetz ; Sergey
> Bylokhov
>  ; Vincent Ryan
> >> 
>  Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>;
> >> security-
>  d...@openjdk.java.net
>  Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> >> issues
>  in awt coding
> 
>  Sorry. it is
> ops->GetRasInfo(env, ops, lockInfo);
>  that initialises it ..
> 
> 
>  That is still before the dereference
>  Anyway, what was the reason for updating one function, but not the
> >> other.
>  I don't mind the change, but the inconsistency looks odd.
> 
>  -phil.
> 
>  On 12/01/2016 03:03 AM, Lindenmaier, Goetz wrote:
> > Hi Phil,
> >
> >> Did you actually compile this ? The variable is called "rasBase", not
> >> "resBase".
> > Yes, I compiled it, and I fixed the error, but that was another repo
> > I use for the coverity checks.  Somehow it did not find its way into
> > the webrev.
> >
> > I don't see where ops->Lock() initializes this field.
> > ops->GetRasInfo(env, ops, lockInfo); does so if it resolves
> > to BufImg_GetRasInfo().  I can't look in our code scan results
> > because the issue is gone after fixing it, that lists the path
> > of execution that leads to the issue.
> > If you are sure this is correct I will remove the initialization,
> > else I will also put it into the other method.
> >
> > DBN_GetPixelPointer checks lockInfo->rasBase for NULL and
> > does what looks like the error case if it's NULL. Therefore NULL
> > seemed a good initialization to me.
> >
> > Best regards,
> >  Goetz.
> >
> >
> >
> >> -Original Message-
> >> From: Phil Race [mailto:philip.r...@oracle.com]
> >> Sent: Mittwoch, 30. November 2016 20:59
> >> To: Sergey Bylokhov ; Lindenmaier,
> >> Goetz
> >> ; Vincent Ryan
> >> 
> >> Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>;
> >> security-
> >> d...@openjdk.java.net
> >> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix
> minor
>  issues
> >> in awt coding
> >>
> >> Hi Goetz,
> >>
> >>>  DataBufferNative.c
> >>> Using uninitialized value lockInfo.rasBase when calling
>  DBN_GetPixelPointer.
> >>   75 lockInfo.resBase = NULL;
> 

Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding

2016-12-05 Thread Phil Race

I tried it .. and just as well I did. It fails in the crypto code on Mac.

jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error: expression 
which evaluates to zero treated as a null pointer constant of type 'mp_digit *' 
(aka 'unsigned long *') [-Werror,-Wnon-literal-null-conversion]
k.dp = (mp_digit)0;
   ^~~
1 error generated.

-phil.



On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:

Hi Phil,

that would be great, unfortunately I don't have access to JPRT.

Thanks,
   Goetz.
  

-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Friday, December 02, 2016 8:46 PM
To: Lindenmaier, Goetz ; Sergey Bylokhov

Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
issues in awt coding

I had no other comments, except that it would be good to be sure
that this builds on all relevant platforms .. using the 'blessed' compilers.
If you like I can submit a JPRT job for you based on this patch.

-phil.

On 12/01/2016 11:58 PM, Lindenmaier, Goetz wrote:

Hi Phil,

I added the initialization to the other function.
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.04/

I missed that because Coverity didn't complain.
It's not a perfect tool, but it finds sufficient issues
to make it worthwhile.   Is the other awt code fine?

Best regards,
Goetz.




-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Donnerstag, 1. Dezember 2016 22:31
To: Lindenmaier, Goetz ; Sergey Bylokhov
; Vincent Ryan



Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>;

security-

d...@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor

issues

in awt coding

Sorry. it is
   ops->GetRasInfo(env, ops, lockInfo);
that initialises it ..


That is still before the dereference
Anyway, what was the reason for updating one function, but not the

other.

I don't mind the change, but the inconsistency looks odd.

-phil.

On 12/01/2016 03:03 AM, Lindenmaier, Goetz wrote:

Hi Phil,


Did you actually compile this ? The variable is called "rasBase", not
"resBase".

Yes, I compiled it, and I fixed the error, but that was another repo
I use for the coverity checks.  Somehow it did not find its way into
the webrev.

I don't see where ops->Lock() initializes this field.
ops->GetRasInfo(env, ops, lockInfo); does so if it resolves
to BufImg_GetRasInfo().  I can't look in our code scan results
because the issue is gone after fixing it, that lists the path
of execution that leads to the issue.
If you are sure this is correct I will remove the initialization,
else I will also put it into the other method.

DBN_GetPixelPointer checks lockInfo->rasBase for NULL and
does what looks like the error case if it's NULL. Therefore NULL
seemed a good initialization to me.

Best regards,
 Goetz.




-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 20:59
To: Sergey Bylokhov ; Lindenmaier,

Goetz

; Vincent Ryan



Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>;

security-

d...@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor

issues

in awt coding

Hi Goetz,


 DataBufferNative.c
Using uninitialized value lockInfo.rasBase when calling

DBN_GetPixelPointer.

  75 lockInfo.resBase = NULL;

Did you actually compile this ? The variable is called "rasBase", not
"resBase".

And strictly there is no problem since inside  DBN_GetPixelPointer
the code calls ops->Lock which should initialise this.
A "rasBase" of 0 isn't really any better than a random one ..

Also I don't see why there's a problem here and not in
the function immediately following since it is the exact same case.

-phil.

On 11/30/2016 10:00 AM, Sergey Bylokhov wrote:

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright
messages.
New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
 Goetz.

(Am I correct that your openJdk name is Vinnie?)


-Original Message-
From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 14:53
To: Lindenmaier, Goetz 
Cc: awt-dev@openjdk.java.net; security-...@openjdk.java.net
Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding

Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in the
4 source
code headers will need to be updated/added.

BTW p11_mutex.c 

Re: OS X - Java Native Foundation

2016-12-05 Thread Michael Hall
> On Dec 4, 2016, at 8:54 PM, serb  wrote:
> 
> Hello, Michael.
> 
> You can find the source of these macro in the JNFJNI.h which is a part of 
> "Java Native Foundation»:
> /System/Library/Frameworks/JavaVM.framework/Frameworks/JavaNativeFoundation.framework/

Using the header for documentation was suggested on the old Apple java-dev 
list. 
I just remembered there being html or pdf documentation as well. Maybe I used 
the html because I can’t find where I have a copy of the pdf. Searching, I do 
find busted links for a pdf document but not the document itself yet.
I can go by the headers but if anyone did happen to pull a copy of the pdf 
maybe just email me a copy.

Thanks,

Michael Hall