Skip pages without a renderer when connecting over CDP

.github/workflows/dotnet.yml

@@ -217,8 +217,10 @@ jobs:
       if: ${{ matrix.browser == 'chromium' && matrix.os == 'ubuntu-latest' && matrix.mode == 'headless' }}
       env:
         PRODUCT: CHROMIUM
+      # Like upstream, give headless runs a display too: some tests force headless: false.
       run: |
-        dotnet test ./src/PlaywrightNative.Tests/PlaywrightNative.Tests.csproj --no-build -c Release -f net10.0 -s src/PlaywrightNative.Tests/test.runsettings ${TEST_FILTER:+--filter "$TEST_FILTER"}
+        xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- \
+          dotnet test ./src/PlaywrightNative.Tests/PlaywrightNative.Tests.csproj --no-build -c Release -f net10.0 -s src/PlaywrightNative.Tests/test.runsettings ${TEST_FILTER:+--filter "$TEST_FILTER"}
     - name: Test (Chromium, Linux headful)
       if: ${{ matrix.browser == 'chromium' && matrix.os == 'ubuntu-latest' && matrix.mode == 'headful' }}
       env:

src/PlaywrightNative.Tests/LibraryConnectOverCdpParityTests.cs

@@ -159,6 +159,145 @@ await WithHostAsync(async (host, endpoint) =>
             }).ConfigureAwait(false);
         }
 
+        [PlaywrightTest("connect-over-cdp.spec.ts", "should connect when an existing page has no renderer")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public async Task ShouldConnectWhenAnExistingPageHasNoRenderer()
+        {
+            EnsureServer();
+            await WithHostAsync(async (host, endpoint) =>
+            {
+                // Prepare a page without a renderer, e.g. one discarded by the Memory Saver.
+                IBrowser cdpBrowser1 = await Playwright.Chromium.ConnectOverCDPAsync(endpoint).ConfigureAwait(false);
+                IBrowserContext context1 = FirstContext(cdpBrowser1);
+                IPage healthyPage1 = await context1.NewPageAsync().ConfigureAwait(false);
+                await healthyPage1.GoToAsync(Prefix + "/title.html").ConfigureAwait(false);
+                IPage crashedPage1 = await context1.NewPageAsync().ConfigureAwait(false);
+                await crashedPage1.GoToAsync(TestConstants.EmptyPage).ConfigureAwait(false);
+                Task<IPage> crashTask = crashedPage1.WaitForCrashAsync();
+                _ = crashedPage1.GoToAsync("chrome://crash").ContinueWith(_ => { }, TaskScheduler.Default);
+                await crashTask.ConfigureAwait(false);
+                await cdpBrowser1.CloseAsync().ConfigureAwait(false);
+
+                // Connecting again should not hang on the page without a renderer,
+                // and should not report it at all.
+                IBrowser cdpBrowser2 = await Playwright.Chromium.ConnectOverCDPAsync(endpoint).ConfigureAwait(false);
+                IReadOnlyList<IPage> pages = FirstContext(cdpBrowser2).Pages;
+                Assert.That(pages.Select(page => page.Url), Is.EqualTo(new[] { Prefix + "/title.html" }));
+                Assert.That(await pages[0].TitleAsync().ConfigureAwait(false), Is.EqualTo("Woof-Woof"));
+                await cdpBrowser2.CloseAsync().ConfigureAwait(false);
+            }).ConfigureAwait(false);
+        }
+
+        [PlaywrightTest("connect-over-cdp.spec.ts", "should connect when the opener of an existing page has no renderer")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public async Task ShouldConnectWhenTheOpenerOfAnExistingPageHasNoRenderer()
+        {
+            EnsureServer();
+
+            // The popup should be in a different process to survive the crash of the opener.
+            await WithHostAsync(
+                async (host, endpoint) =>
+                {
+                    IBrowser cdpBrowser1 = await Playwright.Chromium.ConnectOverCDPAsync(endpoint).ConfigureAwait(false);
+                    IPage crashedPage1 = await FirstContext(cdpBrowser1).NewPageAsync().ConfigureAwait(false);
+                    await crashedPage1.GoToAsync(TestConstants.EmptyPage).ConfigureAwait(false);
+                    Task<IPage> popupTask = crashedPage1.WaitForPopupAsync();
+                    await crashedPage1.EvaluateAsync("url => { window.open(url); }", TestConstants.CrossProcessUrl + "/title.html").ConfigureAwait(false);
+                    IPage popup1 = await popupTask.ConfigureAwait(false);
+                    await popup1.WaitForLoadStateAsync().ConfigureAwait(false);
+                    Task<IPage> crashTask = crashedPage1.WaitForCrashAsync();
+                    _ = crashedPage1.GoToAsync("chrome://crash").ContinueWith(_ => { }, TaskScheduler.Default);
+                    await crashTask.ConfigureAwait(false);
+                    await cdpBrowser1.CloseAsync().ConfigureAwait(false);
+
+                    // Connecting again should not report the opener, and should not wait for it.
+                    IBrowser cdpBrowser2 = await Playwright.Chromium.ConnectOverCDPAsync(endpoint).ConfigureAwait(false);
+                    IReadOnlyList<IPage> pages = FirstContext(cdpBrowser2).Pages;
+                    Assert.That(pages.Select(page => page.Url), Is.EqualTo(new[] { TestConstants.CrossProcessUrl + "/title.html" }));
+                    Assert.That(await pages[0].TitleAsync().ConfigureAwait(false), Is.EqualTo("Woof-Woof"));
+                    Assert.That(await pages[0].OpenerAsync().ConfigureAwait(false), Is.Null);
+                    await cdpBrowser2.CloseAsync().ConfigureAwait(false);
+                },
+                "--site-per-process").ConfigureAwait(false);
+        }
+
+        [PlaywrightTest("connect-over-cdp.spec.ts", "should connect when an existing page has been discarded")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public async Task ShouldConnectWhenAnExistingPageHasBeenDiscarded()
+        {
+            EnsureServer();
+            if (string.IsNullOrEmpty(BrowserExecutableFixture.ChromiumExecutablePath))
+            {
+                Assert.Ignore("Chromium executable not available (download skipped or failed).");
+            }
+
+            int port = FreeCdpPort();
+            string userDataDir = Path.Combine(Path.GetTempPath(), "pwsharp-cdp-discard-" + Guid.NewGuid().ToString("N"));
+            Directory.CreateDirectory(userDataDir);
+
+            // chrome://discards is an internal debug page that is only available with this pref.
+            await File.WriteAllTextAsync(Path.Combine(userDataDir, "Local State"), "{\"internal_only_uis_enabled\":true}").ConfigureAwait(false);
+            IBrowserContext context = await Playwright.Chromium.LaunchPersistentContextAsync(userDataDir, new BrowserTypeLaunchPersistentContextOptions
+            {
+                ExecutablePath = BrowserExecutableFixture.ChromiumExecutablePath,
+                Headless = false,
+
+                // Emulating the viewport of a discarded tab crashes the browser in
+                // WebContentsImpl::SetDeviceEmulationSize, because it has no view.
+                ViewportSize = ViewportSize.NoViewport,
+                Args = new[]
+                {
+                    "--remote-debugging-port=" + port,
+
+                    // Chrome refuses to discard tabs with DevTools attached, and we attach to all of them.
+                    "--enable-features=AllowDevtoolsConnectedDiscard",
+                },
+            }).ConfigureAwait(false);
+            try
+            {
+                string victimUrl = Prefix + "/title.html";
+                IPage victim = await context.NewPageAsync().ConfigureAwait(false);
+                await victim.GoToAsync(victimUrl).ConfigureAwait(false);
+                IPage discards = await context.NewPageAsync().ConfigureAwait(false);
+                await discards.GoToAsync("chrome://discards/").ConfigureAwait(false);
+
+                // Discarding replaces the tab's WebContents, but the url in the table survives.
+                ILocator row = discards.GetByRole(AriaRole.Row, new() { Name = victimUrl });
+                await row.GetByText("Urgent Discard").ClickAsync().ConfigureAwait(false);
+                await Expect(row).ToContainTextAsync("discarded").ConfigureAwait(false);
+
+                // The renderer process is shut down asynchronously after the discard.
+                await discards.WaitForTimeoutAsync(3000).ConfigureAwait(false);
+                IReadOnlyList<string> rows = await discards.GetByRole(AriaRole.Row).AllInnerTextsAsync().ConfigureAwait(false);
+                TestContext.Out.WriteLine("discards rows:\n" + string.Join("\n", rows.Select(text => System.Text.RegularExpressions.Regex.Replace(text, @"\s+", " "))));
+                using (HttpClient http = new HttpClient())
+                {
+                    string targets = await http.GetStringAsync(new Uri("http://127.0.0.1:" + port + "/json/list")).ConfigureAwait(false);
+                    TestContext.Out.WriteLine("targets:\n" + targets);
+                }
+
+                // Connecting should not hang on the discarded page, and the page should not be reported.
+                IBrowser cdpBrowser = await Playwright.Chromium.ConnectOverCDPAsync("http://127.0.0.1:" + port + "/").ConfigureAwait(false);
+                IReadOnlyList<IPage> pages = FirstContext(cdpBrowser).Pages;
+                Assert.That(pages.Select(page => page.Url).Order(StringComparer.Ordinal), Is.EqualTo(new[] { "about:blank", discards.Url }.Order(StringComparer.Ordinal)));
+                await cdpBrowser.CloseAsync().ConfigureAwait(false);
+            }
+            finally
+            {
+                await context.CloseAsync().ConfigureAwait(false);
+                try
+                {
+                    Directory.Delete(userDataDir, recursive: true);
+                }
+                catch (IOException)
+                {
+                }
+            }
+        }
+
         [PlaywrightTest("connect-over-cdp.spec.ts", "should cleanup artifacts dir after connectOverCDP disconnects due to ws close")]
         [Test]
         [Timeout(TestConstants.DefaultTestTimeout)]
@@ -834,12 +973,12 @@ public void ShouldConnectOverCdpUsingAConnectionTransport()
             Assert.Ignore("Official skip: Passing a transport to connectOverCDP is only available in-process");
         }
 
-        private static async Task WithHostAsync(Func<IBrowser, string, Task> body)
+        private static async Task WithHostAsync(Func<IBrowser, string, Task> body, params string[] extraArgs)
         {
             int port = FreeCdpPort();
             IBrowser host = await BrowserLauncher.LaunchAsync(new BrowserTypeLaunchOptions
             {
-                Args = new[] { "--remote-debugging-port=" + port },
+                Args = new[] { "--remote-debugging-port=" + port }.Concat(extraArgs).ToArray(),
             }).ConfigureAwait(false);
             try
             {

src/PlaywrightNative/Chromium/CRBrowser.cs

@@ -150,6 +150,12 @@ internal IReadOnlyCollection<CRBrowserContext> Contexts
         /// </summary>
         internal CRBrowserContext DefaultContext => _defaultContext;
 
+        /// <summary>
+        /// Official <c>CRBrowser._isConnecting</c>: true while
+        /// <c>connectOverCDP</c> attaches to the targets that already exist.
+        /// </summary>
+        internal bool IsConnecting { get; private set; }
+
         /// <summary>
         /// Pages currently attached to this browser.
         /// </summary>
@@ -249,18 +255,30 @@ internal static async Task<CRBrowser> ConnectAsync(
 
             // Enable auto-attach so the browser sends Target.attachedToTarget events
             // for every new target (page, worker, service worker, etc.).
-            await connection.RootSession
-                .SendAsync("Target.setAutoAttach", new
+            browser.IsConnecting = processManager == null;
+            try
+            {
+                await connection.RootSession
+                    .SendAsync("Target.setAutoAttach", new
+                    {
+                        autoAttach = true,
+                        waitForDebuggerOnStart = true,
+                        flatten = true,
+                    }).ConfigureAwait(false);
+
+                browser.FlushPendingOopifs();
+                if (processManager == null)
                 {
-                    autoAttach = true,
-                    waitForDebuggerOnStart = true,
-                    flatten = true,
-                }).ConfigureAwait(false);
+                    await browser.AdoptExistingOopifsAsync().ConfigureAwait(false);
 
-            browser.FlushPendingOopifs();
-            if (processManager == null)
+                    // Official _waitForAllPagesToBeInitialized. Pages without a
+                    // renderer report Inspector.targetCrashed and close instead.
+                    await Task.WhenAll(browser._crPages.Values.Select(page => page.InitializedTask)).ConfigureAwait(false);
+                }
+            }
+            finally
             {
-                await browser.AdoptExistingOopifsAsync().ConfigureAwait(false);
+                browser.IsConnecting = false;
             }
 
             browser._adoptingExistingTargets = false;
@@ -735,6 +753,28 @@ await _connection.RootSession.SendAsync(
             }
         }
 
+        /// <summary>
+        /// Official <c>FrameSession._onTargetCrashed</c> while connecting: an
+        /// existing page without a renderer (crashed or discarded) never
+        /// finishes initializing, so report it as closed right away.
+        /// </summary>
+        /// <param name="page">The crashed page.</param>
+        internal void ClosePageCrashedWhileConnecting(CRPage page)
+        {
+            if (!_crPages.TryRemove(page.TargetId, out _))
+            {
+                return;
+            }
+
+            _defaultContext?.RemovePage(page);
+            foreach (CRBrowserContext context in _contexts.Values)
+            {
+                context.RemovePage(page);
+            }
+
+            page.DidClose();
+        }
+
         /// <summary>
         /// Attaches a flattened CDP session to the browser target.
         /// </summary>
@@ -1135,9 +1175,16 @@ private void OnAttachedToTarget(JsonElement? parameters)
 
             // Initialize the page asynchronously (enable CDP domains).
             _ = crPage.InitializeAsync().ContinueWith(
-                t => _logger?.LogError(t.Exception, "Failed to initialize page {TargetId}", targetId),
+                t =>
+                {
+                    _logger?.LogError(t.Exception, "Failed to initialize page {TargetId}", targetId);
+
+                    // Official reportAsNew(error): a failed page still counts as
+                    // initialized, so connectOverCDP does not wait for it forever.
+                    crPage.DidFailInitialization();
+                },
                 default,
-                TaskContinuationOptions.OnlyOnFaulted,
+                TaskContinuationOptions.NotOnRanToCompletion,
                 TaskScheduler.Default);
         }
 

src/PlaywrightNative/Chromium/CRPage.cs

@@ -5220,6 +5220,12 @@ internal void DidClose()
             _closedTcs.TrySetResult(true);
         }
 
+        /// <summary>
+        /// Official <c>reportAsNew(error)</c>: completes <see cref="InitializedTask"/>
+        /// when <see cref="InitializeAsync"/> fails, so waiters do not hang.
+        /// </summary>
+        internal void DidFailInitialization() => _initializationTcs.TrySetResult(true);
+
         /// <summary>
         /// Marks this page as reported on the context <c>page</c> event.
         /// </summary>
@@ -5886,9 +5892,16 @@ private void OnInspectorTargetCrashed()
                 return;
             }
 
+            // Check before MarkCrashed: failing the pending init calls also completes InitializedTask.
+            bool closeWhileConnecting = _browser.IsConnecting && !InitializedTask.IsCompleted;
             _crashed = true;
             _client.MarkCrashed();
             Crashed?.Invoke(this, EventArgs.Empty);
+            if (closeWhileConnecting)
+            {
+                // When connecting, any crashed/discarded/unloaded page is reported as closed right away.
+                _browser.ClosePageCrashedWhileConnecting(this);
+            }
         }
 
         /// <summary>

src/PlaywrightNative/Helpers/GetByAllScript.cs

@@ -34,7 +34,8 @@ internal static class GetByAllScript
     const visit = (el) => {
         if (!el || skip[el.tagName]) return false;
         let childHit = false;
-        const children = el.children || [];
+        // Official text engine pierces open shadow roots.
+        const children = [...(el.children || []), ...(el.shadowRoot ? el.shadowRoot.children : [])];
         for (let i = 0; i < children.length; i++)
             childHit = visit(children[i]) || childHit;
         if (matches(el) && !childHit) {
@@ -67,7 +68,8 @@ internal static class GetByAllScript
     const visit = (el) => {
         if (!el || skip[el.tagName]) return false;
         let childHit = false;
-        const children = el.children || [];
+        // Official text engine pierces open shadow roots.
+        const children = [...(el.children || []), ...(el.shadowRoot ? el.shadowRoot.children : [])];
         for (let i = 0; i < children.length; i++)
             childHit = visit(children[i]) || childHit;
         if (matches(el) && !childHit) {

src/PlaywrightNative/Helpers/RoleSelectorEngine.cs

@@ -59,6 +59,7 @@ internal static class RoleSelectorEngine
     if (tag === 'OPTION') return 'option';
     if (tag === 'OUTPUT') return 'status';
     if (tag === 'LI') return 'listitem';
+    if (tag === 'TR') return 'row';
     if (tag === 'TEXTAREA') return 'textbox';
     if (tag === 'A' && element.hasAttribute('href')) return 'link';
     if (tag === 'IMG') return 'img';

src/PlaywrightNative/Page.cs

@@ -1078,7 +1078,9 @@ public Task<bool> IsVisibleAsync(string selector, float? timeout = default, bool
         /// <inheritdoc/>
         public Task<IPage> OpenerAsync()
         {
-            if (_crPage.Opener == null)
+            // An opener that closed before this page got a public wrapper (e.g. it had
+            // no renderer when connecting over CDP) has no closed public page to check.
+            if (_crPage.Opener == null || _crPage.Opener.ClosedTask.IsCompleted)
             {
                 return Task.FromResult<IPage>(null);
             }