fix: memory leak in linkDetector

[SimonSiefke]

Fixes a memory leak in linkDetector.ts by replacing HTMLElement.onclick with dom.addDisposableListener('click').

Because of that, now the disposable store also needs to be passed from one function to another and another. But the main change is replacing HTMLElement.onclick with dom.addDisposableListener('click').

Test Results

Before

A keydown, click, mouseleave and mousemove event listener leak related to linkDetector are found:

{
  "eventListenersWithStackTrace": [
    {
      "type": "keydown",
      "description": "l=>{const c=new Li(l);(c.keyCode===3||c.keyCode===10)&&(c.preventDefault(),c.stopPropagation(),r(c.keyCode===10))}",
      "objectId": "-6136671504294265107.1.22263",
      "count": 133,
      "originalStack": ["src/vs/workbench/contrib/debug/browser/linkDetector.ts:338:19"],
      "originalName": "e"
    },
    {
      "type": "click",
      "description": "l=>{const c=Te(e).getSelection();!c||c.type===\"Range\"||(Ye?l.metaKey:l.ctrlKey)&&(l.preventDefault(),l.stopImmediatePropagation(),r(!1))}",
      "objectId": "-6136671504294265107.1.22261",
      "count": 133,
      "originalStack": ["src/vs/workbench/contrib/debug/browser/linkDetector.ts:325:18"],
      "originalName": "event"
    },
    {
      "type": "mouseleave",
      "description": "()=>e.classList.remove(\"pointer\")",
      "objectId": "-6136671504294265107.1.22259",
      "count": 133,
      "originalStack": ["src/vs/workbench/contrib/debug/browser/linkDetector.ts:324:22"],
      "originalName": null
    },
    {
      "type": "mousemove",
      "description": "l=>{e.classList.toggle(\"pointer\",Ye?l.metaKey:l.ctrlKey)}",
      "objectId": "-6136671504294265107.1.22257",
      "count": 133,
      "originalStack": ["src/vs/workbench/contrib/debug/browser/linkDetector.ts:323:22"],
      "originalName": "event"
    }
  ],
  "isLeak": true
}

After

No more leak is detected.

[connor4312]

thanks!

[roblourens]

Build error: Error: [define-class-fields-check] src/vs/workbench/contrib/debug/browser/exceptionWidget.ts(103,89): error TS2345: Argument of type 'boolean' is not assignable to parameter of type 'DebugLinkHoverBehaviorTypeData'.

[SimonSiefke]

Thanks!

• Fixed the typescript error
• Fixed disposable warnings in tests

It should be good to go now!

[connor4312]

thanks!

[connor4312]

Looks like something in the tests might be missing a disposable check 😬

  Debug - Link Detector
    ✔ noLinks
    ✔ trailingNewline
    ✔ trailingNewlineSplit
    ✔ singleLineLink
[10314:1216/164627.144861:INFO:CONSOLE:230] "Error: Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!
    at _DisposableStore.add (file:///mnt/vss/_work/vscode/vscode/out/vs/base/common/lifecycle.js:323:22)
    at LinkDetector.decorateLink (file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:253:25)
    at file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:234:12", source: file:///mnt/vss/_work/vscode/vscode/test/unit/electron/renderer.js (230)
[10314:1216/164627.144934:INFO:CONSOLE:230] "Error: Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!
    at _DisposableStore.add (file:///mnt/vss/_work/vscode/vscode/out/vs/base/common/lifecycle.js:323:22)
    at LinkDetector.decorateLink (file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:256:25)
    at file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:234:12", source: file:///mnt/vss/_work/vscode/vscode/test/unit/electron/renderer.js (230)
[10314:1216/164627.144972:INFO:CONSOLE:230] "Error: Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!
    at _DisposableStore.add (file:///mnt/vss/_work/vscode/vscode/out/vs/base/common/lifecycle.js:323:22)
    at LinkDetector.decorateLink (file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:259:25)
    at file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:234:12", source: file:///mnt/vss/_work/vscode/vscode/test/unit/electron/renderer.js (230)
[10314:1216/164627.144991:INFO:CONSOLE:230] "Error: Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!
    at _DisposableStore.add (file:///mnt/vss/_work/vscode/vscode/out/vs/base/common/lifecycle.js:323:22)
    at LinkDetector.decorateLink (file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:271:25)
    at file:///mnt/vss/_work/vscode/vscode/out/vs/workbench/contrib/debug/browser/linkDetector.js:234:12", source: file:///mnt/vss/_work/vscode/vscode/test/unit/electron/renderer.js (230)
    1) "after each" hook for "singleLineLink"


  8628 passing (25s)
  161 pending
  1 failing

  1) "after each" hook for "singleLineLink":

      
      + expected - actual

      -false
      +true
      
  AssertionError [ERR_ASSERTION]: Error: Unexpected console output in test run. Please ensure no console.[log|error|info|warn] usage in tests or runtime errors.
      at Context.<anonymous> (renderer.js:298:11)

[SimonSiefke]

Thanks!

• Fixed another issue where a adding a decoration after fileSystem stat call, could add elements to a disposed store

/* Since this is async. when the promise resolves, the store might be disposed */
this.fileService.stat(uri).then(stat => {
	if (stat.isDirectory) {
		return;
	}
	this.decorateLink(link, uri, fulltext, hoverBehavior, (preserveFocus: boolean) => this.editorService.openEditor({ resource: uri, options: { ...options, preserveFocus } }));
}

[connor4312]

🚀

[Tyriar]

Red CI

[Tyriar]

Also this is on Jan but had auto merge enabled

[SimonSiefke]

Fixed missing semicolons. Now it really should work.

[bpasero]

@microsoft-github-policy-service rerun