<AWT Dev> Review Request for 8056911: Remove internal API usage from ExtendedRobot class

[Phil Race]

I lost track of this thread. If testing really needs new API
- and Yuri, I fully understand the jigsaw issue -
then I think Robot is a more appropriate place for it than Toolkit,
given that testing is a primary use case for Robot.

-phil.



On 9/9/14 5:33 AM, Anthony Petrov wrote:
> Well, if you want to take the risk, let's do it this way then.
>
> -- 
> best regards,
> Anthony
>
> On 9/9/2014 3:28 PM, Sergey Bylokhov wrote:
>> On 09.09.2014 15:15, Anthony Petrov wrote:
>>> and the current waitForIdle() would simply call waitForIdle(false).
>>> This would be safe enough and elegant enough as no new method names
>>> would have to be introduced.
>> I suggest to take this as a backup plan if something will go wrong. We
>> have a good coverage and we have enough time for testing.
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 9/9/2014 3:07 PM, Sergey Bylokhov wrote:
>>>> On 09.09.2014 14:40, Anthony Petrov wrote:
>>>>> Hi Dmitriy,
>>>>>
>>>>> Robot.sync() could also sound confusing if one remembers there's
>>>>> Toolkit.sync() already.
>>>>>
>>>>> Why not name it waitForNativeIdle(), analogous to the already 
>>>>> existing
>>>>> Robot.waitForIdle()? The methods perform similar actions, the only
>>>>> difference is the event queue they perform these actions on. If we
>>>>> choose this name, then the javadoc could be copied almost verbatim
>>>>> from the waitForIdle() method with a few additional "native" words
>>>>> inserted here and there.
>>>> The simpler way to fix it is to reimplement Robot.waitForIdle() on top
>>>> of SunToolkit.realsync() and extends of documentation of this method.
>>>> Because most of the time waitForIdle/autoWaitForIdle are used after 
>>>> some
>>>> key/mouse manipulation, I guess it is too verbose to write this:
>>>> Robot.waitForNativeIdle()
>>>> Robot.waitForIdle()
>>>>
>>>>> In general, I'm fine with this approach of resolving this issue.
>>>>>
>>>>> -- 
>>>>> best regards,
>>>>> Anthony
>>>>>
>>>>> On 9/4/2014 3:25 PM, Dmitriy Ermashov wrote:
>>>>>> Ok, I've prepared an updated version of patch
>>>>>> http://cr.openjdk.java.net/~dermashov/8056911/webrev.01/
>>>>>> Please, review new version.
>>>>>>
>>>>>> I've moved a method implementation to Robot class. We've decided 
>>>>>> that
>>>>>> adding new method to Toolkit class with similar to sync() method
>>>>>> functionality but with a different name could hardly look 
>>>>>> elegant, and
>>>>>> besides, new solution could be less conspicuous.
>>>>>>
>>>>>> Thanks,
>>>>>> Dima
>>>>>>
>>>>>> On 09/01/2014 04:49 PM, Yuri Nesterenko wrote:
>>>>>>> Phil,
>>>>>>>
>>>>>>> perhaps javadoc should be changed, yes.
>>>>>>> It is the first public spec for Dmitriy.
>>>>>>>
>>>>>>> As to the narrow purpose, are you serious?
>>>>>>> We have nothing to replace this invention. Original idea
>>>>>>> was to replace Toolkit.sync() with realSync() but
>>>>>>> after JDK-6252005 Denis left us, there was no resources
>>>>>>> and no hard demand to avoid the internal API.
>>>>>>> Now it is here. Demand is here, not resources. Are you going
>>>>>>> to design and implement something new to help SQE?
>>>>>>>
>>>>>>> -yan
>>>>>>>
>>>>>>> On 08/29/2014 07:33 PM, Phil Race wrote:
>>>>>>>> So you are proposing adding a new *public* API for this narrow
>>>>>>>> purpose.
>>>>>>>> And it calls out a whole bunch of internal classes in its apI doc
>>>>>>>> !?!
>>>>>>>>
>>>>>>>> 2642      * <p> The method calls {@link
>>>>>>>> sun.awt.SunToolkit#realSync} to
>>>>>>>> 2643      * sync with native event queue
>>>>>>>>
>>>>>>>> @throws  sun.awt.SunToolkit.IllegalThreadException if called on 
>>>>>>>> the
>>>>>>>> AWT event
>>>>>>>> 2646      *          dispatching thread
>>>>>>>> 2647      * @throws sun.awt.SunToolkit.OperationTimedOut if the
>>>>>>>> 2648      *          {@link sun.awt.SunToolkit.OperationTimedOut}
>>>>>>>> exception occurs in
>>>>>>>> 2649      *          {@link sun.awt.SunToolkit#realSync}
>>>>>>>> 2650      * @throws  sun.awt.SunToolkit.InfiniteLoop if the
>>>>>>>> 2651      *          {@link sun.awt.SunToolkit.InfiniteLoop}
>>>>>>>> exception occurs in
>>>>>>>> 2652      *          {@link sun.awt.SunToolkit#realSync}
>>>>>>>> 2653      * @throws  ClassCastException if default toolkit is not
>>>>>>>> SunToolkit
>>>>>>>>
>>>>>>>>
>>>>>>>> I am also not sure how confident I am that the statement
>>>>>>>>
>>>>>>>> his method guarantees that after
>>>>>>>> 2637      * return no additional Java events will be generated,
>>>>>>>> unless
>>>>>>>> 2638      * cause by user.
>>>>>>>>
>>>>>>>> is actually guaranteed.
>>>>>>>>
>>>>>>>>
>>>>>>>> My initial reaction to such a proposal is instead look hard for a
>>>>>>>> better
>>>>>>>> way even if it means re-writing all the tests.
>>>>>>>> There's also the proposed 'jdk.*' name space that can be 
>>>>>>>> considered
>>>>>>>> but re-writing the tests would be better.
>>>>>>>>
>>>>>>>> -phil.
>>>>>>>>
>>>>>>>> On 8/29/14 8:11 AM, Dmitriy Ermashov wrote:
>>>>>>>>> Hi awt team,
>>>>>>>>>
>>>>>>>>> Please review a fix for 8056911, remove internal API usage from
>>>>>>>>> ExtendedRobot class.
>>>>>>>>> We have to throw out all calls of sun.* packages from tests
>>>>>>>>> because of
>>>>>>>>> incompatibility with Jigsaw project.
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~dermashov/8056911/webrev.00/
>>>>>>>>>
>>>>>>>>> The CCC request will take place after the review process will be
>>>>>>>>> completed.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Dima
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>>
>>
>>