From 30fcae262753096da6ca1c46a2cb3bae946d01d0 Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Tue, 17 May 2022 13:15:42 -0700 Subject: [PATCH 1/7] Move code around to match up registering and revoking --- dev/WebView2/WebView2.cpp | 90 +++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/dev/WebView2/WebView2.cpp b/dev/WebView2/WebView2.cpp index ab65c1ac59..9fbc591e6b 100644 --- a/dev/WebView2/WebView2.cpp +++ b/dev/WebView2/WebView2.cpp @@ -74,29 +74,6 @@ void WebView2::OnVisibilityPropertyChanged(const winrt::DependencyObject& /*send UpdateRenderedSubscriptionAndVisibility(); } -void WebView2::UnregisterCoreEventHandlers() -{ - if (m_coreWebView) - { - m_coreNavigationStartingRevoker.revoke(); - m_coreSourceChangedRevoker.revoke(); - m_coreNavigationCompletedRevoker.revoke(); - m_coreWebMessageReceivedRevoker.revoke(); - m_coreProcessFailedRevoker.revoke(); - } - - if (m_coreWebViewController) - { - m_coreMoveFocusRequestedRevoker.revoke(); - m_coreLostFocusRevoker.revoke(); - } - - if (m_coreWebViewCompositionController) - { - m_cursorChangedRevoker.revoke(); - } -} - // Public Close() API void WebView2::Close() { @@ -557,6 +534,28 @@ void WebView2::RegisterCoreEventHandlers() FireWebMessageReceived(args); }}); + m_coreProcessFailedRevoker = m_coreWebView.ProcessFailed(winrt::auto_revoke, { + [this](auto const&, winrt::CoreWebView2ProcessFailedEventArgs const& args) + { + winrt::CoreWebView2ProcessFailedKind coreProcessFailedKind{ args.ProcessFailedKind() }; + if (coreProcessFailedKind == winrt::CoreWebView2ProcessFailedKind::BrowserProcessExited) + { + m_isCoreFailure_BrowserExited_State = true; + + // CoreWebView2 takes care of clearing the event handlers when closing the host, + // but we still need to reset the event tokens + UnregisterCoreEventHandlers(); + + // Null these out so we can't try to use them anymore + m_coreWebViewCompositionController = nullptr; + m_coreWebViewController = nullptr; + m_coreWebView = nullptr; + ResetProperties(); + } + + FireCoreProcessFailedEvent(args); + } }); + m_coreMoveFocusRequestedRevoker = m_coreWebViewController.MoveFocusRequested(winrt::auto_revoke, { [this](auto const&, const winrt::CoreWebView2MoveFocusRequestedEventArgs& args) { @@ -635,28 +634,6 @@ void WebView2::RegisterCoreEventHandlers() m_webHasFocus = false; }}); - m_coreProcessFailedRevoker = m_coreWebView.ProcessFailed(winrt::auto_revoke, { - [this](auto const&, winrt::CoreWebView2ProcessFailedEventArgs const& args) - { - winrt::CoreWebView2ProcessFailedKind coreProcessFailedKind{ args.ProcessFailedKind() }; - if (coreProcessFailedKind == winrt::CoreWebView2ProcessFailedKind::BrowserProcessExited) - { - m_isCoreFailure_BrowserExited_State = true; - - // CoreWebView2 takes care of clearing the event handlers when closing the host, - // but we still need to reset the event tokens - UnregisterCoreEventHandlers(); - - // Null these out so we can't try to use them anymore - m_coreWebViewCompositionController = nullptr; - m_coreWebViewController = nullptr; - m_coreWebView = nullptr; - ResetProperties(); - } - - FireCoreProcessFailedEvent(args); - }}); - m_cursorChangedRevoker = m_coreWebViewCompositionController.CursorChanged(winrt::auto_revoke, { [this](auto const& controller, auto const& obj) { @@ -666,6 +643,29 @@ void WebView2::RegisterCoreEventHandlers() }}); } +void WebView2::UnregisterCoreEventHandlers() +{ + if (m_coreWebView) + { + m_coreNavigationStartingRevoker.revoke(); + m_coreSourceChangedRevoker.revoke(); + m_coreNavigationCompletedRevoker.revoke(); + m_coreWebMessageReceivedRevoker.revoke(); + m_coreProcessFailedRevoker.revoke(); + } + + if (m_coreWebViewController) + { + m_coreMoveFocusRequestedRevoker.revoke(); + m_coreLostFocusRevoker.revoke(); + } + + if (m_coreWebViewCompositionController) + { + m_cursorChangedRevoker.revoke(); + } +} + winrt::IAsyncAction WebView2::CreateCoreObjects() { MUX_ASSERT((m_isImplicitCreationInProgress && !m_isExplicitCreationInProgress) || From 1b0b2d603c151faeda0569b817a95b9669caee86 Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Tue, 17 May 2022 13:16:20 -0700 Subject: [PATCH 2/7] Revoke m_acceleratorKeyActivatedRevoker --- dev/WebView2/WebView2.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dev/WebView2/WebView2.cpp b/dev/WebView2/WebView2.cpp index 9fbc591e6b..fdcd90f2d5 100644 --- a/dev/WebView2/WebView2.cpp +++ b/dev/WebView2/WebView2.cpp @@ -101,6 +101,13 @@ void WebView2::CloseInternal(bool inShutdownPath) m_visibilityChangedToken.value = 0; } + // TODO: We do not have direct analogue for AcceleratorKeyActivated with DispatcherQueue in Islands/ win32. Please refer Task# 30013704 for more details. + if (auto coreWindow = winrt::CoreWindow::GetForCurrentThread()) + { + m_inputWindowHwnd = nullptr; + m_acceleratorKeyActivatedRevoker.revoke(); + } + if (m_coreWebView) { m_coreWebView = nullptr; From 2dad9f66c7263831ec6594061321453979fd1611 Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Wed, 18 May 2022 09:55:30 -0700 Subject: [PATCH 3/7] let xaml handle tab if corewebview2 is closed --- dev/WebView2/WebView2.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev/WebView2/WebView2.cpp b/dev/WebView2/WebView2.cpp index fdcd90f2d5..ce9b9b0656 100644 --- a/dev/WebView2/WebView2.cpp +++ b/dev/WebView2/WebView2.cpp @@ -1334,9 +1334,10 @@ void WebView2::MoveFocusIntoCoreWebView(winrt::CoreWebView2MoveFocusReason reaso // Xaml control and force HWND focus back to itself, popping Xaml focus out of the // WebView2 control. We mark TAB handled in our KeyDown handler so that it is ignored // by XamlRoot's tab processing. +// If the WebView2 has been closed, then we should let Xaml's tab processing handle it. void WebView2::HandleKeyDown(const winrt::Windows::Foundation::IInspectable&, const winrt::KeyRoutedEventArgs& e) { - if (e.Key() == winrt::VirtualKey::Tab) + if (e.Key() == winrt::VirtualKey::Tab && !m_isClosed) { e.Handled(true); } From 732376d632129d87fe2d24b3b5f733abfc347ba7 Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Wed, 18 May 2022 10:31:21 -0700 Subject: [PATCH 4/7] Add test --- .../InteractionTests/WebView2Tests.cs | 67 +++++++++++++++++++ dev/WebView2/TestUI/WebView2BasicPage.xaml | 1 + dev/WebView2/TestUI/WebView2BasicPage.xaml.cs | 9 ++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/dev/WebView2/InteractionTests/WebView2Tests.cs b/dev/WebView2/InteractionTests/WebView2Tests.cs index e4944ade48..24ee590f28 100644 --- a/dev/WebView2/InteractionTests/WebView2Tests.cs +++ b/dev/WebView2/InteractionTests/WebView2Tests.cs @@ -2202,6 +2202,73 @@ public void ParentHiddenThenVisibleTest() } } + [TestMethod] + [TestProperty("TestSuite", "D")] + public void LifetimeStatesTabTest() + { + using (var setup = new WebView2TestSetupHelper(new[] { "WebView2 Tests", "navigateToBasicWebView2" })) + { + Button x1 = new Button(FindElement.ById("TabStopButton1")); // Xaml TabStop 1 + Button x2 = new Button(FindElement.ById("TabStopButton2")); // Xaml TabStop 2 + + // Part 1: Tab with no core webview + + Log.Comment("Part 1: Tab with no core webview"); + Log.Comment("Focus on x1"); + x1.SetFocus(); + Wait.ForIdle(); + Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); + + Log.Comment("Tab x1 -> webview -> x2"); + using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) + { + KeyboardHelper.PressKey(Key.Tab); + KeyboardHelper.PressKey(Key.Tab); + xamlFocusWaiter.Wait(); + Log.Comment("Focus is on " + UIObject.Focused); + Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); + } + + // Part 2: Tab with core webview, ensured but not navigated + + Log.Comment("Part 2: Tab with core webview, not navigated"); + Log.Comment("Focus on x1"); + x1.SetFocus(); + Wait.ForIdle(); + Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); + + Log.Comment("Tab x1 -> webview -> x2"); + using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) + { + KeyboardHelper.PressKey(Key.Tab); + KeyboardHelper.PressKey(Key.Tab); + xamlFocusWaiter.Wait(); + Log.Comment("Focus is on " + UIObject.Focused); + Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); + } + + // Part 3: Tab with closed core webview + + Log.Comment("Part 3: Tab with closed core webview"); + ChooseTest("LifetimeStatesTabTest"); // Creates the CoreWebView2 + CompleteTestAndWaitForResult("LifetimeStatesTabTest"); // Closes the CoreWebView2 + Log.Comment("Focus on x1"); + x1.SetFocus(); + Wait.ForIdle(); + Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); + + Log.Comment("Tab x1 -> webview -> x2"); + using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) + { + KeyboardHelper.PressKey(Key.Tab); + KeyboardHelper.PressKey(Key.Tab); + xamlFocusWaiter.Wait(); + Log.Comment("Focus is on " + UIObject.Focused); + Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); + } + } + } + private static void BeginSubTest(string testName, string testDescription) { Log.Comment(Environment.NewLine + testName + ": " + testDescription); diff --git a/dev/WebView2/TestUI/WebView2BasicPage.xaml b/dev/WebView2/TestUI/WebView2BasicPage.xaml index b2e7fc2cd7..75e6512080 100644 --- a/dev/WebView2/TestUI/WebView2BasicPage.xaml +++ b/dev/WebView2/TestUI/WebView2BasicPage.xaml @@ -45,6 +45,7 @@ HiddenThenVisibleTest HostNameToFolderMappingTest HtmlDropdownTest + LifetimeStatesTabTest MouseCaptureTest MouseLeftClickTest MouseMiddleClickTest diff --git a/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs b/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs index c84cd9f8d1..5e4c4ef590 100644 --- a/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs +++ b/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs @@ -198,7 +198,8 @@ enum TestList OffTreeWebViewInputTest, HtmlDropdownTest, HiddenThenVisibleTest, - ParentHiddenThenVisibleTest + ParentHiddenThenVisibleTest, + LifetimeStatesTabTest, }; // Map of TestList entry to its webpage (index in TestPageNames[]) @@ -264,6 +265,7 @@ enum TestList { TestList.HtmlDropdownTest, 5 }, { TestList.HiddenThenVisibleTest, 1 }, { TestList.ParentHiddenThenVisibleTest, 1 }, + { TestList.LifetimeStatesTabTest, 0 }, }; readonly string[] TestPageNames = @@ -1905,6 +1907,11 @@ async public void CompleteCurrentTest(object sender, RoutedEventArgs args) selectedTest, MyWebView2.IsHitTestVisible)); } break; + case TestList.LifetimeStatesTabTest: + { + MyWebView2.Close(); + } + break; default: break; From c49047536867060c05eb29c63653cd340c9be932 Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Thu, 19 May 2022 15:00:26 -0700 Subject: [PATCH 5/7] fix part two of test, actually do the ensure --- .../InteractionTests/WebView2Tests.cs | 78 ++++++++----------- 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/dev/WebView2/InteractionTests/WebView2Tests.cs b/dev/WebView2/InteractionTests/WebView2Tests.cs index 24ee590f28..a5f51a9978 100644 --- a/dev/WebView2/InteractionTests/WebView2Tests.cs +++ b/dev/WebView2/InteractionTests/WebView2Tests.cs @@ -402,6 +402,12 @@ private static void WaitForLoadCompleted() WaitForTextBoxValue(status2, string.Empty, false /* match = false for not match */); } + private static void WaitForEnsureCompleted() + { + var status1 = new Edit(FindElement.ById("Status1")); + WaitForTextBoxValue(status1, "EnsureCoreWebView2Async() completed", true); + } + private static void ChooseTest(string testName, bool waitForLoadCompleted = true) { WaitForCopyCompleted(); @@ -2208,64 +2214,46 @@ public void LifetimeStatesTabTest() { using (var setup = new WebView2TestSetupHelper(new[] { "WebView2 Tests", "navigateToBasicWebView2" })) { - Button x1 = new Button(FindElement.ById("TabStopButton1")); // Xaml TabStop 1 - Button x2 = new Button(FindElement.ById("TabStopButton2")); // Xaml TabStop 2 - // Part 1: Tab with no core webview Log.Comment("Part 1: Tab with no core webview"); - Log.Comment("Focus on x1"); - x1.SetFocus(); - Wait.ForIdle(); - Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); - - Log.Comment("Tab x1 -> webview -> x2"); - using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) - { - KeyboardHelper.PressKey(Key.Tab); - KeyboardHelper.PressKey(Key.Tab); - xamlFocusWaiter.Wait(); - Log.Comment("Focus is on " + UIObject.Focused); - Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); - } + TabTwicePastWebView2(); // Part 2: Tab with core webview, ensured but not navigated Log.Comment("Part 2: Tab with core webview, not navigated"); - Log.Comment("Focus on x1"); - x1.SetFocus(); - Wait.ForIdle(); - Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); - - Log.Comment("Tab x1 -> webview -> x2"); - using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) - { - KeyboardHelper.PressKey(Key.Tab); - KeyboardHelper.PressKey(Key.Tab); - xamlFocusWaiter.Wait(); - Log.Comment("Focus is on " + UIObject.Focused); - Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); - } + Button ensureButton = new Button(FindElement.ById("EnsureCWV2Button")); + ensureButton.Invoke(); + WaitForEnsureCompleted(); + TabTwicePastWebView2(); // Part 3: Tab with closed core webview Log.Comment("Part 3: Tab with closed core webview"); - ChooseTest("LifetimeStatesTabTest"); // Creates the CoreWebView2 + ChooseTest("LifetimeStatesTabTest" /* waitForLoadCompleted */); CompleteTestAndWaitForResult("LifetimeStatesTabTest"); // Closes the CoreWebView2 - Log.Comment("Focus on x1"); - x1.SetFocus(); - Wait.ForIdle(); - Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); + TabTwicePastWebView2(); + } + } - Log.Comment("Tab x1 -> webview -> x2"); - using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) - { - KeyboardHelper.PressKey(Key.Tab); - KeyboardHelper.PressKey(Key.Tab); - xamlFocusWaiter.Wait(); - Log.Comment("Focus is on " + UIObject.Focused); - Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); - } + private static void TabTwicePastWebView2() + { + Button x1 = new Button(FindElement.ById("TabStopButton1")); // Xaml TabStop 1 + Button x2 = new Button(FindElement.ById("TabStopButton2")); // Xaml TabStop 2 + + Log.Comment("Focus on x1"); + x1.SetFocus(); + Wait.ForIdle(); + Verify.IsTrue(x1.HasKeyboardFocus, "TabStopButton1 has keyboard focus"); + + Log.Comment("Tab x1 -> webview -> x2"); + using (var xamlFocusWaiter = new FocusAcquiredWaiter("TabStopButton2")) + { + KeyboardHelper.PressKey(Key.Tab); + KeyboardHelper.PressKey(Key.Tab); + xamlFocusWaiter.Wait(); + Log.Comment("Focus is on " + UIObject.Focused); + Verify.IsTrue(x2.HasKeyboardFocus, "TabStopButton2 has keyboard focus"); } } From 6f0d5950548eda6b27258ed3a289fb172ab972ff Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Wed, 25 May 2022 10:27:20 -0700 Subject: [PATCH 6/7] rename test --- dev/WebView2/InteractionTests/WebView2Tests.cs | 8 ++++---- dev/WebView2/TestUI/WebView2BasicPage.xaml | 2 +- dev/WebView2/TestUI/WebView2BasicPage.xaml.cs | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dev/WebView2/InteractionTests/WebView2Tests.cs b/dev/WebView2/InteractionTests/WebView2Tests.cs index a5f51a9978..0fe9f5ee54 100644 --- a/dev/WebView2/InteractionTests/WebView2Tests.cs +++ b/dev/WebView2/InteractionTests/WebView2Tests.cs @@ -2209,8 +2209,8 @@ public void ParentHiddenThenVisibleTest() } [TestMethod] - [TestProperty("TestSuite", "D")] - public void LifetimeStatesTabTest() + [TestProperty("TestSuite", "A")] + public void LifetimeTabTest() { using (var setup = new WebView2TestSetupHelper(new[] { "WebView2 Tests", "navigateToBasicWebView2" })) { @@ -2230,8 +2230,8 @@ public void LifetimeStatesTabTest() // Part 3: Tab with closed core webview Log.Comment("Part 3: Tab with closed core webview"); - ChooseTest("LifetimeStatesTabTest" /* waitForLoadCompleted */); - CompleteTestAndWaitForResult("LifetimeStatesTabTest"); // Closes the CoreWebView2 + ChooseTest("LifetimeTabTest" /* waitForLoadCompleted */); + CompleteTestAndWaitForResult("LifetimeTabTest"); // Closes the CoreWebView2 TabTwicePastWebView2(); } } diff --git a/dev/WebView2/TestUI/WebView2BasicPage.xaml b/dev/WebView2/TestUI/WebView2BasicPage.xaml index 75e6512080..e299507a5c 100644 --- a/dev/WebView2/TestUI/WebView2BasicPage.xaml +++ b/dev/WebView2/TestUI/WebView2BasicPage.xaml @@ -45,7 +45,7 @@ HiddenThenVisibleTest HostNameToFolderMappingTest HtmlDropdownTest - LifetimeStatesTabTest + LifetimeTabTest MouseCaptureTest MouseLeftClickTest MouseMiddleClickTest diff --git a/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs b/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs index 5e4c4ef590..4c22563980 100644 --- a/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs +++ b/dev/WebView2/TestUI/WebView2BasicPage.xaml.cs @@ -199,7 +199,7 @@ enum TestList HtmlDropdownTest, HiddenThenVisibleTest, ParentHiddenThenVisibleTest, - LifetimeStatesTabTest, + LifetimeTabTest, }; // Map of TestList entry to its webpage (index in TestPageNames[]) @@ -265,7 +265,7 @@ enum TestList { TestList.HtmlDropdownTest, 5 }, { TestList.HiddenThenVisibleTest, 1 }, { TestList.ParentHiddenThenVisibleTest, 1 }, - { TestList.LifetimeStatesTabTest, 0 }, + { TestList.LifetimeTabTest, 0 }, }; readonly string[] TestPageNames = @@ -1907,7 +1907,7 @@ async public void CompleteCurrentTest(object sender, RoutedEventArgs args) selectedTest, MyWebView2.IsHitTestVisible)); } break; - case TestList.LifetimeStatesTabTest: + case TestList.LifetimeTabTest: { MyWebView2.Close(); } From 7f42cf3dcc8b124b8b0e0c7fa2ff94968ce3d77d Mon Sep 17 00:00:00 2001 From: Kristen Schau Date: Wed, 25 May 2022 13:06:57 -0700 Subject: [PATCH 7/7] update Microsoft.DotNet.Helix.Sdk --- build/Helix/global.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Helix/global.json b/build/Helix/global.json index afa5e25ad1..a31d6c281f 100644 --- a/build/Helix/global.json +++ b/build/Helix/global.json @@ -3,6 +3,6 @@ "version": "3.1" }, "msbuild-sdks": { - "Microsoft.DotNet.Helix.Sdk": "6.0.0-beta.20529.1" + "Microsoft.DotNet.Helix.Sdk": "7.0.0-beta.22275.2" } } \ No newline at end of file