Revert debug controller UI additions (#37661)

Author: Skn0tt

.github/workflows/tests_primary.yml

@@ -187,7 +187,7 @@ jobs:
       env:
         DEBUG: pw:install
     - run: npm run build
-    - run: npx playwright install chromium firefox
+    - run: npx playwright install chromium
     - name: Checkout extension
       run: git clone https://github.com/microsoft/playwright-vscode.git
     - name: Print extension revision

packages/playwright-core/src/browserServerImpl.ts

@@ -26,7 +26,7 @@ import * as validatorPrimitives from './protocol/validatorPrimitives';
 import { ProgressController } from './server/progress';
 
 import type { BrowserServer, BrowserServerLauncher } from './client/browserType';
-import type { LaunchOptions, LaunchServerOptions, Logger } from './client/types';
+import type { LaunchServerOptions, Logger } from './client/types';
 import type { ProtocolLogger } from './server/types';
 import type { WebSocketEventEmitter } from './utilsBundle';
 import type { Browser } from './server/browser';
@@ -38,7 +38,7 @@ export class BrowserServerLauncherImpl implements BrowserServerLauncher {
     this._browserName = browserName;
   }
 
-  async launchServer(options: LaunchOptions & LaunchServerOptions & { _userDataDir?: string } = {}): Promise<BrowserServer> {
+  async launchServer(options: LaunchServerOptions & { _sharedBrowser?: boolean, _userDataDir?: string } = {}): Promise<BrowserServer> {
     const playwright = createPlaywright({ sdkLanguage: 'javascript', isServer: true });
     // 1. Pre-launch the browser
     const metadata = { id: '', startTime: 0, endTime: 0, type: 'Internal', method: '', params: {}, log: [], internal: true };
@@ -78,14 +78,10 @@ export class BrowserServerLauncherImpl implements BrowserServerLauncher {
       throw e;
     }
 
-    return this.launchServerOnExistingBrowser(browser, options);
-  }
-
-  async launchServerOnExistingBrowser(browser: Browser, options: LaunchServerOptions): Promise<BrowserServer> {
     const path = options.wsPath ? (options.wsPath.startsWith('/') ? options.wsPath : `/${options.wsPath}`) : `/${createGuid()}`;
 
     // 2. Start the server
-    const server = new PlaywrightServer({ mode: options._sharedBrowser ? 'launchServerShared' : 'launchServer', path, maxConnections: Infinity, preLaunchedBrowser: browser, debugController: options._debugController });
+    const server = new PlaywrightServer({ mode: options._sharedBrowser ? 'launchServerShared' : 'launchServer', path, maxConnections: Infinity, preLaunchedBrowser: browser });
     const wsEndpoint = await server.listen(options.port, options.host);
 
     // 3. Return the BrowserServer interface

packages/playwright-core/src/client/browser.ts

@@ -24,7 +24,7 @@ import { mkdirIfNeeded } from './fileUtils';
 
 import type { BrowserType } from './browserType';
 import type { Page } from './page';
-import type { BrowserContextOptions, LaunchOptions, LaunchServerOptions, Logger } from './types';
+import type { BrowserContextOptions, LaunchOptions, Logger } from './types';
 import type * as api from '../../types/types';
 import type * as channels from '@protocol/channels';
 
@@ -146,17 +146,6 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
     return CDPSession.from((await this._channel.newBrowserCDPSession()).session);
   }
 
-  async _launchServer(options: LaunchServerOptions = {}) {
-    const serverLauncher = this._browserType._serverLauncher;
-    const browserImpl = this._connection.toImpl?.(this);
-    if (!serverLauncher || !browserImpl)
-      throw new Error('Launching server is not supported');
-    return await serverLauncher.launchServerOnExistingBrowser(browserImpl, {
-      _sharedBrowser: true,
-      ...options,
-    });
-  }
-
   async startTracing(page?: Page, options: { path?: string; screenshots?: boolean; categories?: string[]; } = {}) {
     this._path = options.path;
     await this._channel.startTracing({ ...options, page: page ? page._channel : undefined });

packages/playwright-core/src/client/browserType.ts

@@ -31,11 +31,9 @@ import type { ConnectOptions, LaunchOptions, LaunchPersistentContextOptions, Lau
 import type * as api from '../../types/types';
 import type * as channels from '@protocol/channels';
 import type { ChildProcess } from 'child_process';
-import type { Browser as BrowserImpl } from '../server/browser';
 
 export interface BrowserServerLauncher {
-  launchServer(options?: LaunchOptions & LaunchServerOptions): Promise<api.BrowserServer>;
-  launchServerOnExistingBrowser(browser: BrowserImpl, options?: LaunchServerOptions): Promise<api.BrowserServer>;
+  launchServer(options?: LaunchServerOptions): Promise<api.BrowserServer>;
 }
 
 // This is here just for api generation and checking.

packages/playwright-core/src/client/types.ts

@@ -104,12 +104,10 @@ export type ConnectOptions = {
   timeout?: number,
   logger?: Logger,
 };
-export type LaunchServerOptions = {
+export type LaunchServerOptions = LaunchOptions & {
   host?: string,
   port?: number,
   wsPath?: string,
-  _debugController?: boolean;
-  _sharedBrowser?: boolean;
 };
 
 export type LaunchAndroidServerOptions = {

packages/playwright-core/src/protocol/validator.ts

@@ -445,16 +445,6 @@ scheme.DebugControllerSetModeRequestedEvent = tObject({
 });
 scheme.DebugControllerStateChangedEvent = tObject({
   pageCount: tInt,
-  browsers: tArray(tObject({
-    id: tString,
-    name: tString,
-    channel: tOptional(tString),
-    contexts: tArray(tObject({
-      pages: tArray(tObject({
-        url: tString,
-      })),
-    })),
-  })),
 });
 scheme.DebugControllerSourceChangedEvent = tObject({
   text: tString,
@@ -475,7 +465,6 @@ scheme.DebugControllerSetReportStateChangedParams = tObject({
 });
 scheme.DebugControllerSetReportStateChangedResult = tOptional(tObject({}));
 scheme.DebugControllerSetRecorderModeParams = tObject({
-  browserId: tOptional(tString),
   mode: tEnum(['inspecting', 'recording', 'none']),
   testIdAttributeName: tOptional(tString),
   generateAutoExpect: tOptional(tBoolean),
@@ -490,11 +479,6 @@ scheme.DebugControllerHideHighlightParams = tOptional(tObject({}));
 scheme.DebugControllerHideHighlightResult = tOptional(tObject({}));
 scheme.DebugControllerResumeParams = tOptional(tObject({}));
 scheme.DebugControllerResumeResult = tOptional(tObject({}));
-scheme.DebugControllerCloseBrowserParams = tObject({
-  id: tString,
-  reason: tOptional(tString),
-});
-scheme.DebugControllerCloseBrowserResult = tOptional(tObject({}));
 scheme.DebugControllerKillParams = tOptional(tObject({}));
 scheme.DebugControllerKillResult = tOptional(tObject({}));
 scheme.SocksSupportInitializer = tOptional(tObject({}));

packages/playwright-core/src/remote/playwrightServer.ts

@@ -39,7 +39,6 @@ type ServerOptions = {
   preLaunchedBrowser?: Browser;
   preLaunchedAndroidDevice?: AndroidDevice;
   preLaunchedSocksProxy?: SocksProxy;
-  debugController?: boolean;
 };
 
 export class PlaywrightServer {
@@ -107,19 +106,6 @@ export class PlaywrightServer {
         const allowFSPaths = isExtension;
         launchOptions = filterLaunchOptions(launchOptions, allowFSPaths);
 
-        if (url.searchParams.has('debug-controller')) {
-          if (!(this._options.debugController || isExtension))
-            throw new Error('Debug controller is not enabled');
-          return new PlaywrightConnection(
-              controllerSemaphore,
-              ws,
-              true,
-              this._playwright,
-              async () => { throw new Error('shouldnt be used'); },
-              id,
-          );
-        }
-
         if (isExtension) {
           const connectFilter = url.searchParams.get('connect');
           if (connectFilter) {
@@ -135,6 +121,16 @@ export class PlaywrightServer {
             );
           }
 
+          if (url.searchParams.has('debug-controller')) {
+            return new PlaywrightConnection(
+                controllerSemaphore,
+                ws,
+                true,
+                this._playwright,
+                async () => { throw new Error('shouldnt be used'); },
+                id,
+            );
+          }
           return new PlaywrightConnection(
               reuseBrowserSemaphore,
               ws,

packages/playwright-core/src/server/debugController.ts

@@ -24,8 +24,6 @@ import { unsafeLocatorOrSelectorAsSelector } from '../utils/isomorphic/locatorPa
 import { generateCode } from './codegen/language';
 import { collapseActions } from './recorder/recorderUtils';
 import { JavaScriptLanguageGenerator } from './codegen/javascript';
-import { Frame } from './frames';
-import { Page } from './page';
 
 import type { Language } from '../utils';
 import type { BrowserContext } from './browserContext';
@@ -44,8 +42,7 @@ export class DebugController extends SdkObject {
     SetModeRequested: 'setModeRequested',
   };
 
-  private _reportState = false;
-  private _disposeListeners = new Set<() => void>();
+  private _trackHierarchyListener: InstrumentationListener | undefined;
   private _playwright: Playwright;
   _sdkLanguage: Language = 'javascript';
   _generateAutoExpect = false;
@@ -64,40 +61,25 @@ export class DebugController extends SdkObject {
   }
 
   setReportStateChanged(enabled: boolean) {
-    if (this._reportState === enabled)
-      return;
-    this._reportState = enabled;
-    if (enabled) {
-      const listener: InstrumentationListener = {
-        onPageOpen: page => {
-          this._emitSnapshot(false);
-          const handleNavigation = () => this._emitSnapshot(false);
-          page.mainFrame().on(Frame.Events.InternalNavigation, handleNavigation);
-          const dispose = () => page.mainFrame().off(Frame.Events.InternalNavigation, handleNavigation);
-          this._disposeListeners.add(dispose);
-          page.on(Page.Events.Close, () => this._disposeListeners.delete(dispose));
-        },
+    if (enabled && !this._trackHierarchyListener) {
+      this._trackHierarchyListener = {
+        onPageOpen: () => this._emitSnapshot(false),
         onPageClose: () => this._emitSnapshot(false),
-        onBrowserClose: () => {
-          this._emitSnapshot(false);
-        },
       };
-      this._playwright.instrumentation.addListener(listener, null);
-      this._disposeListeners.add(() => this._playwright.instrumentation.removeListener(listener));
+      this._playwright.instrumentation.addListener(this._trackHierarchyListener, null);
       this._emitSnapshot(true);
-    } else {
-      for (const dispose of this._disposeListeners)
-        dispose();
-      this._disposeListeners.clear();
+    } else if (!enabled && this._trackHierarchyListener) {
+      this._playwright.instrumentation.removeListener(this._trackHierarchyListener);
+      this._trackHierarchyListener = undefined;
     }
   }
 
-  async setRecorderMode(progress: Progress, params: { mode: Mode, testIdAttributeName?: string, generateAutoExpect?: boolean, browserId?: string }) {
+  async setRecorderMode(progress: Progress, params: { mode: Mode, testIdAttributeName?: string, generateAutoExpect?: boolean }) {
     await progress.race(this._closeBrowsersWithoutPages());
     this._generateAutoExpect = !!params.generateAutoExpect;
 
     if (params.mode === 'none') {
-      for (const recorder of await progress.race(this._allRecorders(params.browserId))) {
+      for (const recorder of await progress.race(this._allRecorders())) {
         recorder.hideHighlightedSelector();
         recorder.setMode('none');
       }
@@ -115,14 +97,11 @@ export class DebugController extends SdkObject {
     }
     // Update test id attribute.
     if (params.testIdAttributeName) {
-      for (const page of this._playwright.allPages()) {
-        if (params.browserId && page.browserContext._browser.guid !== params.browserId)
-          continue;
+      for (const page of this._playwright.allPages())
         page.browserContext.selectors().setTestIdAttributeName(params.testIdAttributeName);
-      }
     }
     // Toggle the mode.
-    for (const recorder of await progress.race(this._allRecorders(params.browserId))) {
+    for (const recorder of await progress.race(this._allRecorders())) {
       recorder.hideHighlightedSelector();
       recorder.setMode(params.mode);
     }
@@ -154,13 +133,6 @@ export class DebugController extends SdkObject {
       recorder.resume();
   }
 
-  async closeBrowser(progress: Progress, id: string, reason?: string) {
-    const browser = this._playwright.allBrowsers().find(b => b.guid === id);
-    if (!browser)
-      return;
-    await progress.race(browser.close({ reason }));
-  }
-
   kill() {
     gracefullyProcessExitDoNotHang(0);
   }
@@ -169,28 +141,13 @@ export class DebugController extends SdkObject {
     const pageCount = this._playwright.allPages().length;
     if (initial && !pageCount)
       return;
-    this.emit(DebugController.Events.StateChanged, {
-      pageCount,
-      browsers: this._playwright.allBrowsers().map(browser => ({
-        id: browser.guid,
-        name: browser.options.name,
-        channel: browser.options.channel,
-        contexts: browser.contexts().map(context => ({
-          pages: context.pages().map(page => ({
-            url: page.mainFrame().url(),
-          }))
-        }))
-      }))
-    });
+    this.emit(DebugController.Events.StateChanged, { pageCount });
   }
 
-  private async _allRecorders(browserId?: string): Promise<Recorder[]> {
+  private async _allRecorders(): Promise<Recorder[]> {
     const contexts = new Set<BrowserContext>();
-    for (const page of this._playwright.allPages()) {
-      if (browserId && page.browserContext._browser.guid !== browserId)
-        continue;
+    for (const page of this._playwright.allPages())
       contexts.add(page.browserContext);
-    }
     const recorders = await Promise.all([...contexts].map(c => Recorder.forContext(c, { omitCallTracking: true })));
     const nonNullRecorders = recorders.filter(Boolean) as Recorder[];
     for (const recorder of recorders)

packages/playwright-core/src/server/dispatchers/debugControllerDispatcher.ts

@@ -78,10 +78,6 @@ export class DebugControllerDispatcher extends Dispatcher<DebugController, chann
     this._object.kill();
   }
 
-  async closeBrowser(params: channels.DebugControllerCloseBrowserParams, progress: Progress) {
-    await this._object.closeBrowser(progress, params.id, params.reason);
-  }
-
   override _onDispose() {
     eventsHelper.removeEventListeners(this._listeners);
     this._object.dispose();

packages/playwright-core/src/server/utils/wsServer.ts

@@ -106,13 +106,8 @@ export class WSServer {
       const url = new URL('http://localhost' + (request.url || ''));
       const id = String(++lastConnectionId);
       debugLogger.log('server', `[${id}] serving connection: ${request.url}`);
-      try {
-        const connection = this._delegate.onConnection(request, url, ws, id);
-        (ws as any)[kConnectionSymbol] = connection;
-      } catch (error) {
-        debugLogger.log('server', `[${id}] connection error: ${error}`);
-        ws.close(1011, String(error));
-      }
+      const connection = this._delegate.onConnection(request, url, ws, id);
+      (ws as any)[kConnectionSymbol] = connection;
     });
 
     return wsEndpoint;