Hi :Gankra. Great investigation! Yea, it seems that there is a race condition exists between SetParent() in UI process and SetPosition() on RenderThread in GPU process. I could reproduce the problem on my one low-end Win10 PC and investigated the problem with printf_stderr(). Both on [1] and [2] cases, SetPosition() was succeeded. Further SetParent() and SetPosition() seemed to happen concurrently. When [1] happened,the sequence was like the following -(1) SetParent() called in UI process -(2) Entered WinCompositorWidget::UpdateCompositorWndSizeIfNecessary() -(3) Parent of compositor window was still initial parent window -(4) SetWindowPos() with (0, 0, 432, 165) succeeded -(5) WM_NCCALCSIZE with client rect(0, 0, 432, 165) event happened on WinCompositorWindowThread -(6) WM_NCCALCSIZE with client rect(0, 0, 1, 1) event happened on WinCompositorWindowThread -(7) After SetWindowPos(), GetClientRect() with compositor window returned rect(0, 0, 1, 1) -(8) Exited SetParent() in UI process When [2] happened,the sequence was like the following -(1) SetParent() called in UI process -(2) Entered WinCompositorWidget::UpdateCompositorWndSizeIfNecessary() -(3) Parent of compositor window was still initial parent window -(4) SetWindowPos() with (0, 0, 432, 313) succeeded -(5) WM_NCCALCSIZE with client rect(0, 0, 1, 1) event happened on WinCompositorWindowThread -(6) WM_NCCALCSIZE with client rect(-404, -221, 28, 92) event happened on WinCompositorWindowThread -(7) After SetWindowPos(), ::GetClientRect() with compositor window returned rect(0, 0, 432, 313) -(8) Exited SetParent() in UI process -(9) WM_NCCALCSIZE with client rect(-404, -221, 28, 67) event happened on WinCompositorWindowThread From the above, it seems that we could suppress the problem by "GetParent(mCompositorWnd) == mWnd" as you mentioned. And from the [2] sequence, there is a case that client rect of compositor window becomes strange by OS. Therefore removing SWP_NOMOVE seems reasonable. And It seems better to remove WinCompositorWidget::mLastCompositorWndSize usage. It assumes that a window position by SetWindowPos() does not change. But size of client rect is changed by OS at unexpected timing. And as an option, we might want to trigger WR rendering when SetParent() was completed in UI process.
Bug 1573710 Comment 30 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Hi :Gankra. Great investigation! Yea, it seems that there is a race condition exists between SetParent() in UI process and SetPosition() on RenderThread in GPU process. I could reproduce the problem on my one low-end Win10 PC and investigated the problem with printf_stderr(). Both on [1] and [2] cases, SetPosition() was succeeded. Further SetParent() and SetPosition() seemed to happen concurrently. When [1] happened,the sequence was like the following -(1) SetParent() called in UI process -(2) Entered WinCompositorWidget::UpdateCompositorWndSizeIfNecessary() -(3) Parent of compositor window was still initial parent window -(4) SetWindowPos() with (0, 0, 432, 165) succeeded -(5) WM_NCCALCSIZE with client rect(0, 0, 432, 165) event happened on WinCompositorWindowThread -(6) WM_NCCALCSIZE with client rect(0, 0, 1, 1) event happened on WinCompositorWindowThread -(7) After SetWindowPos(), GetClientRect() with compositor window returned rect(0, 0, 1, 1) -(8) Exited SetParent() in UI process When [2] happened,the sequence was like the following -(1) SetParent() called in UI process -(2) Entered WinCompositorWidget::UpdateCompositorWndSizeIfNecessary() -(3) Parent of compositor window was still initial parent window -(4) SetWindowPos() with (0, 0, 432, 313) succeeded -(5) WM_NCCALCSIZE with client rect(0, 0, 1, 1) event happened on WinCompositorWindowThread -(6) WM_NCCALCSIZE with client rect(-404, -221, 28, 92) event happened on WinCompositorWindowThread -(7) After SetWindowPos(), ::GetClientRect() with compositor window returned rect(0, 0, 432, 313) -(8) Exited SetParent() in UI process -(9) WM_NCCALCSIZE with client rect(-404, -221, 28, 67) event happened on WinCompositorWindowThread From the above, it seems that we could suppress the problem by "GetParent(mCompositorWnd) == mWnd" as you mentioned. And from the [2] sequence, there is a case that client rect of compositor window becomes strange by OS. Therefore removing SWP_NOMOVE seems reasonable. And It seems better to remove WinCompositorWidget::mLastCompositorWndSize usage. It assumes that a window position by SetWindowPos() does not change. But size of client rect is changed by OS at unexpected timing. And as an option, we might want to trigger WR rendering when SetParent() was completed in UI process. It was hard to reproduce [2] .
Hi :Gankra. Great investigation! Yea, it seems that there is a race condition exists between SetParent() in UI process and SetPosition() on RenderThread in GPU process. I could reproduce the problem on my one low-end Win10 PC and investigated the problem with printf_stderr(). Both on [1] and [2] cases, SetPosition() was succeeded. Further SetParent() and SetPosition() seemed to happen concurrently. When [1] happened,the sequence was like the following -(1) SetParent() called in UI process -(2) Entered WinCompositorWidget::UpdateCompositorWndSizeIfNecessary() -(3) Parent of compositor window was still initial parent window -(4) SetWindowPos() with (0, 0, 432, 165) succeeded -(5) WM_NCCALCSIZE with client rect(0, 0, 432, 165) event happened on WinCompositorWindowThread -(6) WM_NCCALCSIZE with client rect(0, 0, 1, 1) event happened on WinCompositorWindowThread -(7) After SetWindowPos(), GetClientRect() with compositor window returned rect(0, 0, 1, 1) -(8) Exited SetParent() in UI process When [2] happened,the sequence was like the following. It was hard to reproduce [2] . -(1) SetParent() called in UI process -(2) Entered WinCompositorWidget::UpdateCompositorWndSizeIfNecessary() -(3) Parent of compositor window was still initial parent window -(4) SetWindowPos() with (0, 0, 432, 313) succeeded -(5) WM_NCCALCSIZE with client rect(0, 0, 1, 1) event happened on WinCompositorWindowThread -(6) WM_NCCALCSIZE with client rect(-404, -221, 28, 92) event happened on WinCompositorWindowThread -(7) After SetWindowPos(), ::GetClientRect() with compositor window returned rect(0, 0, 432, 313) -(8) Exited SetParent() in UI process -(9) WM_NCCALCSIZE with client rect(-404, -221, 28, 67) event happened on WinCompositorWindowThread From the above, it seems that we could suppress the problem by "GetParent(mCompositorWnd) == mWnd" as you mentioned. And from the [2] sequence, there is a case that client rect of compositor window becomes strange by OS. Therefore removing SWP_NOMOVE seems reasonable. And It seems better to remove WinCompositorWidget::mLastCompositorWndSize usage. It assumes that a window position by SetWindowPos() does not change. But size of client rect is changed by OS at unexpected timing. And as an option, we might want to trigger WR rendering when SetParent() was completed in UI process.