| Commit-Queue | +1 |
Hi Kelvin and Devlin!
While working on my previous CL, I noticed the TODO in drop_overlay.ts regarding a listener leak on document.documentElement. This CL addresses that by moving the listener logic to the proper lifecycle methods and utilizing an EventTracker for cleanup.
Since I don't have dry-runner permissions yet, could a reviewer please trigger the CQ Dry Run for me?
PTAL! Thanks.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Commit-Queue | +1 |
Thanks, Ehtesham! I'll take a look after Kelvin stamps, but I'll kick off a dry run now.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks, Ehtesham! I'll take a look after Kelvin stamps, but I'll kick off a dry run now.
Thanks for triggering the dry run, Devlin! Glad to see it passed the bots. I'll wait for Kelvin's review. Thanks!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hi Devlin! Just a gentle ping on this. Kelvin has stamped (+1) and the dry run is green, so this should be ready for your final look. Thanks!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hi Kelvin and Devlin, I've rebased to resolve the conflict and integrated the new fire() helper into the fix. Could you please take a look? If everything looks good, I'd appreciate a CQ dry run. Hopefully, this is ready to land! Thanks! (CC'ing dpapad as they recently refactored this file).
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
them. This resulted in a memory leak where listeners accumulated
every time the extensions page was opened.
Are you sure about this? While `extensions-drop-overlay` indeed wasn't removing the document listener, it is only added once in the DOM at [1]. So it wouldn't leak any listeners, at least not in prod code (perhaps in tests where things get attached and detached multiple times).
Closing the extensions WebUI shouldn't leave any lingering listeners or memory leaks as the entire renderer is destroyed.
I don't think the assessment above is accurate.
(although adding and removing listeners is the recommended approach anyway).
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
LGTM but do edit the commit message re: dpapad's comment
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |