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

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Thu Feb 6 13:39:48 PST 2014 

Hmm, we need someone else, who will solve a problem.
Artem, what do you think?

On 07.02.2014 0:37, Anthony Petrov wrote:
> 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
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>


-- 
Best regards, Sergey.

More information about the awt-dev mailing list