<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 12:37:40 PST 2014 

Well, I disagree. My point is that AWTEvents for internal components 
should not be delivered to listeners that can be installed via public 
API. If JDK developers need to trace those events, they could use some 
logging facilities. Application developers do not need to handle those 
events.

--
best regards,
Anthony

On 2/6/2014 4:24 PM, Sergey Bylokhov wrote:
> On 06.02.2014 15:41, Anthony Petrov wrote:
>> 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.
> The reason is that we easily can check if someone flood the system. And
> it is not contradict the specification.
>>
>> 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.
> My point is that the previous fix is incorrect, because there was no bug
> in jdk and the test should be updated instead.
>>
>> --
>> 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