<AWT Dev> [9] Review Request: 8032187 [macosx] The fix for MACOSX_PORT-424 should be reworked
Artem Ananiev
artem.ananiev at oracle.com
Fri Feb 7 05:53:24 PST 2014
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?
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
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>
More information about the awt-dev
mailing list