fix: update on_message_ to nullptr after window destroy so that dart gets destroy message - #185807

Use static_cast instead of reinterpret_cast for downcasting from a base class to a derived class. static_cast is safer as it provides compile-time checks that the types are related, which aligns with the Google C++ Style Guide.

    flutter::HostWindowTooltip* tooltip_window =
        static_cast<flutter::HostWindowTooltip*>(window);

Use static_cast instead of reinterpret_cast for downcasting from a base class to a derived class. static_cast is safer as it provides compile-time checks that the types are related, which aligns with the Google C++ Style Guide.

    flutter::HostWindowPopup* popup_window =
        static_cast<flutter::HostWindowPopup*>(window);

Should we log a warning if the window doesn't exist?

Sure, will do.

Should we log a warning if the window doesn't exist?

How exactly does this happen? What is the reason for not being able to to get the window for handle? Perhaps we have a race condition somewhere? I'd like to know more about the situation that causes this rather than blindly added nullchecks.

Good question, sorry for the band-aid feel. Looking at it more carefully, I see two real ways the handle ends up stale by the time it reaches GetThisFromHandle:

1. In OnDestroyWindow, we call SetParent(flutter_view_handle, nullptr) before DestroyWindow(hwnd). Once the FlutterView is reparented to null it becomes top-level, so GetAncestor(view, GA_ROOT) (used by GetTopLevelWindowHandle) starts returning the FlutterView's HWND instead of the host window's. That HWND has a different class name, so GetThisFromHandle correctly returns null. If anything on the Dart side calls updatePosition in that small window (before our WM_DESTROY handler sets _destroyed = true), we hit the crash. (So, definately this is a race condition as you think).
2. After WM_NCDESTROY runs, the HostWindow has been erased from active_windows_, the view-controller is destroyed, and RemoveView has cleared views_. From there, engine->view(view_id) returns null and GetTopLevelWindowHandle returns nullptr. So Dart can hand us a null HWND. Same issue if a plugin's window-proc delegate handles WM_DESTROY before us — Dart never learns the window is gone, but the C++ side has already torn it down.

So the null check isn't covering up a corruption — it's the legitimate case where Dart still thinks the popup is alive but the native HostWindow has already been destroyed. Happy to also tighten things on the native side if you'd prefer (e.g. clear GWLP_USERDATA in ~HostWindow) or maybe reorder destroy (not sure about this).

Also one more thing, During engine shutdown, WindowManager::OnEngineShutdown sets on_message_ = nullptr before destroying windows, so Dart never gets WM_DESTROY → _destroyed stays false. Pending Dart callbacks (frame callbacks, microtasks) running before isolate shutdown can still invoke updatePosition.

Ad 1) What dart code will call updatePosition while destroying the window? It seems to be that the only place for it would be from handling WM_DESTROY, which right now would need custom window subclass (we don't expose custom message handlers). Can you provide a realistic scenario how this would happen?

Ad 2) How exactly can plugin handle the WM_DESTROY?

Also one more thing, During engine shutdown, WindowManager::OnEngineShutdown sets on_message_ = nullptr before destroying windows, so Dart never gets WM_DESTROY → _destroyed stays false. Pending Dart callbacks (frame callbacks, microtasks) running before isolate shutdown can still invoke updatePosition.

This seems like a valid point. Would be nice to have testcase for this.

Re: Ad 1) and Ad 2) — yes, plugins absolutely can and do observe these messages. Two paths:

1. FlutterDesktopPluginRegistrarRegisterTopLevelWindowProcDelegate (flutter_windows.cc:318) is a public C ABI entry that dispatches every top-level window message — including WM_DESTROY — to plugin-registered delegates. Custom flutter_inappwebview uses this (flutter_inappwebview_windows_plugin.cpp:39).
2. comctl32 SetWindowSubclass, used by WebView2 (which flutter_inappwebview embeds) on its host window. This is the path actually triggering the crashes.

So UpdatePopupPosition is being called synchronously, on the UI thread, from inside a comctl32 subclass dispatch. This isn't a hypothetical Dart-shutdown race anymore; it's a reentrant call during normal message processing on a real plugin's host window.

Have added tests for engine shutdown race condition.

The plugins generally don't call dart code from these handlers. That is a hypothetical situation. window_toolbox does let use handle the message from dart, but this would imply that whoever handles the message is handling aWM_DESTROY, from which it tries to position the popup. During destruction. That clearly seems like a very bad idea when crash seems completely warranted.

Actually this is happening in an app already, so not so hypothetical situation. But I still feel, engine shutdown race condition is general probable case (I have added tests for that).

Can you make separate PR for the engine shutdown thing? Or remove the null checks from this one and update the PR. I don't one to fix the null without a repro but the engine shutdown seems like a pretty obvious problem.

Sure, I will update this PR