Skip to content

Commit 9227c7b

Browse files
crisbetojelbourn
authored andcommitted
fix(clipboard): leak if directive is destroyed while a copy is pending (#18066)
If the `cdkCopyToClipboard` directive is destroyed while there are pending copies, we'll create a leak since the `PendingCopy` will never be destroyed. These changes add some extra logic to ensure that the copies are destroyed and the timeouts are cleared.
1 parent c319431 commit 9227c7b

File tree

3 files changed

+62
-8
lines changed

3 files changed

+62
-8
lines changed

src/cdk/clipboard/copy-to-clipboard.spec.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('CdkCopyToClipboard', () => {
5959
it('emits copied event false when copy fails', fakeAsync(() => {
6060
spyOn(clipboard, 'copy').and.returnValue(false);
6161
fixture.nativeElement.querySelector('button')!.click();
62-
tick();
62+
tick(1);
6363

6464
expect(fixture.componentInstance.copied).toHaveBeenCalledWith(false);
6565
}));
@@ -76,7 +76,7 @@ describe('CdkCopyToClipboard', () => {
7676

7777
fixture.nativeElement.querySelector('button')!.click();
7878
fixture.detectChanges();
79-
tick();
79+
tick(3);
8080

8181
expect(attempts).toBe(maxAttempts);
8282
expect(fixture.componentInstance.copied).toHaveBeenCalledTimes(1);
@@ -98,10 +98,37 @@ describe('CdkCopyToClipboard', () => {
9898

9999
fixture.nativeElement.querySelector('button')!.click();
100100
fixture.detectChanges();
101-
tick();
101+
tick(3);
102102

103103
expect(attempts).toBe(maxAttempts);
104104
expect(fixture.componentInstance.copied).toHaveBeenCalledTimes(1);
105105
expect(fixture.componentInstance.copied).toHaveBeenCalledWith(false);
106106
}));
107+
108+
it('should destroy any pending copies when the directive is destroyed', fakeAsync(() => {
109+
const fakeCopy = {
110+
copy: jasmine.createSpy('copy spy').and.returnValue(false) as () => boolean,
111+
destroy: jasmine.createSpy('destroy spy') as () => void
112+
} as PendingCopy;
113+
114+
fixture.componentInstance.attempts = 10;
115+
fixture.detectChanges();
116+
117+
spyOn(clipboard, 'beginCopy').and.returnValue(fakeCopy);
118+
fixture.detectChanges();
119+
120+
fixture.nativeElement.querySelector('button')!.click();
121+
fixture.detectChanges();
122+
tick(1);
123+
124+
expect(fakeCopy.copy).toHaveBeenCalledTimes(2);
125+
expect(fakeCopy.destroy).toHaveBeenCalledTimes(0);
126+
127+
fixture.destroy();
128+
tick(1);
129+
130+
expect(fakeCopy.copy).toHaveBeenCalledTimes(2);
131+
expect(fakeCopy.destroy).toHaveBeenCalledTimes(1);
132+
}));
133+
107134
});

src/cdk/clipboard/copy-to-clipboard.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ import {
1515
InjectionToken,
1616
Inject,
1717
Optional,
18+
OnDestroy,
1819
} from '@angular/core';
1920
import {Clipboard} from './clipboard';
21+
import {PendingCopy} from './pending-copy';
2022

2123
/** Object that can be used to configure the default options for `CdkCopyToClipboard`. */
2224
export interface CdkCopyToClipboardConfig {
@@ -38,7 +40,7 @@ export const CKD_COPY_TO_CLIPBOARD_CONFIG =
3840
'(click)': 'copy()',
3941
}
4042
})
41-
export class CdkCopyToClipboard {
43+
export class CdkCopyToClipboard implements OnDestroy {
4244
/** Content to be copied. */
4345
@Input('cdkCopyToClipboard') text: string = '';
4446

@@ -62,6 +64,15 @@ export class CdkCopyToClipboard {
6264
*/
6365
@Output('copied') _deprecatedCopied = this.copied;
6466

67+
/** Copies that are currently being attempted. */
68+
private _pending = new Set<PendingCopy>();
69+
70+
/** Whether the directive has been destroyed. */
71+
private _destroyed: boolean;
72+
73+
/** Timeout for the current copy attempt. */
74+
private _currentTimeout: any;
75+
6576
constructor(
6677
private _clipboard: Clipboard,
6778
/**
@@ -81,16 +92,21 @@ export class CdkCopyToClipboard {
8192
if (attempts > 1) {
8293
let remainingAttempts = attempts;
8394
const pending = this._clipboard.beginCopy(this.text);
95+
this._pending.add(pending);
96+
8497
const attempt = () => {
8598
const successful = pending.copy();
86-
if (!successful && --remainingAttempts) {
99+
if (!successful && --remainingAttempts && !this._destroyed) {
87100
// @breaking-change 10.0.0 Remove null check for `_ngZone`.
88101
if (this._ngZone) {
89-
this._ngZone.runOutsideAngular(() => setTimeout(attempt));
102+
this._currentTimeout = this._ngZone.runOutsideAngular(() => setTimeout(attempt, 1));
90103
} else {
91-
setTimeout(attempt);
104+
// We use 1 for the timeout since it's more predictable when flushing in unit tests.
105+
this._currentTimeout = setTimeout(attempt, 1);
92106
}
93107
} else {
108+
this._currentTimeout = null;
109+
this._pending.delete(pending);
94110
pending.destroy();
95111
this.copied.emit(successful);
96112
}
@@ -100,4 +116,14 @@ export class CdkCopyToClipboard {
100116
this.copied.emit(this._clipboard.copy(this.text));
101117
}
102118
}
119+
120+
ngOnDestroy() {
121+
if (this._currentTimeout) {
122+
clearTimeout(this._currentTimeout);
123+
}
124+
125+
this._pending.forEach(copy => copy.destroy());
126+
this._pending.clear();
127+
this._destroyed = true;
128+
}
103129
}

tools/public_api_guard/cdk/clipboard.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
export declare class CdkCopyToClipboard {
1+
export declare class CdkCopyToClipboard implements OnDestroy {
22
_deprecatedCopied: EventEmitter<boolean>;
33
attempts: number;
44
copied: EventEmitter<boolean>;
55
text: string;
66
constructor(_clipboard: Clipboard,
77
_ngZone?: NgZone | undefined, config?: CdkCopyToClipboardConfig);
88
copy(attempts?: number): void;
9+
ngOnDestroy(): void;
910
static ɵdir: i0.ɵɵDirectiveDefWithMeta<CdkCopyToClipboard, "[cdkCopyToClipboard]", never, { "text": "cdkCopyToClipboard"; "attempts": "cdkCopyToClipboardAttempts"; }, { "copied": "cdkCopyToClipboardCopied"; "_deprecatedCopied": "copied"; }, never>;
1011
static ɵfac: i0.ɵɵFactoryDef<CdkCopyToClipboard>;
1112
}

0 commit comments

Comments
 (0)