<AWT Dev> [9] Review Request: 8032187 [macosx] The fix for MACOSX_PORT-424 should be reworked
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Fri Feb 7 06:14:17 PST 2014
On 07.02.2014 17:53, Artem Ananiev wrote:
>
> On 2/7/2014 1:39 AM, Sergey Bylokhov wrote:
>> Hmm, we need someone else, who will solve a problem.
>> Artem, what do you think?
>
> I would suggest to suppress all the AWT events from our internal
> components. That is, if a component is accessible from application
> (e.g. JScrollPane's viewport), its events should be sent to toolkit
> listeners, otherwise not. As an application developer I would be
> surprised, if my application with a Frame and a Button will fire any
> events for JButton, right?
Ok. I'll prepare the new version of the fix.
>
> Thanks,
>
> Artem
>
>> 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