RFR: 8178698: javax/sound/midi/Sequencer/MetaCallback.java failed with timeout
On m1 mac sometimes (in very rare instance) it takes a long time to get the sequencer so increasing timeout helps with that. Also on m1 mac sequencer got notified about the first MetaMessage twice. The reason is unclear but watching for that fixes the problem. Also on Linux sometimes process hangs indefinitely without finishing hold by the com.sun.media.sound.MediaDispatcher. Closing the sequencer at the end of the test helps with that. - Commit messages: - Handle case when on m1 mac handler being called twice on the same message in the beginning of playback - 8178698: javax/sound/midi/Sequencer/MetaCallback.java failed with timeout Changes: https://git.openjdk.org/jdk/pull/11157/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11157&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8178698 Stats: 14 lines in 2 files changed: 9 ins; 1 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/11157.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11157/head:pull/11157 PR: https://git.openjdk.org/jdk/pull/11157
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v6]
On Mon, 14 Nov 2022 19:44:17 GMT, Xue-Lei Andrew Fan wrote:
>> Hi,
>>
>> May I have this update reviewed?
>>
>> The sprintf is deprecated in Xcode 14 because of security concerns, and the
>> use of it causing building failure. The build could pass if warnings are
>> disabled for codes that use sprintf method. For the long run, the sprintf
>> could be replaced with snprintf. This patch is trying to check if snprintf
>> could be used.
>>
>> Thanks,
>> Xuelei
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> delete swp file
Mostly okay. There are some places where the result from `os::snprintf` could
be used instead of a later `strlen`. Most of those are pre-existing (so could
be considered for later cleanups), but in at least one case there was a new
strlen call introduced, so making the code slightly worse.
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 226:
> 224: char buf[512];
> 225: os::snprintf(buf, sizeof(buf), "0x%02x:0x%x:0x%03x:%d", _cpu,
> _variant, _model, _revision);
> 226: if (_model2) os::snprintf(buf+strlen(buf), sizeof(buf) - strlen(buf),
> "(0x%03x)", _model2);
Instead of using `strlen(buf)` (now called twice!) to get the number of
characters written, use the result of the first call to `os::snprintf`.
src/hotspot/os/bsd/attachListener_bsd.cpp line 251:
> 249: BsdAttachOperation* BsdAttachListener::read_request(int s) {
> 250: char ver_str[8];
> 251: os::snprintf(ver_str, sizeof(ver_str), "%d", ATTACH_PROTOCOL_VER);
We later use `strlen(ver_str)` where we could instead use the result of
`os::snprintf`.
src/hotspot/os/bsd/attachListener_bsd.cpp line 294:
> 292: (atoi(buf) != ATTACH_PROTOCOL_VER)) {
> 293: char msg[32];
> 294: os::snprintf(msg, sizeof(msg), "%d\n",
> ATTACH_ERROR_BADVERSION);
Rather than using `strlen(msg)` in the next line, use the result from
`os::snprintf`.
src/hotspot/os/bsd/attachListener_bsd.cpp line 414:
> 412: // write operation result
> 413: char msg[32];
> 414: os::snprintf(msg, sizeof(msg), "%d\n", result);
Rather than using strlen(msg) in the next line, use the result from
os::snprintf.
src/hotspot/share/classfile/javaClasses.cpp line 2532:
> 2530: // Print module information
> 2531: if (module_name != NULL) {
> 2532: buf_off = (int)strlen(buf);
`buf_off` could be the result of `os::snprintf` instead of calling `strlen`.
src/hotspot/share/code/dependencies.cpp line 780:
> 778: }
> 779: } else {
> 780: char xn[12]; os::snprintf(xn, sizeof(xn), "x%d", j);
Pre-existing very unusual formatting; put a line break between the statements.
-
Changes requested by kbarrett (Reviewer).
PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8293862: javax/swing/JFileChooser/8046391/bug8046391.java failed with 'Cannot invoke "java.awt.Image.getWidth(java.awt.image.ImageObserver)" because "retVal" is null'
On Fri, 11 Nov 2022 08:18:19 GMT, Tejesh R wrote: > Observation found when JFileChooser is instantiated in WindowsLookAndFeel > which invokes getSystemIcon() from WindowsFileChooserUI class. Could not find > the exact root cause so predicting it to be an issue with icons not loaded > where resolutionVariants map is empty in _public Image > getResolutionVariant(double width, double height) _. Hence proposing a null > check before accessing getWidth(). Fix is tested in CI system. > Yeah, the only way the retVal can become null would-be if `resolutionVariants` doesn't have an Icon. I could not reproduce the bug since it is intermittent, I came to this conclusion only by code analysis though. Should I check for `resolutionVariants` if its empty and return null without proceeding further..? I guess both the ways `null` will be returned? - PR: https://git.openjdk.org/jdk/pull/11104
Re: RFR: 8296083: javax/swing/JTree/6263446/bug6263446.java fails intermittently on a VM [v2]
On Mon, 14 Nov 2022 21:17:43 GMT, Phil Race wrote: > So it now passes 100% of the time on the OCI systems ? At least in my testing, links of those OCI jobs have been put in JBS too - PR: https://git.openjdk.org/jdk/pull/11057
Re: RFR: 8295006: Colored text is not shown on disabled checkbox and radio button with GTK LAF for bug4314194. [v4]
On Mon, 14 Nov 2022 04:53:50 GMT, Abhishek Kumar wrote: >> Existing test >> `open/test/jdk/javax/swing/JRadioButton/4314194/bug4314194.java` was not >> showing colored text for disabled checkbox and radiobutton in GTK LAF. >> >> The fix is to get the disabled state color for checkbox and radiobutton from >> UIManager if it exists. >> >> Test case `open/test/jdk/javax/swing/JRadioButton/4314194/bug4314194.java` >> has been checked in CI pipeline. Link is attached in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Used bitwise operator for condition check Marked as reviewed by tr (Committer). Fix is tested and working fine. - PR: https://git.openjdk.org/jdk/pull/10755
RFR: 8148041: Test java/awt/Mouse/TitleBarDoubleClick/TitleBarDoubleClick fails on Ubuntu with mouseReleased event after double click on title bar
This test was problem listed with ["some tests that leave windows open on the desktop" reason](https://github.com/openjdk/jdk/commit/fd8df3adf13f5df8f5f95482b8e0a70bc519f39f) The main issue with the test is that it may pass, but in fact the actual test was not even performed. `jtreg` kills the test window after exiting main and ends the test before it double clicks on window's title(actual test starts from `windowActivated` event). So the test was refactored and stabilized: * robot invocation moved from EDT to main thread. * throwing test exception is performed only after frame is disposed * extra delay added after showing window. Old Ubuntu 16.04 and macOS 10.10.5 failures is no longer reproducible on modern systems. CI testing looks good. - Commit messages: - refactor - clean up Changes: https://git.openjdk.org/jdk/pull/11150/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11150&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8148041 Stats: 71 lines in 2 files changed: 19 ins; 8 del; 44 mod Patch: https://git.openjdk.org/jdk/pull/11150.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11150/head:pull/11150 PR: https://git.openjdk.org/jdk/pull/11150
Re: RFR: 8295369: Update LCMS to 2.14 [v3]
On Mon, 14 Nov 2022 19:09:40 GMT, Alisen Chung wrote: >> These look to me like tabs have not been replaced by spaces.. >> I saw a lot of "shifts" and I spot-checked some of them but they looked like >> ones where upstream had readily changed indentation. >> This does not look like that. > > I'm not sure why the script did that, but I manually reverted these lines to > what it was before the script was run. Now you definitely have tabs - skara's jcheck is reporting them - for example Check failure on line 2369 in src/java.desktop/share/native/liblcms/cmscgats.c openjdk / jcheck Whitespace error Column 0: tab Column 1: tab - PR: https://git.openjdk.org/jdk/pull/11000
Re: RFR: 8296083: javax/swing/JTree/6263446/bug6263446.java fails intermittently on a VM [v2]
On Mon, 14 Nov 2022 06:56:20 GMT, Prasanta Sadhukhan wrote: >> Test intermittently fails in VM citing "Tree is not editing". Seems to be >> problem with mouse clicks not getting registered properly.. >> Similar test test/jdk/javax/swing/JTable/6263446/bug6263446.java is not >> affected, so made the test similar to it by using same safeguard using >> Robot.waitForIdle() and modifying clickpoint to tree cell midpoint. >> >> Several iterations of the test pass in the OCI VM and all other physical >> platforms (link in JBS) > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Test update So it now passes 100% of the time on the OCI systems ? - PR: https://git.openjdk.org/jdk/pull/11057
Re: RFR: 8293862: javax/swing/JFileChooser/8046391/bug8046391.java failed with 'Cannot invoke "java.awt.Image.getWidth(java.awt.image.ImageObserver)" because "retVal" is null'
On Fri, 11 Nov 2022 08:18:19 GMT, Tejesh R wrote: > Observation found when JFileChooser is instantiated in WindowsLookAndFeel > which invokes getSystemIcon() from WindowsFileChooserUI class. Could not find > the exact root cause so predicting it to be an issue with icons not loaded > where resolutionVariants map is empty in _public Image > getResolutionVariant(double width, double height) _. Hence proposing a null > check before accessing getWidth(). Fix is tested in CI system. I agree with Alexey. - PR: https://git.openjdk.org/jdk/pull/11104
Re: RFR: 8293862: javax/swing/JFileChooser/8046391/bug8046391.java failed with 'Cannot invoke "java.awt.Image.getWidth(java.awt.image.ImageObserver)" because "retVal" is null'
On Fri, 11 Nov 2022 08:18:19 GMT, Tejesh R wrote: > Observation found when JFileChooser is instantiated in WindowsLookAndFeel > which invokes getSystemIcon() from WindowsFileChooserUI class. Could not find > the exact root cause so predicting it to be an issue with icons not loaded > where resolutionVariants map is empty in _public Image > getResolutionVariant(double width, double height) _. Hence proposing a null > check before accessing getWidth(). Fix is tested in CI system. Can you verify that the MultiResolutionImage contains no variants? If it's the case, a cleaner way would be to return `null` right away. Or rather, the function that creates the `MultiResolutionIconImage` instance should be modified to return `null` rather than returning an MRI with no variants. This should never happen because if the system does not provide an icon, a default icon gets requested. By adding the null-check we don't resolve the real problem but pretend it does not exist. - PR: https://git.openjdk.org/jdk/pull/11104
Re: RFR: 8157173: [macosx] java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java fails [v2]
On Mon, 14 Nov 2022 18:39:46 GMT, Phil Race wrote: > So - apart from the last change to use volatile - you found nothing > functionally wrong with the test ? There's so much clean up I can't be sure > I'm not missing some functional change you made. If there was nothing wrong > with it, why was it leaving windows open ? Next time I'll put clean up into different commit. Regarding the test stabilization and functional changes: * wrapped keyRelease, frame.dispose calls with finally * increased delay after frame appearance * increased AutoDelay - PR: https://git.openjdk.org/jdk/pull/11128
Integrated: 8286624: Regression Test CoordinateTruncationBug.java fails on OL8.3
On Sat, 12 Nov 2022 20:29:39 GMT, Phil Race wrote: > This was a closed test - simply because it hadn't been reviewed for moving to > open > so I am doing it now whilst fixing a problem that was observed on the OL 8.x > (and > by implication an RHEL 8.x) desktop due to the screen menu bar rendering over > one > line of the undecorated window. > That was fixed by changing the location to use setLocationRelativeTo(null) vs > the previous implied (0,0) This pull request has now been integrated. Changeset: c71d87e5 Author:Phil Race URL: https://git.openjdk.org/jdk/commit/c71d87e54ca0c0173583bed978e06c7faa0fa283 Stats: 271 lines in 1 file changed: 271 ins; 0 del; 0 mod 8286624: Regression Test CoordinateTruncationBug.java fails on OL8.3 Reviewed-by: azvegint, kizune - PR: https://git.openjdk.org/jdk/pull/11120
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v6]
> Hi, > > May I have this update reviewed? > > The sprintf is deprecated in Xcode 14 because of security concerns, and the > use of it causing building failure. The build could pass if warnings are > disabled for codes that use sprintf method. For the long run, the sprintf > could be replaced with snprintf. This patch is trying to check if snprintf > could be used. > > Thanks, > Xuelei Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: delete swp file - Changes: - all: https://git.openjdk.org/jdk/pull/5/files - new: https://git.openjdk.org/jdk/pull/5/files/32e18955..ca4ddcc4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=5&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=5&range=04-05 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/5.diff Fetch: git fetch https://git.openjdk.org/jdk pull/5/head:pull/5 PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8296496: Overzealous check in sizecalc.h prevents large memory allocation [v3]
On Mon, 14 Nov 2022 19:27:44 GMT, Sergey Bylokhov wrote: >> src/java.desktop/share/native/common/awt/utility/sizecalc.h line 94: >> >>> 92: >>> 93: #define SAFE_SIZE_NEW_ARRAY2(type, n, m) \ >>> 94: (IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_MUL(sizeof(type), (n) * >>> (m)) ? \ >> >> Why we do not cast it here: (n) * (m)? > > Filed https://bugs.openjdk.org/browse/JDK-8296957 You're right, it must be cast here too, I have missed it. Would you mind submitting a bug? - PR: https://git.openjdk.org/jdk/pull/11030
Re: RFR: 8296496: Overzealous check in sizecalc.h prevents large memory allocation [v3]
On Mon, 14 Nov 2022 19:20:25 GMT, Sergey Bylokhov wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fixed casting in SAFE_SIZE_NEW_ARRAY2 > > src/java.desktop/share/native/common/awt/utility/sizecalc.h line 94: > >> 92: >> 93: #define SAFE_SIZE_NEW_ARRAY2(type, n, m) \ >> 94: (IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_MUL(sizeof(type), (n) * >> (m)) ? \ > > Why we do not cast it here: (n) * (m)? Filed https://bugs.openjdk.org/browse/JDK-8296957 - PR: https://git.openjdk.org/jdk/pull/11030
Re: RFR: 8296496: Overzealous check in sizecalc.h prevents large memory allocation [v3]
On Tue, 8 Nov 2022 23:43:23 GMT, Alexander Zuev wrote: >> Removed the additional multiplication overflow detection. >> Instead cast all the parameters to type_t just the way they are treated in >> the existing size check macro. >> This way there is no possibility to accidentally provide parameters that >> will pass the size check macro while being cast to size_t there but then due >> to the missing cast cause the wrong size passed the actual allocation >> function. >> Since this checking macro was used in couple of different places all of them >> needs to be updated in the similar way. > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed casting in SAFE_SIZE_NEW_ARRAY2 src/java.desktop/share/native/common/awt/utility/sizecalc.h line 94: > 92: > 93: #define SAFE_SIZE_NEW_ARRAY2(type, n, m) \ > 94: (IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_MUL(sizeof(type), (n) * > (m)) ? \ Why we do not cast it here: (n) * (m)? - PR: https://git.openjdk.org/jdk/pull/11030
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v4]
On Mon, 14 Nov 2022 10:21:07 GMT, Thomas Stuefe wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> include missing os head file
>
> src/hotspot/share/adlc/output_c.cpp line 2570:
>
>> 2568: int idx = inst.operand_position_format(arg_name);
>> 2569: if (strcmp(arg_name, "constanttablebase") == 0) {
>> 2570: ib += snprintf(ib, (buflen - (ib - idxbuf)), " unsigned
>> idx_%-5s = mach_constant_base_node_input(); \t// %s, \t%s\n",
>
> Use sizeof(buffer) instead of buflen?
> Also, possibly using a helper macro like this:
>
>
> #define remaining_buflen(buffer, position) (sizeof(buffer) - (position -
> buffer))
>
> would make the code a bit easier on the eye. Or, if not a macro, an inline
> helper function, that could assert also array boundaries.
Thanks for suggestion, which makes the code much easier to read.
-
PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v5]
On Mon, 14 Nov 2022 19:05:16 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> May I have this update reviewed? >> >> The sprintf is deprecated in Xcode 14 because of security concerns, and the >> use of it causing building failure. The build could pass if warnings are >> disabled for codes that use sprintf method. For the long run, the sprintf >> could be replaced with snprintf. This patch is trying to check if snprintf >> could be used. >> >> Thanks, >> Xuelei > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > use helper macro The single client change looks fine. I didn't look at the rest. - Marked as reviewed by prr (Reviewer). PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v5]
> Hi, > > May I have this update reviewed? > > The sprintf is deprecated in Xcode 14 because of security concerns, and the > use of it causing building failure. The build could pass if warnings are > disabled for codes that use sprintf method. For the long run, the sprintf > could be replaced with snprintf. This patch is trying to check if snprintf > could be used. > > Thanks, > Xuelei Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: use helper macro - Changes: - all: https://git.openjdk.org/jdk/pull/5/files - new: https://git.openjdk.org/jdk/pull/5/files/128bc806..32e18955 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=5&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=5&range=03-04 Stats: 10 lines in 3 files changed: 4 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/5.diff Fetch: git fetch https://git.openjdk.org/jdk pull/5/head:pull/5 PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8295369: Update LCMS to 2.14 [v3]
On Thu, 10 Nov 2022 23:28:13 GMT, Phil Race wrote: >> src/java.desktop/share/native/liblcms/cmscgats.c line 2389: >> >>> 2387: >>> 2388: SetData(it8, i, idField, Buffer); >>> 2389: } >> >> Are these shifts caused by our script or that is the change in the upstream? > > These look to me like tabs have not been replaced by spaces.. > I saw a lot of "shifts" and I spot-checked some of them but they looked like > ones where upstream had readily changed indentation. > This does not look like that. I'm not sure why the script did that, but I manually reverted these lines to what it was before the script was run. - PR: https://git.openjdk.org/jdk/pull/11000
Re: RFR: 8295369: Update LCMS to 2.14 [v4]
> Updating LCMS to newest release Alisen Chung has updated the pull request incrementally with one additional commit since the last revision: fixed indentation problem in cmscgats.c - Changes: - all: https://git.openjdk.org/jdk/pull/11000/files - new: https://git.openjdk.org/jdk/pull/11000/files/8b88afb6..5141ed55 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11000&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11000&range=02-03 Stats: 38 lines in 2 files changed: 0 ins; 29 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/11000.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11000/head:pull/11000 PR: https://git.openjdk.org/jdk/pull/11000
Re: RFR: 8286624: Regression Test CoordinateTruncationBug.java fails on OL8.3 [v2]
On Mon, 14 Nov 2022 18:26:21 GMT, Phil Race wrote: >> This was a closed test - simply because it hadn't been reviewed for moving >> to open >> so I am doing it now whilst fixing a problem that was observed on the OL 8.x >> (and >> by implication an RHEL 8.x) desktop due to the screen menu bar rendering >> over one >> line of the undecorated window. >> That was fixed by changing the location to use setLocationRelativeTo(null) vs >> the previous implied (0,0) > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8286624 Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11120
Re: RFR: 8157173: [macosx] java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java fails [v2]
On Mon, 14 Nov 2022 14:38:38 GMT, Alexander Zvegintsev wrote: >> This test currently passing as it is on all platforms, but it was >> problemlisted with ["Problem List some tests that leave windows open on the >> desktop" >> reason](https://github.com/openjdk/jdk/commit/fd8df3adf13f5df8f5f95482b8e0a70bc519f39f) >> >> So this test is cleaned up and stabilized. >> >> CI testing looks good. > > Alexander Zvegintsev has updated the pull request incrementally with one > additional commit since the last revision: > > Addressing review comments So - apart from the last change to use volatile - you found nothing functionally wrong with the test ? There's so much clean up I can't be sure I'm not missing some functional change you made. If there was nothing wrong with it, why was it leaving windows open ? - PR: https://git.openjdk.org/jdk/pull/11128
Re: RFR: 8286624: Regression Test CoordinateTruncationBug.java fails on OL8.3 [v2]
> This was a closed test - simply because it hadn't been reviewed for moving to > open > so I am doing it now whilst fixing a problem that was observed on the OL 8.x > (and > by implication an RHEL 8.x) desktop due to the screen menu bar rendering over > one > line of the undecorated window. > That was fixed by changing the location to use setLocationRelativeTo(null) vs > the previous implied (0,0) Phil Race has updated the pull request incrementally with one additional commit since the last revision: 8286624 - Changes: - all: https://git.openjdk.org/jdk/pull/11120/files - new: https://git.openjdk.org/jdk/pull/11120/files/41753a1a..ef7287be Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11120&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11120&range=00-01 Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11120.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11120/head:pull/11120 PR: https://git.openjdk.org/jdk/pull/11120
Re: RFR: 8296222: SwingEventMonitor - installListeners(Component , int ) - CELLEDITOR - bug
On Thu, 10 Nov 2022 08:01:38 GMT, Abhishek Kumar wrote: > It seems there was a typo inside function `installListeners(Component, int)` > and `removeListeners(Component)` on case EventID.CELLEDITOR. > > Changed the string from "getCellEditorMethod" to "getCellEditor" in > `protected void installListeners(Component c, int eventID)` and `protected > void removeListeners(Component c)` methods. > > Didn't add any test case. Looks good - Marked as reviewed by kizune (Reviewer). PR: https://git.openjdk.org/jdk/pull/11082
Re: RFR: 8295707: Create a regression test for JDK-7184401 [v2]
On Mon, 7 Nov 2022 07:28:02 GMT, Srinivas Mandalika wrote: >> 8295707: Create a regression test for JDK-7184401 >> >> JDK-7184401 - JDk7u6 : Missing main menu bar in Netbeans after fix for >> 7162144 >> Above bug got introduced due to a fix for >> [JDK-7162144](https://bugs.openjdk.java.net/browse/JDK-7162144). >> The issue was observed on the netbeans UI. >> The test below recreates a standalone test to mimic the failure reported in >> Netbeans in [JDK-7184401](https://bugs.openjdk.java.net/browse/JDK-7184401) >> and verifies that it is working as expected after it got fixed via >> [JDK-7189350](https://bugs.openjdk.java.net/browse/JDK-7189350)) >> >> The Test attempts to reproduce specific behavior of NetBeans at the certain >> toolbar creation stage. Widgets are created on EDT; Another code posts some >> events to them on EDT; From another thread some code calls explicitly >> edt.interrupt(). >> Before this got fixed, events from a second code got lost. >> >> This review is for migrating tests from a closed test suite to open. >> Testing: >> 1.Tested the code on jdk7u6 to reproduce the issue - the UI hangs when run >> on this build. >> 2. Tested the code on jdk7u361 b01 to validate the fix - the test passed. >> 3.Mach5 Testing(40 times per platform) in macos x64, linux x64 and windows >> x64 - the .results are clean > > Srinivas Mandalika has updated the pull request incrementally with one > additional commit since the last revision: > > Fixed Review Comments: Removed redundant code LGTM - PR: https://git.openjdk.org/jdk/pull/10784
Re: RFR: 8296632: Write a test to verify the content change of TextArea sends TextEvent. [v4]
On Fri, 11 Nov 2022 08:20:55 GMT, ravi gupta wrote: >> This testcase Verify the content changes of a TextArea for the following >> assertions. >> >> a. TextListener get invoked when the content of a TextArea gets changed. >> b. TextListener not get invoked during text selection or when Special keys >> such as Function Keys are pressed. >> >> >> Testing: >> Tested using Mach5(20 times per platform) in macos,linux and windows and got >> all pass. > > ravi gupta has updated the pull request incrementally with one additional > commit since the last revision: > > 8296632: Review fixes. LGTM - PR: https://git.openjdk.org/jdk/pull/11052
Integrated: 8164464: Consistent failure of java/awt/dnd/MissingEventsOnModalDialog/MissingEventsOnModalDialogTest.java
On Mon, 14 Nov 2022 00:23:09 GMT, Alexander Zvegintsev wrote: > This is an old issue, which is no longer reproducible. > The test is slightly stabilized for slow machines. > > CI testing looks good. This pull request has now been integrated. Changeset: b0edfc11 Author:Alexander Zvegintsev URL: https://git.openjdk.org/jdk/commit/b0edfc1159b160eb329a066dc2805c22937a5da8 Stats: 24 lines in 2 files changed: 7 ins; 10 del; 7 mod 8164464: Consistent failure of java/awt/dnd/MissingEventsOnModalDialog/MissingEventsOnModalDialogTest.java Reviewed-by: jdv, serb - PR: https://git.openjdk.org/jdk/pull/11129
Re: RFR: 8157173: [macosx] java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java fails [v2]
On Mon, 14 Nov 2022 08:08:53 GMT, Sergey Bylokhov wrote: >> Alexander Zvegintsev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Addressing review comments > > test/jdk/java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java line 159: > >> 157: >> 158: robot.mouseMove( >> 159: (int) frame.getLocationOnScreen().getX() + > > Looks like the frame is set on one thread and accessed on another. Updated. - PR: https://git.openjdk.org/jdk/pull/11128
Re: RFR: 8157173: [macosx] java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java fails [v2]
> This test currently passing as it is on all platforms, but it was > problemlisted with ["Problem List some tests that leave windows open on the > desktop" > reason](https://github.com/openjdk/jdk/commit/fd8df3adf13f5df8f5f95482b8e0a70bc519f39f) > > So this test is cleaned up and stabilized. > > CI testing looks good. Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision: Addressing review comments - Changes: - all: https://git.openjdk.org/jdk/pull/11128/files - new: https://git.openjdk.org/jdk/pull/11128/files/8065bbb2..b906399e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11128&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11128&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/11128.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11128/head:pull/11128 PR: https://git.openjdk.org/jdk/pull/11128
Re: RFR: 8271846 a11y API lacks setSelectedIndex method [v4]
On Fri, 11 Nov 2022 21:40:40 GMT, Sergey Bylokhov wrote: >> We tried to do this, it leads to an endless repetition of the selected line >> until it freezes. > > Probably because of the endless notifications loop? implementation of > addSelectionInterval looks similar to setSelectionInterval. In some cases the > addSelectionInterval just call setSelectionInterval. It looks like it is. Please formulate the question differently. - PR: https://git.openjdk.org/jdk/pull/8578
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v4]
On Mon, 14 Nov 2022 05:32:20 GMT, Xue-Lei Andrew Fan wrote:
>> Hi,
>>
>> May I have this update reviewed?
>>
>> The sprintf is deprecated in Xcode 14 because of security concerns, and the
>> use of it causing building failure. The build could pass if warnings are
>> disabled for codes that use sprintf method. For the long run, the sprintf
>> could be replaced with snprintf. This patch is trying to check if snprintf
>> could be used.
>>
>> Thanks,
>> Xuelei
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> include missing os head file
src/hotspot/share/adlc/output_c.cpp line 2570:
> 2568: int idx = inst.operand_position_format(arg_name);
> 2569: if (strcmp(arg_name, "constanttablebase") == 0) {
> 2570: ib += snprintf(ib, (buflen - (ib - idxbuf)), " unsigned idx_%-5s
> = mach_constant_base_node_input(); \t// %s, \t%s\n",
Use sizeof(buffer) instead of buflen?
Also, possibly using a helper macro like this:
#define remaining_buflen(buffer, position) (sizeof(buffer) - (position -
buffer))
would make the code a bit easier on the eye. Or, if not a macro, an inline
helper function, that could assert also array boundaries.
-
PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8296812: sprintf is deprecated in Xcode 14 [v4]
On Mon, 14 Nov 2022 05:32:20 GMT, Xue-Lei Andrew Fan wrote: >> Hi, >> >> May I have this update reviewed? >> >> The sprintf is deprecated in Xcode 14 because of security concerns, and the >> use of it causing building failure. The build could pass if warnings are >> disabled for codes that use sprintf method. For the long run, the sprintf >> could be replaced with snprintf. This patch is trying to check if snprintf >> could be used. >> >> Thanks, >> Xuelei > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > include missing os head file Kim said: > As a general note, as a reviewer my preference is against non-trivial and > persnickety code changes that are scattered all over the code base. For > something like this I'd prefer multiple more bite-sized changes that were > dealing with specific uses. I doubt everyone agrees with me though. There's a lot of wisdom in what you say. It's far too easy to mess things up when doing cleanups for compiler warnings. Also, long patches never get enough reviewing. - PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8295707: Create a regression test for JDK-7184401 [v2]
On Mon, 7 Nov 2022 07:28:02 GMT, Srinivas Mandalika wrote: >> 8295707: Create a regression test for JDK-7184401 >> >> JDK-7184401 - JDk7u6 : Missing main menu bar in Netbeans after fix for >> 7162144 >> Above bug got introduced due to a fix for >> [JDK-7162144](https://bugs.openjdk.java.net/browse/JDK-7162144). >> The issue was observed on the netbeans UI. >> The test below recreates a standalone test to mimic the failure reported in >> Netbeans in [JDK-7184401](https://bugs.openjdk.java.net/browse/JDK-7184401) >> and verifies that it is working as expected after it got fixed via >> [JDK-7189350](https://bugs.openjdk.java.net/browse/JDK-7189350)) >> >> The Test attempts to reproduce specific behavior of NetBeans at the certain >> toolbar creation stage. Widgets are created on EDT; Another code posts some >> events to them on EDT; From another thread some code calls explicitly >> edt.interrupt(). >> Before this got fixed, events from a second code got lost. >> >> This review is for migrating tests from a closed test suite to open. >> Testing: >> 1.Tested the code on jdk7u6 to reproduce the issue - the UI hangs when run >> on this build. >> 2. Tested the code on jdk7u361 b01 to validate the fix - the test passed. >> 3.Mach5 Testing(40 times per platform) in macos x64, linux x64 and windows >> x64 - the .results are clean > > Srinivas Mandalika has updated the pull request incrementally with one > additional commit since the last revision: > > Fixed Review Comments: Removed redundant code Any volunteers for reviewing this PR ? - PR: https://git.openjdk.org/jdk/pull/10784
Re: RFR: 8157173: [macosx] java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java fails
On Mon, 14 Nov 2022 00:12:00 GMT, Alexander Zvegintsev wrote: > This test currently passing as it is on all platforms, but it was > problemlisted with ["Problem List some tests that leave windows open on the > desktop" > reason](https://github.com/openjdk/jdk/commit/fd8df3adf13f5df8f5f95482b8e0a70bc519f39f) > > So this test is cleaned up and stabilized. > > CI testing looks good. test/jdk/java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java line 159: > 157: > 158: robot.mouseMove( > 159: (int) frame.getLocationOnScreen().getX() + Looks like the frame is set on one thread and accessed on another. - PR: https://git.openjdk.org/jdk/pull/11128
Re: RFR: 8164464: Consistent failure of java/awt/dnd/MissingEventsOnModalDialog/MissingEventsOnModalDialogTest.java
On Mon, 14 Nov 2022 00:23:09 GMT, Alexander Zvegintsev wrote: > This is an old issue, which is no longer reproducible. > The test is slightly stabilized for slow machines. > > CI testing looks good. Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11129
Re: RFR: 8296812: sprintf is deprecated in Xcode 14
On Sun, 13 Nov 2022 22:55:52 GMT, Xue-Lei Andrew Fan wrote: > Please don't add uses of `jio_snprintf` or `::snprintf` to hotspot. Use > `os::snprintf`. I did not know this was our policy now. Sorry for giving the wrong advice. Maybe we should add this to the hotspot style guide since I'm probably not the only one not knowing this. > > Regarding `jio_snprintf`, see https://bugs.openjdk.org/browse/JDK-8198918. > Regarding `os::snprintf` and `os::vsnprintf`, see > https://bugs.openjdk.org/browse/JDK-8285506. > > I think the only reason we haven't marked `::sprintf` and `::snprintf` > forbidden (FORBID_C_FUNCTION) is there are a lot of uses, and nobody has > gotten around to dealing with it. `::snprintf` in the list of candidates for > https://bugs.openjdk.org/browse/JDK-8214976, some of which have already been > marked. But I don't see new bugs for the as-yet unmarked ones. > > As a general note, as a reviewer my preference is against non-trivial and > persnickety code changes that are scattered all over the code base. For > something like this I'd prefer multiple more bite-sized changes that were > dealing with specific uses. I doubt everyone agrees with me though. I agree with you. Makes backporting a bit easier too. - PR: https://git.openjdk.org/jdk/pull/5
Re: RFR: 8296905: Replace the native LCMS#getProfileID() method with the accessor
On Sat, 12 Nov 2022 09:18:57 GMT, Andrey Turbanov wrote:
>> The native method used to access the private method in the `ICC_Profile`
>> class is replaced by the accessor.
>
> src/java.desktop/share/classes/sun/awt/AWTAccessor.java line 891:
>
>> 889: */
>> 890: public static ICC_ProfileAccessor getICC_ProfileAccessor() {
>> 891: if (iccProfileAccessor == null) {
>
> For `SharedSecrets` in java.base code was updated to use single read (to
> avoid concurrency problems) - see
> [JDK-8259021](https://bugs.openjdk.org/browse/JDK-8259021)
> Shouldn't we use the same patter here?
Yes, you are right, let me fix that first.
-
PR: https://git.openjdk.org/jdk/pull/6
