<AWT Dev> [9] Review Request: 8032187 [macosx] The fix for MACOSX_PORT-424 should be reworked

Anthony Petrov anthony.petrov at oracle.com
Thu Feb 6 03:41:34 PST 2014


Indeed, it wasn't an external developer who filed the original bug. 
Thanks for the clarification.

Still, I don't see any reason to post events for components that belong 
to our peers' implementations and shouldn't be accessible via public API.

I'd like to ask you again: does the original fix really break anyone? Or 
is the only reason to back it out is the fact that we don't like the 
implementation of the filtering mechanism (and I admit, I don't like it 
too)? If it's the latter, I'd suggest you to actually _rework_ that fix 
as the synopsis of 8032187 suggests, rather than simply remove it. Or 
leave it as is and keep the 8032187 bug open for now.

--
best regards,
Anthony

On 2/6/2014 2:45 AM, Sergey Bylokhov wrote:
> On 06.02.2014 1:26, Anthony Petrov wrote:
>> [ adding back the awt-dev@ mailing list ]
>>
>> Hi Sergey,
>>
>> This all boils down to whether we want to deliver events related to
>> our internal components to the user code or not. The fact that the
>> MACOSX_PORT-424 was filed by an external developer suggests that this
>> behavior may be unwanted. Also, common sense suggests that such events
>> are useless for external consumers, and hence sending them just
>> doesn't make any sense.
> No, it is not an external developer. It was filed because some test,
> which was provided by Apple, failed.
>>
>> Could you please clarify again exactly what the problem is with the
>> original fix for this issue? I do not quite like it myself (e.g. when
>> I see "enableEvents(0xFFFFFFFF)" - this just looks fishy, and the
>> setting/unsetting the listener looks like a hack, I agree), but I
>> don't see any serious consequences that that fix could cause. Could
>> you please elaborate on that?
> That's because this fix try to eliminate events during peers creation
> only. This was a hack and the goal was to fix one particular test.
>>
>> If we see that there's some serious flows with the original solution,
>> let's get rid of it. However, we'll need to file a new bug to deal
>> with all the unwanted events, and implement it some other way then.
> Why they are unwanted? AWT fires AWTEvents, and toolkit have an ability
> to catch all events, which are dispatched system-wide without any
> exceptions. As far as I understand toolkit listeners is kind of debug
> mechanism which can help to track all events.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 2/5/2014 5:54 PM, Sergey Bylokhov wrote:
>>> On 05.02.2014 16:34, Anthony Petrov wrote:
>>>> I agree that the listener should receive all the events for compound
>>>> controls. They make sense because all the parts of compound controls
>>>> are accessible via our public API, so the app could just assign
>>>> listeners to specific sub-components if it needed to.
>>> Not necessary, for example for our text component in xawt. The user
>>> should filter out unnecessary events anyway, because they can belongs to
>>> a different application or some internal components(like
>>> SharedOwnerFrame).
>>>>
>>>> However, this isn't true for our internal LWAWT components hierarchy.
>>>> This hierarchy is hidden from users (they can't access the underlying
>>>> Swing components via public API, and moreover, they shouldn't be able
>>>> to do so even if they want to). It is an implementation detail of the
>>>> LWAWT toolkit. And therefore, I don't see a reason to post events
>>>> related to this hidden hierarchy to a listener that can be installed
>>>> using the public API.
>>>>
>>>> Can we find another way to filter the events out?
>>> Only if we will add additional checks to the places, where
>>> Toolkit.enabledOnToolkit is used.
>>>>
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 2/5/2014 1:26 AM, Sergey Bylokhov wrote:
>>>>> On 04.02.2014 23:06, Anthony Petrov wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> From the bug report:
>>>>>>> After discussion, it was decided to accept MACOSX_PORT-424 as not a
>>>>>>> defect
>>>>>>
>>>>>> Are you saying that user code will start receiving hierarchy events
>>>>>> related to the internal hierarchy of our LWAWT peers? Why would
>>>>>> anyone
>>>>>> want to process these events and why would we want to post them to
>>>>>> user code? Is there any way to filter them out?
>>>>> Documentation of Toolkit.addAWTEventListener() states:
>>>>>       * Adds an AWTEventListener to receive all AWTEvents dispatched
>>>>>       * system-wide that conform to the given <code>eventMask</code>.
>>>>> ....
>>>>>       * Note:  event listener use is not recommended for normal
>>>>>       * application use, but are intended solely to support special
>>>>>       * purpose facilities including support for accessibility,
>>>>>       * event record/playback, and diagnostic tracing.
>>>>>
>>>>> And components intentionally cannot filter them out. All our non
>>>>> trivial
>>>>> compound components posts lots of such events.
>>>>>
>>>>>> While I agree that using reflection is a bad idea, but is there any
>>>>>> other real problem with the original fix for MACOSX_PORT-424? I.e.
>>>>>> does that fix break anything? If not, can we simply replace usages of
>>>>>> reflection with AWTAccessor-like mechanism?
>>>>> The problem not in reflection but in replacing global list of
>>>>> listeners.
>>>>>>
>>>>>> --
>>>>>> best regards,
>>>>>> Anthony
>>>>>>
>>>>>> On 2/4/2014 3:52 PM, Sergey Bylokhov wrote:
>>>>>>> Hello.
>>>>>>> Please review the fix for jdk 9.
>>>>>>>   - Initial fix for MACOSX_PORT-424 was reverted back.
>>>>>>>   - delegate.addNotify(),because it was called from
>>>>>>> delegateContainer.addNotify();
>>>>>>>   - Testcase was updated to filter out events not from the Frame:
>>>>>>>    84             if (e.getSource() instanceof Frame) {
>>>>>>>    85                 counter++;
>>>>>>>    86                 notify();
>>>>>>>    87             }
>>>>>>>
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8032187
>>>>>>> Webrev can be found at:
>>>>>>> http://cr.openjdk.java.net/~serb/8032187/webrev.00
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>


More information about the awt-dev mailing list