<Swing Dev> [11][JDK-8176512][TEST_BUG] add a minimal delay to java/awt/Paint/bug8024864.java

[Krishna Addepalli]

"Because it will flush the native queue a few times, then flush EDT, and then it will wait 100 ms(minimum)."
In my opinion, flushing will only push the events to the queue, but will not guarantee that the last event pushed has been executed on EDT. Also, the wait for 100ms seems arbitrary. I mean, it cannot be universally reasoned that it will work on all systems.

"If it is incorrect then why do you need to wait when OPEN_EVENT will be dispatched at all, since you mention that "super.show()" call which ultimately leads to AwtComponent::_Show function". So AwtComponent::_Show will be called before the end of Window.show()."
This is because EDT and main thread are different. EDT can get swapped by Main thread right after the show() call, but before EDT had a chance to draw something on the screen. Although this happens rarely, it cannot be ruled out.

"My point is that the fix and previous implementation is mostly identical. And if old implementation does not work then it is a bug somewhere in the robot."
I feel this problem arises because of the complex interaction between Main thread and EDT, which can be hard to debug. Anyway, I agree that we need to investigate this, but I guess we should fix it in a separate bug.

Thanks,
Krishna

-----Original Message-----
From: Sergey Bylokhov 
Sent: Saturday, February 3, 2018 12:48 PM
To: Krishna Addepalli <krishna.addepalli at oracle.com>; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [11][JDK-8176512][TEST_BUG] add a minimal delay to java/awt/Paint/bug8024864.java

On 02/02/2018 04:19, Krishna Addepalli wrote:
> Hi Sergey,
> 
> "It does not guarantee but this wait + waitForIdle() should be enough."
> If it is not guaranteed, how can we say it is enough?

Because it will flush the native queue a few times, then flush EDT, and then it will wait 100 ms(minimum).

> "Take a look to the code which generates the OPEN_EVENT used in the fix(see Window.show()).
> It is posted without relation to the native peer:"
> 
> This is incorrect, since before posting the OPEN_EVENT, there is a "super.show()" call which ultimately leads to AwtComponent::_Show function in awt_Component.cpp, which actually posts a message to show the window.

If it is incorrect then why do you need to wait when OPEN_EVENT will be dispatched at all, since you mention that "super.show()" call which ultimately leads to AwtComponent::_Show function". So AwtComponent::_Show will be called before the end of Window.show().

The approach which you used in the fix(wait for OPEN_EVENT) is identical to implementation of waitForIdle().
  - Both will post events(OPEN_EVENT/dummy) after Window.show();
  - Both will wait when it will be dispatched.

But only waitForIdle() will flush the native, and can do this a few time(EDT/native/EDT/native etc).

> Also, before OPEN_EVENT, ComponentShown event is also posted(if there are active listeners to the component) which also seems to be coming from native side.
> So, I think it is more appropriate to rely on OPEN_EVENT or even ComponentShown event, rather than having to use wait.

My point is that the fix and previous implementation is mostly identical. And if old implementation does not work then it is a bug somewhere in the robot. See example below which compare both cases:

> ==========
> 1) EDT: Window.setVisible()->Window.show()->Window.postWindowEvent()->
> barrier.await();
> 2) Main: blocked by barrier.await();
> 
> So your code waits when the setVisible() will complete and some 
> additional event
> (OPEN_EVENT) will be dispatched by EDT.
> 
> ==========
> The usage of waitForIdle() should produce similar result:
> 
> 1) EDT: Window.setVisible()->Window.show();
> 3) Main: post dummy event to EDT->waitLock.notifyAll();
> 2) Main: blocked in SunToolkit.waitForIdle() by waitLock.wait();
> 
> The old code waits when the setVisible() will complete and some 
> additional event
> (dummy) will be dispatched by EDT. But it also flush the native queue.
> 
> So it is still unclear why waitForIdle does not work.


--
Best regards, Sergey.