Fixed memory management issue while closing native Win32 window

[nicorac]

Description

Intercepting window close in JS leads to wrong memory management when native window is closed (possibly invalid window handle).
I've seen it in debug-builds within VS2019, but it could possibly happen on release.

These are the steps to describe what happens:

• native window X button is clicked by the user
• JS receives the windowClose event, does its own tasks and then calls app.exit()
• app.exit() calls window::_close(code);

window::_close() function contains the misbehavior:

// window.cpp
void _close(int exitCode) {
    if(nativeWindow) {
        nativeWindow->terminate(exitCode);  // <-- THIS CALL IS NON-BLOCKING!
        delete nativeWindow;
    }
}

// webview.h
void terminate(int exitCode = 0) {
  processExitCode = exitCode;
  dispatch([=]() {
      DestroyWindow(m_window);
  });
}
void dispatch(dispatch_fn_t f) {
  PostThreadMessage(m_main_thread, WM_APP, 0, (LPARAM) new dispatch_fn_t(f));  // <-- NON BLOCKING!
}

The marked line contains "non blocking" code, since it only calls PostThreadMessage(), so it returns immediately.
The delete nativeWindow; line is so executed immediately (before the Window message posted by PostThreadMessage has been handled) and clears nativeWindow instance data.

Once the message is handled, DestroyWindow(m_window) is executed but the value of m_window field could be invalid because its owning class was already deleted!

It actually works just because... luckily no other code had ever reused/overwritten the same memory block previously used by nativeWindow..

Changes proposed

We need to ensure that terminate() won't return (and so delete won't be called) till native window has been destroyed.
To do this I've used an event to synchronize the two threads:

  void terminate(int exitCode = 0) {

    // event to wait for window close completion
    auto evtWindowClosed = CreateEvent(
        NULL,               // default security attributes
        TRUE,               // manual-reset event
        FALSE,              // initial state is nonsignaled
        TEXT("WindowClosedEvent")  // object name
    );
    processExitCode = exitCode;

    dispatch([=]() {
        DestroyWindow(m_window);
        SetEvent(evtWindowClosed);
    });

    // wait for dispatch() to complete
    WaitForSingleObject(evtWindowClosed, 10000);
    CloseHandle(evtWindowClosed);

  }

[nicorac]

Still wondering why Windows tests fail; they work correctly on my side (Win10-22H2).
The failure is completely unrelated with my changes...

[shalithasuranga]

Hi.. this looks great :) Thanks so much for sending this PR with a detailed explanation about window events