Hello Semyon, Sergey and Alexander,
Thank you very much for review of this fix. The new version of the fix
was created. Could you please review the second version of the fix.
Webrev (the 2nd version):
http://cr.openjdk.java.net/~alitvinov/8041694/jdk9/webrev.01
The second version of the fix is different from the first version only
in the regression test part, where the following changes were introduced:
1. The shell script was eliminated.
2. The number of threads involved in the test was reduced from 3 to 2,
therefore extra synchronization was removed.
3. The main thread waits until JFileChooser is shown using
"robot.delay(1000);" and "robot.waitForIdle();" calls instead of
previously used do/while loop with "Thread.sleep(50)" in the 3rd thread,
which is eliminated in the new version of the fix.
4. The test was made to be only Windows specific "if (OSInfo.getOSType()
!= OSInfo.OSType.WINDOWS) {", because, for example, on OS X JFileChooser
is absolutely different from Windows version and does not allow to type
a file name from the keyboard. Support of OS X platform by the
regression test would require complete rework of the whole test idea.
Thank you,
Anton
On 5/13/2016 8:19 PM, Semyon Sadetsky wrote:
On 5/13/2016 7:54 PM, Alexander Potochkin wrote:
Hello
I just checked the code in JDK9 and found that it still syncs only
with the EDT
and completely ignores the toolkit thread.
I cannot agree. Robot uses native SunToolkit.syncNativeQueue() to
syncs it with the native event queue.
Robot.waitForIdle() doesn't use SunToolkit.syncNativeQueue()
it uses SunToolkit.flushPendingEvents() but it is a different story
Alexander, I just double-checked the jdk9 code. See this
public synchronized void waitForIdle() {
checkNotDispatchThread();
SunToolkit.flushPendingEvents();
((SunToolkit) Toolkit.getDefaultToolkit()).realSync();
}
it calls both flushPendingEvents() and the realSync().
--Semyon
The javadoc for Robot.waitForIdle() says:
"Waits until all events currently on the event queue have been
processed."
realSync() does use syncNativeQueue()
so its javadoc states:
* Forces toolkit to synchronize with the native windowing
* sub-system, flushing all pending work and waiting for all the
* events to be processed. This method guarantees that after
* return no additional Java events will be generated, unless
* cause by user."
Thanks
alexp
--Semyon
So I don't recommend using it at all.
The most comprehensive approach is to use the realSync() method
from the SunToolkit
(as many of the Swing jtregs tests do).
This method is definitely more stable than waitForIdle and it works
well for most of the tests.
Thanks
alexp
On 5/11/2016 18:38, Semyon Sadetsky wrote:
On 5/11/2016 5:49 PM, Anton Litvinov wrote:
Hi Semyon,
Thank you for this information. Current version of the regression
test using the shell script is tested, cross platform and does
not contain any code specific to Windows platform. This shell
script is a copy of many other stable regression tests existing
in "jdk9/jdk/test" directory and is different from them only in
its commented test header (jtreg options like: @summary, @author)
and 7 code lines between "############### YOUR TEST CODE
HERE!!!!!!! #############" and "############### END YOUR TEST
CODE !!!!! ############".
I guess other platforms don't remove trailing spaces in the path.
What is so bad in usage of the well established approach based on
shell script?
Nothing really bad. It just a bit more complex then it might be.
You use Thread.sleep and critical sections in the test. Why not to
use AWT robot's waitForIdle() ?
--Semyon
Thank you,
Anton
On 5/11/2016 5:29 PM, Semyon Sadetsky wrote:
Hi Anton,
In windows you may use "\\?\" prefix with absolute path of a new
folder.
For example:
new File("\\\\?\\C:\\tmp\\test2 ").mkdir();
--Semyon
On 5/11/2016 4:47 PM, Anton Litvinov wrote:
Hello Sergey,
Thank you for review of this fix. No, unfortunately, on MS
Windows OS, if the method "java.io.File.mkdir()" is called on
"java.io.File" instance which contains trailing space
characters in the directory name, the corresponding directory
is created but without trailing space characters in its name in
file system.
The method "java.nio.file.Files.createDirectory(Path dir,
FileAttribute<?>... attrs)" cannot be used for this purpose
also, because "java.nio.file.Path" cannot be constructed for
the directory name ending with spaces and
"java.io.File.toPath()" throws the exception
"java.nio.file.InvalidPathException: Trailing char < > at index
N: <DIRECTORY_PATH>".
It is possible to create a directory with such a name from the
shell script on MS Windows OS, therefore I decided to use the
shell script for this regression test.
Thank you,
Anton
On 5/11/2016 4:16 PM, Sergey Bylokhov wrote:
Hi, Anton.
Probably the test can create the folder w/o the shell script?
On 11.05.16 15:14, Anton Litvinov wrote:
The bug consists in the fact that the method
"JFileChooser.getSelectedFile()" returns "java.io.File"
object which
does not contain trailing spaces in the directory name, in
spite of the
fact that the corresponding directory in the file system has
trailing
spaces in its name. The fix deletes the code in the method
"javax.swing.plaf.basic.BasicFileChooserUI.ApproveSelectionAction.actionPerformed"
which deliberately modifies the selected directory string
name by
removing trailing spaces from it.
All automatic regression tests from open and closed sets
located in
"javax/swing/JFileChooser" directories were run on MS Windows
7 OS
during verification of the fix.
