-
Notifications
You must be signed in to change notification settings - Fork 2k
[Android] Fix RenderThread SIGSEGV crash with multiple auto-sizing WebViews in ScrollView #35822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 35771, "Android SIGSEGV crash with multiple auto-sizing WebViews in ScrollView on navigated page", PlatformAffected.Android)] | ||
| public class Issue35771 : NavigationPage | ||
| { | ||
| public Issue35771() : base(new Issue35771HomePage()) { } | ||
|
|
||
| class Issue35771HomePage : ContentPage | ||
| { | ||
| public Issue35771HomePage() | ||
| { | ||
| Content = new VerticalStackLayout | ||
| { | ||
| VerticalOptions = LayoutOptions.Center, | ||
| Children = | ||
| { | ||
| new Button | ||
| { | ||
| AutomationId = "Issue35771NavigateButton", | ||
| Text = "Open repro page", | ||
| Command = new Command(async () => await Navigation.PushAsync(new Issue35771ReproPage())) | ||
| }, | ||
| new Button | ||
| { | ||
| AutomationId = "Issue35771HorizontalNavigateButton", | ||
| Text = "Open horizontal repro page", | ||
| Command = new Command(async () => await Navigation.PushAsync(new Issue35771HorizontalReproPage())) | ||
| }, | ||
| new Button | ||
| { | ||
| AutomationId = "Issue35771PopAsyncNavigateButton", | ||
| Text = "Open PopAsync repro page", | ||
| Command = new Command(async () => await Navigation.PushAsync(new Issue35771PopAsyncReproPage())) | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| class Issue35771ReproPage : ContentPage | ||
| { | ||
| public Issue35771ReproPage() | ||
| { | ||
| var stack = new VerticalStackLayout | ||
| { | ||
| Children = | ||
| { | ||
| new Label | ||
| { | ||
| AutomationId = "Issue35771Ready", | ||
| Text = "Page loaded — no crash" | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| for (int i = 1; i <= 6; i++) | ||
| { | ||
| stack.Children.Add(new WebView | ||
| { | ||
| Source = new HtmlWebViewSource { Html = $"<html><body><p>WebView {i}</p></body></html>" } | ||
| }); | ||
| } | ||
|
|
||
| Content = new ScrollView { Content = stack }; | ||
| } | ||
| } | ||
|
|
||
| // Reproduces the horizontal auto-sizing crash scenario (width == 0, height > 0): | ||
| // WebViews inside a HorizontalStackLayout have a fixed HeightRequest but no WidthRequest, | ||
| // so the first layout pass produces (w=0, h>0) on the native view. | ||
| class Issue35771HorizontalReproPage : ContentPage | ||
| { | ||
| public Issue35771HorizontalReproPage() | ||
| { | ||
| var stack = new HorizontalStackLayout(); | ||
|
|
||
| for (int i = 1; i <= 6; i++) | ||
| { | ||
| stack.Children.Add(new WebView | ||
| { | ||
| HeightRequest = 200, | ||
| Source = new HtmlWebViewSource { Html = $"<html><body><p>WebView {i}</p></body></html>" } | ||
| }); | ||
| } | ||
|
|
||
| Content = new ScrollView | ||
| { | ||
| Orientation = ScrollOrientation.Horizontal, | ||
| Content = new VerticalStackLayout | ||
| { | ||
| Children = | ||
| { | ||
| new Label | ||
| { | ||
| AutomationId = "Issue35771HorizontalReady", | ||
| Text = "Horizontal page loaded — no crash" | ||
| }, | ||
| stack | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| // Reproduces the PopAsync crash scenario: popping a page while WebViews are still in | ||
| // the dangerous (w>0, h=0) first-layout state previously caused a RenderThread SIGSEGV. | ||
| class Issue35771PopAsyncReproPage : ContentPage | ||
| { | ||
| public Issue35771PopAsyncReproPage() | ||
| { | ||
| var stack = new VerticalStackLayout | ||
| { | ||
| Children = | ||
| { | ||
| new Label | ||
| { | ||
| AutomationId = "Issue35771PopAsyncReady", | ||
| Text = "Page loaded — tap button to pop" | ||
| }, | ||
| new Button | ||
| { | ||
| AutomationId = "Issue35771PopAsyncPopButton", | ||
| Text = "Pop back", | ||
| Command = new Command(async () => await Navigation.PopAsync()) | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| for (int i = 1; i <= 6; i++) | ||
| { | ||
| stack.Children.Add(new WebView | ||
| { | ||
| Source = new HtmlWebViewSource { Html = $"<html><body><p>WebView {i}</p></body></html>" } | ||
| }); | ||
| } | ||
|
|
||
| Content = new ScrollView { Content = stack }; | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // Crash is Android-specific: RenderThread GL functor receives zero-area Skia canvas when ClipBounds=(0,0,0,0) at (w>0,h=0) | ||
| #if ANDROID | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue35771 : _IssuesUITest | ||
| { | ||
| public Issue35771(TestDevice device) : base(device) { } | ||
|
|
||
| public override string Issue => "Android SIGSEGV crash with multiple auto-sizing WebViews in ScrollView on navigated page"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.WebView)] | ||
| public void MultipleAutoSizingWebViewsInScrollViewShouldNotCrash() | ||
| { | ||
| App.WaitForElement("Issue35771NavigateButton"); | ||
| App.Tap("Issue35771NavigateButton"); | ||
| App.WaitForElement("Issue35771Ready"); | ||
| App.Back(); | ||
| App.WaitForElement("Issue35771NavigateButton"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.WebView)] | ||
| public void HorizontalAutoSizingWebViewsShouldNotCrash() | ||
| { | ||
| App.WaitForElement("Issue35771HorizontalNavigateButton"); | ||
| App.Tap("Issue35771HorizontalNavigateButton"); | ||
| App.WaitForElement("Issue35771HorizontalReady"); | ||
| App.Back(); | ||
| App.WaitForElement("Issue35771NavigateButton"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.WebView)] | ||
| public void PopAsyncFromAutoSizingWebViewPageShouldNotCrash() | ||
| { | ||
| App.WaitForElement("Issue35771PopAsyncNavigateButton"); | ||
| App.Tap("Issue35771PopAsyncNavigateButton"); | ||
| App.WaitForElement("Issue35771PopAsyncReady"); | ||
| App.Tap("Issue35771PopAsyncPopButton"); | ||
| App.WaitForElement("Issue35771NavigateButton"); | ||
| } | ||
| } | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,12 @@ public class MauiWebView : WebView, IWebViewDelegate | |
| readonly WebViewHandler _handler; | ||
| readonly Rect _clipRect; | ||
|
|
||
| // True after the first layout pass where exactly one dimension is positive and the other is zero. | ||
| // Auto-sizing layouts produce this intermediate state; a zero-area ClipBounds here | ||
| // causes RenderThread to crash on an incomplete Skia canvas (SIGSEGV). | ||
| // https://github.com/dotnet/maui/issues/35771 | ||
| bool _isAutoSizing; | ||
|
|
||
| public MauiWebView(WebViewHandler handler, Context context) : base(context) | ||
| { | ||
| _handler = handler ?? throw new ArgumentNullException(nameof(handler)); | ||
|
|
@@ -40,6 +46,20 @@ protected override void OnAttachedToWindow() | |
|
|
||
| void UpdateClipBounds(int width, int height) | ||
| { | ||
| // Auto-sizing layouts produce an intermediate layout pass where exactly one dimension | ||
| // is positive and the other is zero: vertical layouts give (w>0, h=0) first; horizontal | ||
| // layouts give (w=0, h>0) first. A zero-area ClipBounds in either state causes | ||
| // RenderThread to crash (SIGSEGV). Null disables clipping; the latch prevents later | ||
| // layout passes from re-enabling it before both dimensions are stable. | ||
| // https://github.com/dotnet/maui/issues/35771 | ||
| if (_isAutoSizing || (width > 0 && height == 0) || (width == 0 && height > 0)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [major] Logic and Correctness — Once a WebView sees one partial-size pass, |
||
| { | ||
| _isAutoSizing = true; | ||
| ClipBounds = null; | ||
| return; | ||
| } | ||
|
|
||
| // Normal (non-auto-sizing) WebView: apply flash prevention from issue #31475. | ||
| if (width > 0 && height > 0) | ||
| { | ||
| if (Parent is WrapperView) | ||
|
|
@@ -59,7 +79,7 @@ void UpdateClipBounds(int width, int height) | |
| } | ||
| else | ||
| { | ||
| // Re-apply empty clip bounds when the view becomes zero-sized or hidden. | ||
| // View has no area yet or is fully collapsed — keep a zero clip rect. | ||
| _clipRect.Set(0, 0, 0, 0); | ||
| ClipBounds = _clipRect; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Logic and Correctness — Once a HybridWebView sees one partial-size pass,
_isAutoSizingstays true forever, so later stable(width > 0 && height > 0)and fully collapsed(0,0)states keepClipBounds = nullinstead of restoring the exact/zero clip rect. Concrete regression: an auto-sizing HybridWebView that later reaches a normal non-WrapperView size permanently loses the flash-prevention clipping added for #31475/#33207/#35057; if it later collapses to(0,0), it also skips the existing zero-clip behavior. This only needs transient suppression for the partial-size state.