diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index e65952d5..eb6a7968 100644 --- a/.github/workflows/dotnet.yml +++ b/.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: diff --git a/src/PlaywrightNative.Tests/LibraryConnectOverCdpParityTests.cs b/src/PlaywrightNative.Tests/LibraryConnectOverCdpParityTests.cs index 469e5652..8029d0c9 100644 --- a/src/PlaywrightNative.Tests/LibraryConnectOverCdpParityTests.cs +++ b/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 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 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 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 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 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 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 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 body) + private static async Task WithHostAsync(Func 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 { diff --git a/src/PlaywrightNative/Chromium/CRBrowser.cs b/src/PlaywrightNative/Chromium/CRBrowser.cs index ab9f1bb9..85dc7fbf 100644 --- a/src/PlaywrightNative/Chromium/CRBrowser.cs +++ b/src/PlaywrightNative/Chromium/CRBrowser.cs @@ -150,6 +150,12 @@ internal IReadOnlyCollection Contexts /// internal CRBrowserContext DefaultContext => _defaultContext; + /// + /// Official CRBrowser._isConnecting: true while + /// connectOverCDP attaches to the targets that already exist. + /// + internal bool IsConnecting { get; private set; } + /// /// Pages currently attached to this browser. /// @@ -249,18 +255,30 @@ internal static async Task 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( } } + /// + /// Official FrameSession._onTargetCrashed while connecting: an + /// existing page without a renderer (crashed or discarded) never + /// finishes initializing, so report it as closed right away. + /// + /// The crashed page. + 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(); + } + /// /// Attaches a flattened CDP session to the browser target. /// @@ -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); } diff --git a/src/PlaywrightNative/Chromium/CRPage.cs b/src/PlaywrightNative/Chromium/CRPage.cs index a3064605..8bbc1e6d 100644 --- a/src/PlaywrightNative/Chromium/CRPage.cs +++ b/src/PlaywrightNative/Chromium/CRPage.cs @@ -5220,6 +5220,12 @@ internal void DidClose() _closedTcs.TrySetResult(true); } + /// + /// Official reportAsNew(error): completes + /// when fails, so waiters do not hang. + /// + internal void DidFailInitialization() => _initializationTcs.TrySetResult(true); + /// /// Marks this page as reported on the context page event. /// @@ -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); + } } /// diff --git a/src/PlaywrightNative/Helpers/GetByAllScript.cs b/src/PlaywrightNative/Helpers/GetByAllScript.cs index c8394d8c..199989ef 100644 --- a/src/PlaywrightNative/Helpers/GetByAllScript.cs +++ b/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) { diff --git a/src/PlaywrightNative/Helpers/RoleSelectorEngine.cs b/src/PlaywrightNative/Helpers/RoleSelectorEngine.cs index 3832e005..d599bce5 100644 --- a/src/PlaywrightNative/Helpers/RoleSelectorEngine.cs +++ b/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'; diff --git a/src/PlaywrightNative/Page.cs b/src/PlaywrightNative/Page.cs index 5b018218..96aab757 100644 --- a/src/PlaywrightNative/Page.cs +++ b/src/PlaywrightNative/Page.cs @@ -1078,7 +1078,9 @@ public Task IsVisibleAsync(string selector, float? timeout = default, bool /// public Task 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(null); }