Skip to content

Commit

Permalink
Revoke HandleAcceleratorKeyActivated when a WebView2 is closed (#7117)
Browse files Browse the repository at this point in the history 
* Move code around to match up registering and revoking

* Revoke m_acceleratorKeyActivatedRevoker

* let xaml handle tab if corewebview2 is closed

* Add test

* update Microsoft.DotNet.Helix.Sdk

Co-authored-by: Kristen Schau <[email protected]>
  • Loading branch information
@krschau
krschau and krschau authored May 26, 2022
1 parent 5116379 commit 0b6e07d
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 48 deletions.
2 changes: 1 addition & 1 deletion build/Helix/global.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
55 changes: 55 additions & 0 deletions dev/WebView2/InteractionTests/WebView2Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -2202,6 +2208,55 @@ public void ParentHiddenThenVisibleTest()
}
}

[TestMethod]
[TestProperty("TestSuite", "A")]
public void LifetimeTabTest()
{
using (var setup = new WebView2TestSetupHelper(new[] { "WebView2 Tests", "navigateToBasicWebView2" }))
{
// Part 1: Tab with no core webview

Log.Comment("Part 1: Tab with no core webview");
TabTwicePastWebView2();

// Part 2: Tab with core webview, ensured but not navigated

Log.Comment("Part 2: Tab with core webview, not navigated");
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("LifetimeTabTest" /* waitForLoadCompleted */);
CompleteTestAndWaitForResult("LifetimeTabTest"); // Closes the CoreWebView2
TabTwicePastWebView2();
}
}

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");
}
}

private static void BeginSubTest(string testName, string testDescription)
{
Log.Comment(Environment.NewLine + testName + ": " + testDescription);
Expand Down
1 change: 1 addition & 0 deletions dev/WebView2/TestUI/WebView2BasicPage.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
<ComboBoxItem AutomationProperties.Name="HiddenThenVisibleTest">HiddenThenVisibleTest</ComboBoxItem>
<ComboBoxItem AutomationProperties.Name="HostNameToFolderMappingTest">HostNameToFolderMappingTest</ComboBoxItem>
<ComboBoxItem AutomationProperties.Name="HtmlDropdownTest">HtmlDropdownTest</ComboBoxItem>
<ComboBoxItem AutomationProperties.Name="LifetimeTabTest">LifetimeTabTest</ComboBoxItem>
<ComboBoxItem AutomationProperties.Name="MouseCaptureTest">MouseCaptureTest</ComboBoxItem>
<ComboBoxItem AutomationProperties.Name="MouseLeftClickTest">MouseLeftClickTest</ComboBoxItem>
<ComboBoxItem AutomationProperties.Name="MouseMiddleClickTest">MouseMiddleClickTest</ComboBoxItem>
Expand Down
9 changes: 8 additions & 1 deletion dev/WebView2/TestUI/WebView2BasicPage.xaml.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ enum TestList
OffTreeWebViewInputTest,
HtmlDropdownTest,
HiddenThenVisibleTest,
ParentHiddenThenVisibleTest
ParentHiddenThenVisibleTest,
LifetimeTabTest,
};

// Map of TestList entry to its webpage (index in TestPageNames[])
Expand Down Expand Up @@ -264,6 +265,7 @@ enum TestList
{ TestList.HtmlDropdownTest, 5 },
{ TestList.HiddenThenVisibleTest, 1 },
{ TestList.ParentHiddenThenVisibleTest, 1 },
{ TestList.LifetimeTabTest, 0 },
};

readonly string[] TestPageNames =
Expand Down Expand Up @@ -1905,6 +1907,11 @@ async public void CompleteCurrentTest(object sender, RoutedEventArgs args)
selectedTest, MyWebView2.IsHitTestVisible));
}
break;
case TestList.LifetimeTabTest:
{
MyWebView2.Close();
}
break;

default:
break;
Expand Down
100 changes: 54 additions & 46 deletions dev/WebView2/WebView2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -124,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;
Expand Down Expand Up @@ -557,6 +541,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)
{
Expand Down Expand Up @@ -635,28 +641,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)
{
Expand All @@ -666,6 +650,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) ||
Expand Down Expand Up @@ -1327,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);
}
Expand Down

0 comments on commit 0b6e07d

Please sign in to comment.