On Fri, 21 Apr 2023 21:31:54 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Jeremy has updated the pull request incrementally with one additional commit >> since the last revision: >> >> 4200096: removing whitespace diffs >> >> prrace pointed out this branch introduced some pointless whitespace >> changes. >> https://github.com/openjdk/jdk/pull/13408#discussion_r1164740594 > > src/java.desktop/share/classes/sun/awt/image/OffScreenImageSource.java line > 208: > >> 206: // ImageConsumer detaches itself from this ImageProducer >> mid-production. >> 207: >> 208: if (theConsumer != null) { > > Do we need here and a few lines above save the theConsumer to the local, then > check to null, then call imageComplete? My understanding is: This ticket focuses on the use case where an ImageConsumer detaches itself from this OffScreenImageSource mid-production by calling `removeConsumer(ImageConsumer)`. So in this situation: I'd argue no, the ImageConsumer should not get an `imageComplete(int)` notification. Because it opted to remove itself. Does this answer your question, or are you considering a different use case? (Note: `addConsumer` is synchronized, so it's *usually* reasonable to assume the field `OffScreeImageSource#theConsumer` acts *mostly* like a local variable. I could contrive a new failing test case if one ImageConsumer called `addConsumer(newDifferentConsumer)` while receiving a notification, but that's a separate discussion. It's beyond the scope of this ticket, and (to my knowledge) it's a contrived edge case that nobody has ever actually complained about in the real world. And it would fail with or without the changes in this PR...) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13408#discussion_r1174219442