Implemented threadsafe platform channel replies on windows

[gaaclarke]

This makes it so platform message handlers on Windows can invoke their replies from any thread. The reason this is possible is:

• There is little actual work that is happening on the thread invoking the reply. It has mostly to do with manipulating resources uniquely assigned to the message and thread hopping to the ui thread.
• We've introduced a lock that makes sure the dispatcher used to handle messages isn't deleted while the background thread is invoking the reply.
• We decoupled the FlutterDesktopMessenger from the Engine so that the FlutterDesktopMessenger can outlive the Engine but be in an invalid state. (This surfaced a bug that already existed in the implementation since a reply could always have outlasted an engine).

issue: flutter/flutter#93945

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[gaaclarke]

No existing tests for the client_wrapper actually hit this code. In terms of code coverage it isn't strictly needed since I have coverage in the windows unit tests.

[gaaclarke]

This is an existing error in a test that landed in a different PR. I'm fixing it here.

[loic-sharma]

LGTM but consider getting at least one other approval as my knowledge here is shaky :)

[gaaclarke]

friendly ping @stuartmorgan or @cbracken

[stuartmorgan]

Optional nit: this block is entirely local, so could be done before acquiring the lock instead.

[gaaclarke]

IsAvailable has to happen inside the safety of the lock.

[stuartmorgan]

My comment was on this code:

    if (!response_handle) {
      std::cerr << "Error: Response can be set only once. Ignoring "
                   "duplicate response."
                << std::endl;
      return;
    }

which doesn't have any calls to IsAvailable.

[gaaclarke]

friendly ping @stuartmorgan

[stuartmorgan]

Why are these part of the class rather than in flutter_window.cc with the others?

The intended model here is that there's a coherent C++ interface that all internal logic is developed against, and then flutter_windows.* is a minimal layer over that for the sole purpose of creating a DLL interface (which has to be C). These opaque handles are purely a function of the C API layer, which is why the other functions are in flutter_window.cc; the C++ classes shouldn't know about the C layer when we can avoid it since that's a layering inversion. (In pattern terms I would view the C layer as basically a Facade, if that helps.)

[gaaclarke]

They were put there as the most convenient location. They can't live in flutter_window.cc because they are also used in flutter_windows_engine.cc because of BinaryMessengerImpl and IncomingMessageDispatcher using FlutterDesktopMessengerRef. I considered changing those interfaces but decided to do the least amount of change possible.

[stuartmorgan]

Ah, right. Could you put a TODO on these then to move them to flutter_windows.cc functions and remove the internal usage of FlutterDesktopMessengerRef?

[gaaclarke]

done, added issue: flutter/flutter#115021

[stuartmorgan]

You should be able to completely remove this. The pointers that have already been fully converted to classes use the opaque struct design, where the struct is forward declared but never defined, because it's only used to create a typed pointer.

[gaaclarke]

done