Improve E2E tests performance

[sebastienros]

Fixes #56004

Without this PR the chrome instances are referenced indefinitely until the test fixture is disposed.
The PR replaces GUIDs that were used to get a fresh browser by null to note that the Browser should not be reused instead. And then when the test class is disposed the Browser is disposed. This limits the number of dangling chrome instances which accumulate overtime and slow down the machine (each instance takes 10% of cpu continuously on my machine, even after the test)

[sebastienros]

ThreadingAppTest is failing (a quarantined one, so I will investigate).

Speed: so far it's not faster, like 10 minutes slower than a good run. Another aspect to take into account is the stability that the PR adds for local runs (dangling chrome instances), but this may be something that could be ignored if these are never run locally.

[javiercn]

Another aspect to take into account is the stability that the PR adds for local runs (dangling chrome instances), but this may be something that could be ignored if these are never run locally.

These tests are definitely run locally.

It's been a while and I don't remember the exact details of how this worked, but we passed in a GUID so that chrome created a separate profile because it affected some tests that we had.

I've seen the chrome instances problem you are referring to (and I've usually just killed the instances after the test run). Is there a way we can kill the instances "earlier" and ensure we maintain the isolation with the separate profiles?

[sebastienros]

Is there a way we can kill the instances "earlier" and ensure we maintain the isolation with the separate profiles?

This PR ;)

It provides the same behavior as with the Guid, new instances, but by using a guid (instead of null and this PR changes) the instances were cached and only released when all the tests were done.

[SteveSandersonMS]

Thanks for proposing this, @sebastienros.

In terms of priorities:

• For local runs, I don't think we need to prioritize speed. If a full run takes 10 mins longer with this PR but is more reliable in some sense, that's a win for local runs. People rarely if ever do a complete run of all the E2E tests locally, since they nearly always just run the ones related to their current work area (at least that's true for me - curious if anyone works differently).
• For CI runs, overall speed is more of a concern since it already takes ~50min and if that were to eventually double, it would risk sometimes being the critical path. Adding 10 mins is a significant chunk. Is there reason to believe this PR improves reliablility in CI specifically?

It's not an easy call to make because we probably don't want to run in two different modes (local vs CI) as then we'll get more CI-only failures.

[javiercn]

@sebastienros is this still something we want to push for, or should we close it.

[sebastienros]

@javiercn I think the changes are simple, so as long as the tests are still passing I don't see why we could not apply them if they improve the time it takes to run tests. I won't spend the time on syncing it with main though if others prefer not to touch this part of the code.