Hot Restart should dispose all previous Platform Views (macOS)

[PaulAllanSturm]

When using Platform Views on macOS, performing a Hot Restart throws an exception with message "trying to create an already created view". This is because the old Platform Views are not cleaned up. So, here we dispose of the old Platform Views as part of the Hot Restart process.

Fixes issue: #110381

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, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

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

[chinmaygarde]

cc @cbracken @jonahwilliams

[jonahwilliams]

I think this should be sufficient but adding @knopp to verify.

[knopp]

I'm not sure about the test. Wouldn't it be easier to create say 2 platform views, call reset and then try to get the views by Id to see if the result is nil for both of them, instead of testing failure when creating view with identical Id?

[PaulAllanSturm]

I'm not sure about the test. Wouldn't it be easier to create say 2 platform views, call reset and then try to get the views by Id to see if the result is nil for both of them, instead of testing failure when creating view with identical Id?

I'm happy to do it either way, but the way it is now is intended to match what was happening with the original bug report. I.e., a platform view was created, a Hot Restart was done, and then another platform view with the same id gets created. The test is basically saying that without calling "reset()", you get the exception as described in the original bug report, and then with the "reset()", the problem is resolved.

[knopp]

I'm not sure about the test. Wouldn't it be easier to create say 2 platform views, call reset and then try to get the views by Id to see if the result is nil for both of them, instead of testing failure when creating view with identical Id?

I'm happy to do it either way, but the way it is now is intended to match what was happening with the original bug report. I.e., a platform view was created, a Hot Restart was done, and then another platform view with the same id gets created. The test is basically saying that without calling "reset()", you get the exception as described in the original bug report, and then with the "reset()", the problem is resolved.

In theory somebody can break the reset and change view creation to overwrite existing view (or just fail silently), the test will still pass but the behavior will be wrong. I don't feel too strongly about this either though.

[PaulAllanSturm]

In theory somebody can break the reset and change view creation to overwrite existing view (or just fail silently), the test will still pass but the behavior will be wrong. I don't feel too strongly about this either though.

I'll make the change. It makes more sense as a unit test to consider specifically the contract promised by the code under test, rather than trying to play out a scenario from some remote bug report.

[PaulAllanSturm]

I've updated the test.

[knopp]

LGTM!

[jonahwilliams]

LGTM