Add ability to set WebView ExecutionMode property (addresses #4720) #12509

t-johnson · 2020-10-16T15:28:16Z

Description of Change

Add platform specific to set the ExecutionMode on a WebView on UWP

When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds. This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website.

This change is related to issue #4720, however I do not believe that issue is actually a bug. This change does nothing to address a "memory leak" directly, and due to the addition of the new protected member, I wasn't sure if this is classed as an "API change" but thought it would be better to be on the safe side...

Issues Resolved

partially fixes UWP: Webview: Memory Leak #4720

API Changes

Added:

        public static readonly BindableProperty ExecutionModeProperty = BindableProperty.Create("ExecutionMode", typeof(WebViewExecutionMode), typeof(WebView), WebViewExecutionMode.SameThread);
	public enum WebViewExecutionMode
	{
		SameThread = 0,
		SeparateThread = 1,
		SeparateProcess = 2
	}

Xamarin.Forms.Platform.UWP.WebViewRenderer

protected WebViewExecutionMode ExecutionMode

Changed:
None

Removed:
None

Platforms Affected

UWP

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Simply opening and closing a view that contains a WebView, pointed at something like youtube.com will cause huge memory usage which is not immediately reclaimed. When the WebView is running in SeparateProcess mode, there is a child process that is created that handles the memory itself (see ExecutionMode property

PR Checklist

Targets the correct branch
Tests are passing (or failures are unrelated)

PureWeen

It looks like this PR is having some issues with Issue12134.CookiesCorrectlyLoadWithMultipleWebViews()

  at Xamarin.Forms.Controls.Issues.Issue12134.CookiesCorrectlyLoadWithMultipleWebViews() in D:\agent\1\s\Xamarin.Forms.Controls.Issues\Xamarin.Forms.Controls.Issues.Shared\Issue12134.cs:line 120

t-johnson · 2020-11-17T03:31:23Z

@PureWeen I'll take a look and see if I can figure out the test failure.

PureWeen · 2020-11-24T18:15:14Z

@t-johnson

I rebased this PR (hopefully that doesn't mess you up)
I added some instructions (let me know if those seem correct)

Xamarin.Forms.Platform.UAP/WebViewRenderer.cs

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue4720.cs

PureWeen · 2020-11-24T18:36:12Z

/azp run

azure-pipelines · 2020-11-24T18:36:18Z

No pipelines are associated with this pull request.

PureWeen

I couldn't quite get it to crash even when loading YouTube a bunch of times. I did notice that after awhile it would start to lag. For now I changed the UI Test to just use Microsoft and reload the page ever 200 Ms

Waiting 2 seconds and reloading it 10 times just makes this test a bit too slow. I also just converted this property over to a platform specific so users can opt into it if they want to from the xplat code opposed to a renderer

PureWeen · 2020-12-03T18:17:17Z

/azp run

azure-pipelines · 2020-12-03T18:17:25Z

No pipelines are associated with this pull request.

t-johnson

thanks, much cleaner to have the platform specific

PureWeen · 2020-12-04T02:16:09Z

/azp run

azure-pipelines · 2020-12-04T02:16:18Z

No pipelines are associated with this pull request.

…4720) (xamarin#12509) * To address issue xamarin#4720: Add option for custom renderers to control the execution mode of the WebView control This is 'opt-in' as by default this commit will not change behavior of existing applciations. to opt-in, people would need to set the ExecutionMode property in the constructor of their custom WebViewRenderer, like: public class MyWebViewRenderer : WebViewRenderer { public MyWebViewRenderer() { ExecutionMode = Windows.UI.Xaml.Controls.WebViewExecutionMode.SeparateProcess; } } When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds. This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website. * tabs not spaces * TemplatedItemsList: Ensure items are unhooked correctly when removing them, whether as individual removals or list resets. Without this, the native cells do not actually get removed. CellTableViewCell: use event PropertyChangedEventHandler instead of action (seems more standard) TextCellRenderer: Correct the event delegate hookup ListViewRenderer: Actually re-use the UITableViewCell when creating header sections, otherwise we endlessly create new ones, leaving the old ones alive through event handlers * Revert "TemplatedItemsList: Ensure items are unhooked correctly when removing them, whether as individual removals or list resets. Without this, the native cells do not actually get removed." This reverts commit 7da9ffb. * Make the custom renderer only apply to the test for this issue, as it seems the SeparateProcess affects access to cookies, breaking other tests * Move additional classes inside test class to decrease namespace cluttering. * - cleanup and rebase * - add instructions * - move WebViewExecutionMode to platform specific * - fix up UI Test * - clean up tests * - fix tabs * - fix formatting * - fix teardown call * - fix async ui test quirk on uwp Co-authored-by: Shane Neuville <shneuvil@microsoft.com>

t-johnson changed the title ~~4720webview~~ Add ability to set WebView ExecutionMode property (addresses #4720) Oct 16, 2020

t-johnson requested a review from StephaneDelcroix as a code owner October 19, 2020 03:00

samhouts added ControlGallery p/UWP a/performance a/webview partner/cat 😻 t/bug 🐛 labels Oct 28, 2020

samhouts added this to In Review in vNext+1 (5.0.0) Nov 2, 2020

rmarinho added this to In progress in v5.0.1 via automation Nov 3, 2020

rmarinho removed this from In Review in vNext+1 (5.0.0) Nov 3, 2020

rmarinho moved this from In progress to Review in progress in v5.0.1 Nov 3, 2020

rmarinho added this to the 5.0.1 milestone Nov 3, 2020

samhouts added this to In Review in vNext+1 (5.0.0) Nov 3, 2020

samhouts removed this from Review in progress in v5.0.1 Nov 3, 2020

PureWeen added this to In progress in v5.0.1 via automation Nov 5, 2020

PureWeen removed this from In Review in vNext+1 (5.0.0) Nov 5, 2020

PureWeen added this to To do in vNext+1 (5.0.0) via automation Nov 5, 2020

PureWeen removed this from In progress in v5.0.1 Nov 5, 2020

PureWeen modified the milestones: 5.0.1, 5.0.0 Nov 5, 2020

PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 5, 2020

rmarinho moved this from To do to In Review in vNext+1 (5.0.0) Nov 5, 2020

rmarinho requested review from PureWeen and hartez November 6, 2020 11:36

rmarinho assigned hartez and PureWeen Nov 13, 2020

PureWeen removed the request for review from StephaneDelcroix November 16, 2020 14:44

PureWeen suggested changes Nov 16, 2020

View reviewed changes

PureWeen force-pushed the 4720webview branch from c8530ec to 7f21cc4 Compare November 24, 2020 18:06

- add instructions

fa88f5f

PureWeen reviewed Nov 24, 2020

View reviewed changes

Xamarin.Forms.Platform.UAP/WebViewRenderer.cs Outdated Show resolved Hide resolved

PureWeen reviewed Nov 24, 2020

View reviewed changes

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue4720.cs Outdated Show resolved Hide resolved

xamarin deleted a comment from azure-pipelines bot Nov 24, 2020

rmarinho requested a review from PureWeen November 27, 2020 13:18

- move WebViewExecutionMode to platform specific

ce8068d

PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Dec 3, 2020

PureWeen added 5 commits December 3, 2020 12:04

- fix up UI Test

93cfdbe

- clean up tests

943bd20

- fix tabs

6840c16

- fix formatting

e94ef42

- fix teardown call

fa96fb7

PureWeen approved these changes Dec 3, 2020

View reviewed changes

PureWeen removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Dec 3, 2020

hartez approved these changes Dec 3, 2020

View reviewed changes

t-johnson commented Dec 4, 2020

View reviewed changes

PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Dec 4, 2020

- fix async ui test quirk on uwp

cf0a7f3

rmarinho merged commit c80a908 into xamarin:5.0.0 Dec 4, 2020

vNext+1 (5.0.0) automation moved this from In Review to Done Dec 4, 2020

PureWeen mentioned this pull request Feb 11, 2022

Remove the ExecutionMode Platform Specific from WebView dotnet/maui#4638

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to set WebView ExecutionMode property (addresses #4720) #12509

Add ability to set WebView ExecutionMode property (addresses #4720) #12509

t-johnson commented Oct 16, 2020 •

edited by PureWeen

Loading

PureWeen left a comment

t-johnson commented Nov 17, 2020

PureWeen commented Nov 24, 2020

PureWeen commented Nov 24, 2020

azure-pipelines bot commented Nov 24, 2020

PureWeen left a comment

PureWeen commented Dec 3, 2020

azure-pipelines bot commented Dec 3, 2020

t-johnson left a comment

PureWeen commented Dec 4, 2020

azure-pipelines bot commented Dec 4, 2020

Add ability to set WebView ExecutionMode property (addresses #4720) #12509

Add ability to set WebView ExecutionMode property (addresses #4720) #12509

Conversation

t-johnson commented Oct 16, 2020 • edited by PureWeen Loading

Description of Change

Issues Resolved

API Changes

Platforms Affected

Behavioral/Visual Changes

Before/After Screenshots

Testing Procedure

PR Checklist

PureWeen left a comment

Choose a reason for hiding this comment

t-johnson commented Nov 17, 2020

PureWeen commented Nov 24, 2020

PureWeen commented Nov 24, 2020

azure-pipelines bot commented Nov 24, 2020

PureWeen left a comment

Choose a reason for hiding this comment

PureWeen commented Dec 3, 2020

azure-pipelines bot commented Dec 3, 2020

t-johnson left a comment

Choose a reason for hiding this comment

PureWeen commented Dec 4, 2020

azure-pipelines bot commented Dec 4, 2020

t-johnson commented Oct 16, 2020 •

edited by PureWeen

Loading