<AWT Dev> [9] Review Request: JDK-8032960 Running forms URL throws NullPointerException in Javaconsole.

[Sergey Bylokhov]

Hi, Petr.
The fix looks good. But a few notes about the test.
  - There is no @bug.
  - You should check isFullScreenSupported and isDisplayChangeSupported.

On 27.02.2014 19:01, Petr Pchelko wrote:
> After an offline discussion with Sergey I've got a new version of he fix: http://cr.openjdk.java.net/~pchelko/9/8032960/webrev.02/
>
> Looks like previously the DesktopPropertyChangeSupport was intended to be called only on some EDT. But now we have to call it on a Toolkit thread,
> so I've removed the short path, so it now always posts an events and never calls listeners directly.
>
> With best regards. Petr.
>
> On 27.02.2014, at 18:18, Petr Pchelko <petr.pchelko at oracle.com> wrote:
>
>> Hello, Anthony.
>>
>> You were right, in case the Toolkit thread has an AppContext we should invoke the updateProperties through invokeLater as the DesktopPropertyChangeSupport could invoke a propertyChangeListener directly.
>>
>> Please see the updated fix here: http://cr.openjdk.java.net/~pchelko/9/8032960/webrev.01/
>>
>> I've also added an automatic test, but it's quite complicated.
>>
>> With best regards. Petr.
>>
>> On 20.02.2014, at 12:45, Anthony Petrov <anthony.petrov at oracle.com> wrote:
>>
>>> Hi Petr,
>>>
>>> The fire...() method is now invoked on the Toolkit thread, and I see that the Toolkit.DesktopPropertyChangeSupport.firePropertyChange() will fire the event on the current thread if the toolkit thread belongs to the current app context.
>>>
>>> The question is: are we 100% sure that on MS Windows the AWT Toolkit thread *never* belongs to a user app context? Even if we're running in an unusual threading mode (like the one for FX where we combine the EDT and the FX toolkit thread)?
>>>
>>> If this is so, meaning that we always only postEvent() from the T.DPCS.fire...() method on Windows, then I'm fine with the fix.
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>> On 2/19/2014 3:29 PM, Petr Pchelko wrote:
>>>> Hello, AWT Team.
>>>>
>>>> Please review the fix for the issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8032960
>>>> The fix is available at:
>>>> http://cr.openjdk.java.net/~pchelko/9/8032960/webrev.00/
>>>>
>>>> The problem:
>>>> The Toolkit thread does not have an AppContext so we can't use EventQueue.invokeLater on it.
>>>>
>>>> The solution:
>>>> Remove invokeLater. This is safe, because the updateProperties is thread safe. The result of this call is a post of a PropertyChangeEvent for each AppContext in the application.
>>>>
>>>> The test:
>>>> Hard to create, because we can't synthesize the WM_SETTINGCHANGE event.
>>>>
>>>> Thank you.
>>>> With best regards. Petr.
>>>>


-- 
Best regards, Sergey.