[Windows] Allow adding/removing views

[loic-sharma]

This enables the Windows embedder to render to multiple views using the new FlutterEngineAddView and FlutterEngineRemoveView embedder APIs. See: https://flutter.dev/go/multi-view-embedder-apis
 
Prepares for flutter/flutter#144810
Part of flutter/flutter#142845

Sync over async

Windows expects synchronous operations: windows are created, resized, and destroyed synchronously. However, Flutter native is asynchronous due to its threading model.

This change blocks the platform thread when a non-implicit view is added or removed. See: https://flutter.dev/go/multi-view-sync-over-async

Synchronization

The embedder and engine have separate view states that they synchronize asynchronously. The engine can present a view on the raster thread while the embedder is destroying that same view on the platform thread. This change introduces a mutex to protect against this:

1. The platform thread acquires a shared lock whenever it needs to access the view.
2. The platform thread acquires an exclusive lock to add a view to the embedder, before it notifies the engine of the view
3. The platform thread acquires an exclusive lock to remove a view from the embedder, after the engine has acknowledged the view's removal but before the embedder destroys the view.
4. The raster thread acquires a shared lock to present to a view. This lock is held for the entirety of the present operation, thereby blocking the platform thread from destroying the view.

The implicit view is an important corner case. The framework/engine believe the implicit view always exists, even if the app is in headless mode. The embedder does not notify the engine when it destroys the implicit view. In other words, the embedder must safely ignore presents to the implicit view when it does not exist.

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 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.

[flutter-dashboard]

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

[dkwingsmt]

Suggested change

	// Whether this view is the implicit view.
	//
	// The implicit view is a special view for backwards compatibility.
	// The engine assumes it can always render to this view, even if the app has
	// destroyed the window for this view. The embedder must ignore presents to
	// this view after it is destroyed.
	//
	// The implicit view is the only view that can be created before the
	// engine is launched.
	// Whether this view is the implicit view.
	//
	// The implicit view is the first view created on an engine, and has some special
	// properties in order to keep backwards compatibility.
	// * The `FlutterWindowsView` for the implicit view can be created before the
	// engine is launched.
	// * Even before the `FlutterWindowsView` for the implicit view is created.
	// the Dart app thinks the implicit view exists and may render into the view
	// (with no effects).
	// * Even after the `FlutterWindowsView` for the implicit view is removed, the Dart
	// app still thinks the implicit view exists and may render into the view (with
	// no effects).

[loic-sharma]

Is there something you think is unclear or missing in the original comment? I currently prefer the original comment, but I'm happy to expand/reword anything in it.

[dkwingsmt]

Nevermind, I just realized that this entire file is not for the end user but is internal.

[dkwingsmt]

Still, I think we should add:

• The implicit view is the first view created on the engine.
• The engine assumes it can always render to this view, even before the app creates the FlutterWindowsView or after the app has destroyed the window for this view.

[loic-sharma]

Thanks for expanding. This doc is not for end users, this doc is for Windows embedder developers. This method will never be called by an end user.

I'll clarify that the embedder must ignore presents to this view before it is created.

[loic-sharma]

Updated!

[dkwingsmt]

LGTM

[loic-sharma]

@yaakovschectman @cbracken This is ready for review :)

[yaakovschectman]

Should this struct be defined within the function, and if so, should the variable be declared separately like this?

[loic-sharma]

Yup, here's prior art in the Windows embedder:

engine/shell/platform/windows/flutter_windows_engine.cc

Lines 752 to 767 in e4e2748

	struct Captures {
	fml::closure callback;
	};
	auto captures = new Captures();
	captures->callback = std::move(callback);
	if (embedder_api_.PostRenderThreadTask(
	engine_,
	[](void* opaque) {
	auto captures = reinterpret_cast<Captures*>(opaque);
	captures->callback();
	delete captures;
	},
	captures) == kSuccess) {
	return true;
	}
	delete captures;

Slight difference: for adding/removing views we can stack allocate the captures, we don't need to heap allocate. (We block until the add/remove callback is invoked, guaranteeing the captures exist until the latch is signaled)

[cbracken]

Here and elsewhere, consider zero-initialising this as:

Captures captures = {};

By default, locals are not zero-initialised so worth being on the safe side.

[loic-sharma]

Updated!

[cbracken]

lgtm with a teensy nit.