Disable TransientKeepAlivePolicy in TabManagerTests. [chromium/src : main]
Alex Attar (Gerrit)
Alex Attar voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Attar
| Commit-Queue | +1 |
1 comment
Related details
- Joe Mason
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Mason (Gerrit)
Joe Mason voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joe Mason
| Code-Review | +1 |
1 comment
{performance_manager::features::kTransientKeepAlivePolicy, false}});If it's important that the process ends immediately - important enough to test - doesn't this failure mean that TransientKeepAlivePolicy is broken? It seems like if we're ok with TransientKeepAlive we should just stop testing that the process ends, since it's really not important.
Also this test is gonna break if we launch TransientKeepAlivePolicy, and we'll have to fix it then.
OTOH if we're just experimenting with TransientKeepAlivePolicy to see if it's worth pursuing, maybe it's not worth fixing the test until we're ready to consider launching it anyway, so I'll +1 this and let you decide whether to land as-is.
Related details
- Alex Attar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Attar (Gerrit)
Alex Attar added 1 comment![]( "Open in Gerrit")
{performance_manager::features::kTransientKeepAlivePolicy, false}});If it's important that the process ends immediately - important enough to test - doesn't this failure mean that TransientKeepAlivePolicy is broken? It seems like if we're ok with TransientKeepAlive we should just stop testing that the process ends, since it's really not important.
Also this test is gonna break if we launch TransientKeepAlivePolicy, and we'll have to fix it then.
OTOH if we're just experimenting with TransientKeepAlivePolicy to see if it's worth pursuing, maybe it's not worth fixing the test until we're ready to consider launching it anyway, so I'll +1 this and let you decide whether to land as-is.
Thanks! For now we are only exploring if it's worth to have this feature or not. I agree if the feature is worth enabling by default we should look at either fixing the test or removing it as the conditions for a process to terminate would become different to what it is now.
I'll leave as is for now since it's mostly experimental for now and depending on the results will adjust the tests where this feature was disabled.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Attar (Gerrit)
Alex Attar voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Disable TransientKeepAlivePolicy in TabManagerTests.
The `TransientKeepAlivePolicy` feature extends the renderer process
lifecycle before destruction, causing failures in `TabManagerTest` and
`TabManagerTestWithTwoTabs` due to conflicting expectations of immediate
process termination. This CL explicitly disables the feature in these
tests to ensure their stability.
- M chrome/browser/resource_coordinator/tab_manager_browsertest.cc
Code-Review: +1 by Joe Mason
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |