chore: address api review for page.forceGarbageCollection (#32824)

- Renamed to `page.requestGC`. - Added a useful snippet to the docs. References #32278. --------- Signed-off-by: Dmitry Gozman <[email protected]> Co-authored-by: Max Schmitt <[email protected]>
microsoft · Sep 26, 2024 · a9d5c39 · a9d5c39
1 parent 6c20318
commit a9d5c39
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 28 deletions.
diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md
@@ -2333,10 +2333,57 @@ last redirect. If cannot go forward, returns `null`.
 
 Navigate to the next page in history.
 
-## async method: Page.forceGarbageCollection
-* since: v1.47
+## async method: Page.requestGC
+* since: v1.48
+
+Request the page to perform garbage collection. Note that there is no guarantee that all unreachable objects will be collected.
+
+This is useful to help detect memory leaks. For example, if your page has a large object `'suspect'` that might be leaked, you can check that it does not leak by using a [`WeakRef`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef).
+
+```js
+// 1. In your page, save a WeakRef for the "suspect".
+await page.evaluate(() => globalThis.suspectWeakRef = new WeakRef(suspect));
+// 2. Request garbage collection.
+await page.requestGC();
+// 3. Check that weak ref does not deref to the original object.
+expect(await page.evaluate(() => !globalThis.suspectWeakRef.deref())).toBe(true);
+```
+
+```java
+// 1. In your page, save a WeakRef for the "suspect".
+page.evaluate("globalThis.suspectWeakRef = new WeakRef(suspect)");
+// 2. Request garbage collection.
+page.requestGC();
+// 3. Check that weak ref does not deref to the original object.
+assertTrue(page.evaluate("!globalThis.suspectWeakRef.deref()"));
+```
 
-Force the browser to perform garbage collection.
+```python async
+# 1. In your page, save a WeakRef for the "suspect".
+await page.evaluate("globalThis.suspectWeakRef = new WeakRef(suspect)")
+# 2. Request garbage collection.
+await page.request_gc()
+# 3. Check that weak ref does not deref to the original object.
+assert await page.evaluate("!globalThis.suspectWeakRef.deref()")
+```
+
+```python sync
+# 1. In your page, save a WeakRef for the "suspect".
+page.evaluate("globalThis.suspectWeakRef = new WeakRef(suspect)")
+# 2. Request garbage collection.
+page.request_gc()
+# 3. Check that weak ref does not deref to the original object.
+assert page.evaluate("!globalThis.suspectWeakRef.deref()")
+```
+
+```csharp
+// 1. In your page, save a WeakRef for the "suspect".
+await Page.EvaluateAsync("globalThis.suspectWeakRef = new WeakRef(suspect)");
+// 2. Request garbage collection.
+await Page.RequestGCAsync();
+// 3. Check that weak ref does not deref to the original object.
+Assert.True(await Page.EvaluateAsync("!globalThis.suspectWeakRef.deref()"));
+```
 
 ### option: Page.goForward.waitUntil = %%-navigation-wait-until-%%
 * since: v1.8

diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts
@@ -478,8 +478,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     return Response.fromNullable((await this._channel.goForward({ ...options, waitUntil })).response);
   }
 
-  async forceGarbageCollection() {
-    await this._channel.forceGarbageCollection();
+  async requestGC() {
+    await this._channel.requestGC();
   }
 
   async emulateMedia(options: { media?: 'screen' | 'print' | null, colorScheme?: 'dark' | 'light' | 'no-preference' | null, reducedMotion?: 'reduce' | 'no-preference' | null, forcedColors?: 'active' | 'none' | null } = {}) {

diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts
@@ -1137,8 +1137,8 @@ scheme.PageGoForwardParams = tObject({
 scheme.PageGoForwardResult = tObject({
   response: tOptional(tChannel(['Response'])),
 });
-scheme.PageForceGarbageCollectionParams = tOptional(tObject({}));
-scheme.PageForceGarbageCollectionResult = tOptional(tObject({}));
+scheme.PageRequestGCParams = tOptional(tObject({}));
+scheme.PageRequestGCResult = tOptional(tObject({}));
 scheme.PageRegisterLocatorHandlerParams = tObject({
   selector: tString,
   noWaitAfter: tOptional(tBoolean),

diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts
@@ -340,7 +340,7 @@ export class BidiPage implements PageDelegate {
     }).then(() => true).catch(() => false);
   }
 
-  async forceGarbageCollection(): Promise<void> {
+  async requestGC(): Promise<void> {
     throw new Error('Method not implemented.');
   }
 

diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts
@@ -247,7 +247,7 @@ export class CRPage implements PageDelegate {
     return this._go(+1);
   }
 
-  async forceGarbageCollection(): Promise<void> {
+  async requestGC(): Promise<void> {
     await this._mainFrameSession._client.send('HeapProfiler.collectGarbage');
   }
 

diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts
@@ -139,8 +139,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
     return { response: ResponseDispatcher.fromNullable(this.parentScope(), await this._page.goForward(metadata, params)) };
   }
 
-  async forceGarbageCollection(params: channels.PageForceGarbageCollectionParams, metadata: CallMetadata): Promise<channels.PageForceGarbageCollectionResult> {
-    await this._page.forceGarbageCollection();
+  async requestGC(params: channels.PageRequestGCParams, metadata: CallMetadata): Promise<channels.PageRequestGCResult> {
+    await this._page.requestGC();
   }
 
   async registerLocatorHandler(params: channels.PageRegisterLocatorHandlerParams, metadata: CallMetadata): Promise<channels.PageRegisterLocatorHandlerResult> {

diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts
@@ -399,7 +399,7 @@ export class FFPage implements PageDelegate {
     return success;
   }
 
-  async forceGarbageCollection(): Promise<void> {
+  async requestGC(): Promise<void> {
     await this._session.send('Heap.collectGarbage');
   }
 

diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts
@@ -54,7 +54,7 @@ export interface PageDelegate {
   reload(): Promise<void>;
   goBack(): Promise<boolean>;
   goForward(): Promise<boolean>;
-  forceGarbageCollection(): Promise<void>;
+  requestGC(): Promise<void>;
   addInitScript(initScript: InitScript): Promise<void>;
   removeNonInternalInitScripts(): Promise<void>;
   closePage(runBeforeUnload: boolean): Promise<void>;
@@ -431,8 +431,8 @@ export class Page extends SdkObject {
     }), this._timeoutSettings.navigationTimeout(options));
   }
 
-  forceGarbageCollection(): Promise<void> {
-    return this._delegate.forceGarbageCollection();
+  requestGC(): Promise<void> {
+    return this._delegate.requestGC();
   }
 
   registerLocatorHandler(selector: string, noWaitAfter: boolean | undefined) {

diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts
@@ -767,7 +767,7 @@ export class WKPage implements PageDelegate {
     });
   }
 
-  async forceGarbageCollection(): Promise<void> {
+  async requestGC(): Promise<void> {
     await this._session.send('Heap.gc');
   }
 

diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts
@@ -2558,11 +2558,6 @@ export interface Page {
     timeout?: number;
   }): Promise<void>;
 
-  /**
-   * Force the browser to perform garbage collection.
-   */
-  forceGarbageCollection(): Promise<void>;
-
   /**
    * Returns frame matching the specified criteria. Either `name` or `url` must be specified.
    *
@@ -3712,6 +3707,26 @@ export interface Page {
    */
   removeLocatorHandler(locator: Locator): Promise<void>;
 
+  /**
+   * Request the page to perform garbage collection. Note that there is no guarantee that all unreachable objects will
+   * be collected.
+   *
+   * This is useful to help detect memory leaks. For example, if your page has a large object `'suspect'` that might be
+   * leaked, you can check that it does not leak by using a
+   * [`WeakRef`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef).
+   *
+   * ```js
+   * // 1. In your page, save a WeakRef for the "suspect".
+   * await page.evaluate(() => globalThis.suspectWeakRef = new WeakRef(suspect));
+   * // 2. Request garbage collection.
+   * await page.requestGC();
+   * // 3. Check that weak ref does not deref to the original object.
+   * expect(await page.evaluate(() => !globalThis.suspectWeakRef.deref())).toBe(true);
+   * ```
+   *
+   */
+  requestGC(): Promise<void>;
+
   /**
    * Routing provides the capability to modify network requests that are made by a page.
    *

diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts
@@ -1956,7 +1956,7 @@ export interface PageChannel extends PageEventTarget, EventTargetChannel {
   exposeBinding(params: PageExposeBindingParams, metadata?: CallMetadata): Promise<PageExposeBindingResult>;
   goBack(params: PageGoBackParams, metadata?: CallMetadata): Promise<PageGoBackResult>;
   goForward(params: PageGoForwardParams, metadata?: CallMetadata): Promise<PageGoForwardResult>;
-  forceGarbageCollection(params?: PageForceGarbageCollectionParams, metadata?: CallMetadata): Promise<PageForceGarbageCollectionResult>;
+  requestGC(params?: PageRequestGCParams, metadata?: CallMetadata): Promise<PageRequestGCResult>;
   registerLocatorHandler(params: PageRegisterLocatorHandlerParams, metadata?: CallMetadata): Promise<PageRegisterLocatorHandlerResult>;
   resolveLocatorHandlerNoReply(params: PageResolveLocatorHandlerNoReplyParams, metadata?: CallMetadata): Promise<PageResolveLocatorHandlerNoReplyResult>;
   unregisterLocatorHandler(params: PageUnregisterLocatorHandlerParams, metadata?: CallMetadata): Promise<PageUnregisterLocatorHandlerResult>;
@@ -2098,9 +2098,9 @@ export type PageGoForwardOptions = {
 export type PageGoForwardResult = {
   response?: ResponseChannel,
 };
-export type PageForceGarbageCollectionParams = {};
-export type PageForceGarbageCollectionOptions = {};
-export type PageForceGarbageCollectionResult = void;
+export type PageRequestGCParams = {};
+export type PageRequestGCOptions = {};
+export type PageRequestGCResult = void;
 export type PageRegisterLocatorHandlerParams = {
   selector: string,
   noWaitAfter?: boolean,

diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml
@@ -1450,7 +1450,7 @@ Page:
         slowMo: true
         snapshot: true
 
-    forceGarbageCollection:
+    requestGC:
 
     registerLocatorHandler:
       parameters:

diff --git a/tests/page/page-force-gc.spec.ts → tests/page/page-request-gc.spec.ts b/tests/page/page-force-gc.spec.ts → tests/page/page-request-gc.spec.ts
@@ -18,10 +18,17 @@ import { test, expect } from './pageTest';
 
 test('should work', async ({ page }) => {
   await page.evaluate(() => {
-    globalThis.objectToDestroy = {};
+    globalThis.objectToDestroy = { hello: 'world' };
     globalThis.weakRef = new WeakRef(globalThis.objectToDestroy);
   });
+
+  await page.requestGC();
+  expect(await page.evaluate(() => globalThis.weakRef.deref())).toEqual({ hello: 'world' });
+
+  await page.requestGC();
+  expect(await page.evaluate(() => globalThis.weakRef.deref())).toEqual({ hello: 'world' });
+
   await page.evaluate(() => globalThis.objectToDestroy = null);
-  await page.forceGarbageCollection();
+  await page.requestGC();
   expect(await page.evaluate(() => globalThis.weakRef.deref())).toBe(undefined);
 });