Backport WebView2 to WinUI2 #5708

jevansaks · 2021-08-17T17:15:10Z

This is a backport of the WinUI3 control and tests into WinUI2. Notable differences from the experience you get in WinUI3 via WinAppSDK are:

This change adds a nuspec dependency from Microsoft.UI.Xaml onto WebView2's nupkg. In WinAppSDK the winrt implementation dlls are carried in the WinAppSDK runtime. We may change to do that as a follow-up.
In WinUI3 the Focus related methods were moved so that WebView2 can derive from FrameworkElement. We don't have that in OS XAML so WebView2 control derives from Control instead. 🤷

WinUI2 also compiles and supports Arm32, but WebView2 package does not because there is no WebView2 runtime for Arm32. I've asked the WebView2 folks to consider adding at least the type information (with stub implementation), but I don't know if/when that will happen. As such I have made the build conditional so that the Arm32 version does not include WebView2 control. Consumers will need to write conditional code as well if they support Arm32 and want to use WebView2.

Fixes #5610

jevansaks · 2021-08-17T17:15:19Z

/azp run

azure-pipelines · 2021-08-17T17:15:30Z

Azure Pipelines successfully started running 1 pipeline(s).

test/MUXControlsReleaseTest/LibraryThatUsesMUX/LibraryThatUsesMUX.csproj

test/MUXControlsReleaseTest/RuntimeComponentThatUsesMUX/packages.config

jevansaks · 2021-08-18T02:23:07Z

/azp run

azure-pipelines · 2021-08-18T02:23:20Z

Azure Pipelines successfully started running 1 pipeline(s).

jevansaks · 2021-08-18T04:56:00Z

/azp run

azure-pipelines · 2021-08-18T04:56:12Z

Azure Pipelines successfully started running 1 pipeline(s).

jevansaks · 2021-08-18T05:03:19Z

/azp run

azure-pipelines · 2021-08-18T05:03:31Z

Azure Pipelines successfully started running 1 pipeline(s).

asklar · 2021-08-18T17:52:34Z

dev/WebView2/InteractionTests/WebView2Tests.cs

+            {
+                if (textbox.Value != expectedValue)
+                {
+                    return;


shouldn't this assert/throw/fail the test?

Hmmm .. I don't know. This is what it looks like in WinUI3's code as well. @DmitriyKomin ?

Good catch. This function does two things (1)wait for textbox to attain a new value, and then (2) match that against expectedString (or the negation of it).

In the two places we use it, looks like it mostly just depends on part (1) - if the textbox value, changes then things are good, we don't depend on (2)... I am not sure that's true 100% of the time it's called, and also there could be new usages in the future expecting the matching part to work correctly...

In short, I agree this should be changed to return a bool reflecting the matching result.

dev/WebView2/WebView2.h

dev/WebView2/WebView2AutomationPeer.cpp

dev/WebView2/WebView2AutomationPeer.h

asklar · 2021-08-18T17:56:56Z

dev/WebView2/InteractionTests/WebView2Tests.cs

+            public static uint PW_RENDERFULLCONTENT = 2;
+            [DllImport("user32.dll")] public static extern bool PrintWindow(IntPtr hWnd, IntPtr hdc, uint flags);
+            [DllImport("user32.dll")] public static extern IntPtr GetDC(IntPtr hWnd);
+            [DllImport("user32.dll")] public static extern IntPtr ReleaseDC(IntPtr hWnd, IntPtr hdc);
+            [DllImport("user32.dll", CharSet = CharSet.Unicode)] public static extern IntPtr FindWindowEx(IntPtr parentHandle, IntPtr childAfter, string className, string windowTitle);
+            [DllImport("user32.dll")] public static extern int GetWindowRect(IntPtr hWnd, ref RECT rectangle);
+            [DllImport("user32.dll")] public static extern int GetDpiForWindow([In] IntPtr hWnd);
+
+            [StructLayout(LayoutKind.Sequential)]
+            public struct RECT
+            {
+                public int Left;        // x position of upper-left corner
+                public int Top;         // y position of upper-left corner
+                public int Right;       // x position of lower-right corner
+                public int Bottom;      // y position of lower-right corner
+            }
+
+            [DllImport("gdi32.dll")] public static extern IntPtr CreateCompatibleDC(IntPtr hdc);
+            [DllImport("gdi32.dll")] public static extern bool DeleteDC(IntPtr hdc);
+            [DllImport("gdi32.dll")] public static extern IntPtr CreateCompatibleBitmap(IntPtr hdc, int width, int height);
+            [DllImport("gdi32.dll")] public static extern IntPtr SelectObject(IntPtr hdc, IntPtr obj);
+            [DllImport("gdi32.dll")] public static extern bool DeleteObject(IntPtr obj);
+            [DllImport("gdi32.dll")] public static extern uint GetPixel(IntPtr hdc, int x, int y);
+        }


nit, we could be using win32metadata instead : )

asklar · 2021-08-18T18:01:24Z

dev/WebView2/TestUI/WebView2BasicPage.xaml.cs

+            }
+        }
+
+        private static bool AreEqualIgnoringSpace(string string1, string string2)


nit, can probably use Uri.EscapeUriString instead?

asklar · 2021-08-18T18:03:17Z

dev/WebView2/TestUI/WebView2BasicPage.xaml.cs

+                                    "(async function() { " +
+                                        "let result = await window.chrome.webview.hostObjects.bridge.AnotherObject.Prop; " +
+                                        "window.chrome.webview.postMessage(result); })();");
+                                // Listen for WebMessageReceived


mark as TODO / file an issue?

asklar · 2021-08-18T18:04:46Z

dev/WebView2/TestUI/WebView2Common.cs

+        public static Uri GetTestPageUri(string testPageName)
+        {
+            string localAppDataPath = AppDataPaths.GetDefault().LocalAppData;
+            string fullUri = @"file:///" + localAppDataPath + @"/" + testPageName;


nit you can make this a little more readable with an interpolated string

Suggested change

string fullUri = @"file:///" + localAppDataPath + @"/" + testPageName;

string fullUri = $"file:///{localAppDataPath}/{testPageName}";

asklar · 2021-08-18T18:08:41Z

dev/WebView2/WebView2.cpp

+#include <winuser.h>
+#include <uiautomationclient.h>
+
+using namespace Microsoft::WRL;


curious, could we not use wrl and use the cppwinrt winrt:: types instead?

Interesting, I suspect this is stale, since I don't recall seeing any WRL usage in this file. Let me try to remove.

asklar · 2021-08-18T18:31:59Z

dev/WebView2/WebView2.cpp

+void WebView2::CreateMissingAnaheimWarning()
+{
+    auto warning = winrt::TextBlock();
+    warning.Text(L"A suitable version of Microsoft Edge WebView2 Runtime was not detected. ");


shouldn't these strings come from resources / be localized?

asklar · 2021-08-18T18:35:03Z

dev/WebView2/WebView2.cpp

+    float leftVal = inputProperties.ContactRect().X * m_rasterizationScale;
+    float topVal = inputProperties.ContactRect().Y * m_rasterizationScale;
+
+    winrt::Windows::Foundation::Rect outputPt_touchContact(static_cast<float>(leftVal), static_cast<float>(topVal), static_cast<float>(width), static_cast<float>(height));


these are already floats, can remove the casts. same below.

dev/WebView2/WebView2.cpp

asklar · 2021-08-18T18:40:28Z

dev/WebView2/WebView2.cpp

+        LRESULT result = ::SendMessageW(GetActiveInputWindowHwnd(), message, wparam, lparam);
+        if (!result)
+        {
+            winrt::check_hresult(HRESULT_FROM_WIN32(::GetLastError()));


noexcept with check_hresult #ByDesign

dev/dll/Microsoft.UI.Xaml.vcxproj

jevansaks · 2021-08-18T21:00:33Z

/azp run

azure-pipelines · 2021-08-18T21:00:44Z

Azure Pipelines successfully started running 1 pipeline(s).

jevansaks · 2021-08-19T03:05:39Z

/azp run

azure-pipelines · 2021-08-19T03:05:51Z

Azure Pipelines successfully started running 1 pipeline(s).

jevansaks · 2021-08-19T23:36:34Z

/azp run

azure-pipelines · 2021-08-19T23:36:46Z

Azure Pipelines successfully started running 1 pipeline(s).

dev/WebView2/WebView2.cpp

DmitriyKomin · 2021-08-20T18:51:47Z

dev/WebView2/WebView2.cpp

+        {
+            auto coreWindowInterop = coreWindow.as<ICoreWindowInterop>();
+            winrt::check_hresult(coreWindowInterop->get_WindowHandle(&m_xamlHostHwnd));
+        }


What happens in the context of OS Xaml islands / DWXS? I belive the CoreWindow will exist, but is not used for rendering... WebView2 will fail in some way. Should we early out to avoid unsafe behavior? Also, is there an issue/task to track WebView2 in island?

This is just to get it working in UWP. We can have a follow-up to figure out how (if at all) to make it work in islands. I suspect there will be other things that need to be done.

agree with getting it to work on UWP first and figuring out islands post "1.0".

Having said that, an error message would be good if possible :)

dev/WebView2/WebView2.cpp

dev/WebView2/WebView2.h

DmitriyKomin · 2021-08-20T20:00:12Z

dev/WebView2/WebView2.cpp

+    if (m_visual)
+    {
+        winrt::float2 newSize = winrt::float2(width * m_rasterizationScale, height * m_rasterizationScale);
+        winrt::float3 newScale = winrt::float3(1.0f / m_rasterizationScale, 1.0f / m_rasterizationScale, 1.0);


Extract duplicatd code #WontFix

DmitriyKomin · 2021-08-20T20:10:48Z

dev/WebView2/WebView2.cpp

+    outputMatrix.m44 = 1.0f;
+
+    return outputMatrix;
+}


This is no longer used (was needed for older input forwarding API). Please remove.

How about you remove in WinUI3 first and then i can copy those deltas over here?

dev/WebView2/WebView2.h

dev/WebView2/WebView2.cpp

asklar · 2021-08-20T20:53:23Z

dev/WebView2/WebView2.cpp

+WebView2::WebView2()
+{
+    if (HMODULE user32module = LoadLibraryW(L"user32.dll"))
+    {


nit, GetModuleHandle since user32 will have already been loaded (also gets rid of needing to free the hmodule) #Resolved

DmitriyKomin · 2021-08-20T21:02:22Z

                string installerArgs_WV2Runtime = "/silent /install";

remove comments since we aren't useing mini_installer here

In reply to: 902955384

Refers to: dev/WebView2/InteractionTests/WebView2Tests.cs:147 in 1ffea83. [](commit_id = 1ffea83, deletion_comment = False)

asklar

main remaining thing for me is to please make sure the noexcept/throws are reasonable (i.e. does it make sense to [not] bubble exceptions up to the app)

DmitriyKomin · 2021-08-20T23:35:00Z

Per discussion with @jevansaks and @asklar , some of the comments made by @asklar and myself are existing issues in WinUI3 codebase and that are lower priority. I will capture these for fixing in WinUI3 codebase (and porting the changes back to WinUI2).

jevansaks · 2021-08-20T23:42:19Z

/azp run

azure-pipelines · 2021-08-20T23:42:30Z

Azure Pipelines successfully started running 1 pipeline(s).

jevansaks · 2021-08-21T20:35:48Z

/azp run

azure-pipelines · 2021-08-21T20:35:59Z

Azure Pipelines successfully started running 1 pipeline(s).

EP012014 · 2021-08-23T00:52:29Z

Congratulations!

michael-hawker · 2021-08-23T07:35:11Z

How's the extra dependency going to work for an app that doesn't use WebView2? Will it be optimized out or increase the footprint of all apps using WinUI 2?

jevansaks · 2021-08-23T17:59:29Z

How's the extra dependency going to work for an app that doesn't use WebView2? Will it be optimized out or increase the footprint of all apps using WinUI 2?

The dlls are relatively small (~500KB). For now it's getting copied into the consuming apps. We are discussing alternatives, since WinUI3 is already doing this slightly differently by including the WebView2 client-side dlls in the WinAppSDK framework package.

jevansaks added 2 commits August 13, 2021 13:18

WebView2 backport

798d5fc

Bump webview2 version to latest

d9325e0

jevansaks requested review from ranjeshj and codendone August 17, 2021 17:15

ghost added the needs-triage Issue needs to be triaged by the area owners label Aug 17, 2021

ranjeshj added area-WebView team-Rendering Issue for the Rendering team labels Aug 17, 2021

kmahone reviewed Aug 17, 2021

View reviewed changes

test/MUXControlsReleaseTest/LibraryThatUsesMUX/LibraryThatUsesMUX.csproj Outdated Show resolved Hide resolved

kmahone reviewed Aug 17, 2021

View reviewed changes

test/MUXControlsReleaseTest/RuntimeComponentThatUsesMUX/packages.config Outdated Show resolved Hide resolved

Attempt to fix NavigationStartingTest with retry logic

43daf1a

Disable one test

9282c18

Disable another test temporarily

7c92981

asklar reviewed Aug 18, 2021

View reviewed changes

jonwis reviewed Aug 18, 2021

View reviewed changes

dev/WebView2/WebView2.h Show resolved Hide resolved

dev/WebView2/WebView2AutomationPeer.cpp Outdated Show resolved Hide resolved

dev/WebView2/WebView2AutomationPeer.cpp Show resolved Hide resolved

dev/WebView2/WebView2AutomationPeer.h Outdated Show resolved Hide resolved

asklar reviewed Aug 18, 2021

View reviewed changes

jevansaks added 2 commits August 18, 2021 13:20

PR feedback

176698d

Try fixing the release test project references

54f15a1

Update package reference version

3932498

DmitriyKomin reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.cpp Outdated Show resolved Hide resolved

DmitriyKomin reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.cpp Show resolved Hide resolved

DmitriyKomin reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.cpp Show resolved Hide resolved

DmitriyKomin reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.cpp Show resolved Hide resolved

DmitriyKomin reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.h Show resolved Hide resolved

DmitriyKomin reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.h Outdated Show resolved Hide resolved

asklar reviewed Aug 20, 2021

View reviewed changes

dev/WebView2/WebView2.cpp Show resolved Hide resolved

asklar reviewed Aug 20, 2021

View reviewed changes

asklar approved these changes Aug 20, 2021

View reviewed changes

DmitriyKomin approved these changes Aug 20, 2021

View reviewed changes

PR feedback

135b2af

Remove unneeded using.

dc9bb4c

jevansaks merged commit 0bd0d22 into main Aug 22, 2021

jevansaks deleted the user/jevansaks/webview2 branch August 22, 2021 20:01

codendone removed the needs-triage Issue needs to be triaged by the area owners label Sep 2, 2021

robloo mentioned this pull request Nov 4, 2021

👨‍💻☎ [Cancelled] WinUI Community Call: October 20, 2021 #6050

Closed

	string fullUri = @"file:///" + localAppDataPath + @"/" + testPageName;
	string fullUri = $"file:///{localAppDataPath}/{testPageName}";

Backport WebView2 to WinUI2 #5708

Backport WebView2 to WinUI2 #5708

Conversation

jevansaks commented Aug 17, 2021 • edited Loading

jevansaks commented Aug 17, 2021

azure-pipelines bot commented Aug 17, 2021

jevansaks commented Aug 18, 2021

azure-pipelines bot commented Aug 18, 2021

jevansaks commented Aug 18, 2021

azure-pipelines bot commented Aug 18, 2021

jevansaks commented Aug 18, 2021

azure-pipelines bot commented Aug 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asklar Aug 18, 2021 • edited by jevansaks Loading

Choose a reason for hiding this comment

jevansaks commented Aug 18, 2021

azure-pipelines bot commented Aug 18, 2021

jevansaks commented Aug 19, 2021

azure-pipelines bot commented Aug 19, 2021

jevansaks commented Aug 19, 2021

azure-pipelines bot commented Aug 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyKomin Aug 20, 2021 • edited by jevansaks Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asklar Aug 20, 2021 • edited Loading

Choose a reason for hiding this comment

DmitriyKomin commented Aug 20, 2021 • edited by jevansaks Loading

asklar left a comment

Choose a reason for hiding this comment

DmitriyKomin commented Aug 20, 2021 • edited Loading

jevansaks commented Aug 20, 2021

azure-pipelines bot commented Aug 20, 2021

jevansaks commented Aug 21, 2021

azure-pipelines bot commented Aug 21, 2021

EP012014 commented Aug 23, 2021

michael-hawker commented Aug 23, 2021

jevansaks commented Aug 23, 2021

jevansaks commented Aug 17, 2021 •

edited

Loading

asklar Aug 18, 2021 •

edited by jevansaks

Loading

DmitriyKomin Aug 20, 2021 •

edited by jevansaks

Loading

asklar Aug 20, 2021 •

edited

Loading

DmitriyKomin commented Aug 20, 2021 •

edited by jevansaks

Loading

DmitriyKomin commented Aug 20, 2021 •

edited

Loading