Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revoke HandleAcceleratorKeyActivated when a WebView2 is closed #7117

Merged
merged 7 commits into from
May 26, 2022
Merged

Revoke HandleAcceleratorKeyActivated when a WebView2 is closed #7117

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
30fcae2
Move code around to match up registering and revoking
krschau May 17, 2022
1b0b2d6
Revoke m_acceleratorKeyActivatedRevoker
krschau May 17, 2022
2dad9f6
let xaml handle tab if corewebview2 is closed
krschau May 18, 2022
732376d
Add test
krschau May 18, 2022
c490475
fix part two of test, actually do the ensure
krschau May 19, 2022
6f0d595
rename test
krschau May 25, 2022
7f42cf3
update Microsoft.DotNet.Helix.Sdk
krschau May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions dev/WebView2/InteractionTests/WebView2Tests.cs
View file
Open in desktop
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", "D")]
public void LifetimeStatesTabTest()
{
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("LifetimeStatesTabTest" /* waitForLoadCompleted */);
CompleteTestAndWaitForResult("LifetimeStatesTabTest"); // 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
Open in desktop
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="LifetimeStatesTabTest">LifetimeStatesTabTest</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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ enum TestList
OffTreeWebViewInputTest,
HtmlDropdownTest,
HiddenThenVisibleTest,
ParentHiddenThenVisibleTest
ParentHiddenThenVisibleTest,
LifetimeStatesTabTest,
};

// 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.LifetimeStatesTabTest, 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.LifetimeStatesTabTest:
{
MyWebView2.Close();
}
break;

default:
break;
Expand Down
100 changes: 54 additions & 46 deletions dev/WebView2/WebView2.cpp
View file
Open in desktop
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();
}

krschau marked this conversation as resolved.
Show resolved Hide resolved
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