Commit 122062be authored by Ella Ge's avatar Ella Ge Committed by Commit Bot

Deflaky SitePerProcessHitTestBrowserTest.RenderWidgetUserActivationStateTest

The test was flaky failing with RemovePendingUserActivationIfAvailable true
and HasTransientUserActivation false.
It's probably because of timing issue (InputEventAck arrive sooner than
the UpdateUserActivationState IPC).
This CL changes to wait until the target frame get activated.

This CL also adds the test to INSTANTIATE_TEST_SUITE_P. (The test wasn't
actually running after the crrev.com/c/1850736 fixed the scope feature list)


Bug: 995285
Change-Id: I5651115487d7dd1beba51933ec67dbc3d38c28e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869072
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarMustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709797}
parent 8bd5dae9
...@@ -786,6 +786,34 @@ class SitePerProcessNonIntegerScaleFactorHitTestBrowserTest ...@@ -786,6 +786,34 @@ class SitePerProcessNonIntegerScaleFactorHitTestBrowserTest
} }
}; };
//
// SitePerProcessUserActivationHitTestBrowserTest
//
class SitePerProcessUserActivationHitTestBrowserTest
: public SitePerProcessHitTestBrowserTest {
public:
SitePerProcessUserActivationHitTestBrowserTest() {}
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
SitePerProcessBrowserTest::SetUpCommandLine(command_line);
ui::PlatformEventSource::SetIgnoreNativePlatformEvents(true);
if (std::get<0>(GetParam()) == HitTestType::kDrawQuad) {
feature_list_.InitAndEnableFeature(
features::kBrowserVerifiedUserActivation);
// Default enabled.
} else if (std::get<0>(GetParam()) == HitTestType::kSurfaceLayer) {
feature_list_.InitWithFeatures({features::kEnableVizHitTestSurfaceLayer,
features::kBrowserVerifiedUserActivation},
{});
}
}
private:
base::test::ScopedFeatureList feature_list_;
};
// Restrict to Aura to we can use routable MouseWheel event via // Restrict to Aura to we can use routable MouseWheel event via
// RenderWidgetHostViewAura::OnScrollEvent(). // RenderWidgetHostViewAura::OnScrollEvent().
#if defined(USE_AURA) #if defined(USE_AURA)
...@@ -6575,29 +6603,8 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest, ...@@ -6575,29 +6603,8 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
} }
} }
class SitePerProcessHitTestBrowserTestWithBrowserVerifiedUserActivation IN_PROC_BROWSER_TEST_P(SitePerProcessUserActivationHitTestBrowserTest,
: public SitePerProcessHitTestBrowserTest { RenderWidgetUserActivationStateTest) {
public:
SitePerProcessHitTestBrowserTestWithBrowserVerifiedUserActivation() {
feature_list_.InitAndEnableFeature(
features::kBrowserVerifiedUserActivation);
}
private:
base::test::ScopedFeatureList feature_list_;
};
#if defined(OS_LINUX)
// Test is flaky. See https://crbug.com/995285
#define MAYBE_RenderWidgetUserActivationStateTest \
DISABLED_RenderWidgetUserActivationStateTest
#else
#define MAYBE_RenderWidgetUserActivationStateTest \
RenderWidgetUserActivationStateTest
#endif
IN_PROC_BROWSER_TEST_P(
SitePerProcessHitTestBrowserTestWithBrowserVerifiedUserActivation,
MAYBE_RenderWidgetUserActivationStateTest) {
GURL main_url(embedded_test_server()->GetURL( GURL main_url(embedded_test_server()->GetURL(
"foo.com", "/frame_tree/page_with_positioned_frame.html")); "foo.com", "/frame_tree/page_with_positioned_frame.html"));
EXPECT_TRUE(NavigateToURL(shell(), main_url)); EXPECT_TRUE(NavigateToURL(shell(), main_url));
...@@ -6635,19 +6642,21 @@ IN_PROC_BROWSER_TEST_P( ...@@ -6635,19 +6642,21 @@ IN_PROC_BROWSER_TEST_P(
DispatchMouseEventAndWaitUntilDispatch(web_contents(), mouse_event, rwhv_root, DispatchMouseEventAndWaitUntilDispatch(web_contents(), mouse_event, rwhv_root,
click_point, rwhv_root, click_point); click_point, rwhv_root, click_point);
EXPECT_TRUE(main_frame_monitor.EventWasReceived()); EXPECT_TRUE(main_frame_monitor.EventWasReceived());
base::RunLoop().RunUntilIdle();
// Root frame pending activation state has been cleared by activation // Wait for root frame gets activated.
// notification, and it has user activation. while (!root->HasTransientUserActivation()) {
base::RunLoop loop;
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
loop.QuitClosure());
loop.Run();
}
// Child frame doesn't have user activation.
EXPECT_FALSE(child->HasTransientUserActivation());
// Root frame's pending activation state has been cleared by activation.
EXPECT_FALSE(root->current_frame_host() EXPECT_FALSE(root->current_frame_host()
->GetRenderWidgetHost() ->GetRenderWidgetHost()
->RemovePendingUserActivationIfAvailable()); ->RemovePendingUserActivationIfAvailable());
EXPECT_TRUE(root->HasTransientUserActivation());
// Child frame doesn't have allowed_activation state set, and does not have
// user activation.
EXPECT_FALSE(child->current_frame_host()
->GetRenderWidgetHost()
->RemovePendingUserActivationIfAvailable());
EXPECT_FALSE(child->HasTransientUserActivation());
// Clear the activation state. // Clear the activation state.
root->UpdateUserActivationState( root->UpdateUserActivationState(
...@@ -6660,19 +6669,24 @@ IN_PROC_BROWSER_TEST_P( ...@@ -6660,19 +6669,24 @@ IN_PROC_BROWSER_TEST_P(
rwhv_child, click_point, rwhv_child, rwhv_child, click_point, rwhv_child,
click_point); click_point);
EXPECT_TRUE(child_frame_monitor.EventWasReceived()); EXPECT_TRUE(child_frame_monitor.EventWasReceived());
base::RunLoop().RunUntilIdle();
// Child frame's activation state has been cleared by // Wait for child frame to get activated.
// the activation notification, and it has user activation. while (!child->HasTransientUserActivation()) {
EXPECT_FALSE(child->current_frame_host() base::RunLoop loop;
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
loop.QuitClosure());
loop.Run();
}
// With UAV2, ancestor frames get activated too.
EXPECT_TRUE(root->HasTransientUserActivation());
// Both child frame and root frame don't have allowed_activation state
EXPECT_FALSE(root->current_frame_host()
->GetRenderWidgetHost() ->GetRenderWidgetHost()
->RemovePendingUserActivationIfAvailable()); ->RemovePendingUserActivationIfAvailable());
EXPECT_TRUE(child->HasTransientUserActivation()); EXPECT_FALSE(child->current_frame_host()
// Root frame doesn't have allowed_activation state set, but has user
// activation because with UAv2, ancestor frames get activated as well.
EXPECT_FALSE(root->current_frame_host()
->GetRenderWidgetHost() ->GetRenderWidgetHost()
->RemovePendingUserActivationIfAvailable()); ->RemovePendingUserActivationIfAvailable());
EXPECT_TRUE(root->HasTransientUserActivation());
} }
class SitePerProcessHitTestDataGenerationBrowserTest class SitePerProcessHitTestDataGenerationBrowserTest
...@@ -7162,6 +7176,10 @@ INSTANTIATE_TEST_SUITE_P(/* no prefix */, ...@@ -7162,6 +7176,10 @@ INSTANTIATE_TEST_SUITE_P(/* no prefix */,
SitePerProcessHitTestBrowserTest, SitePerProcessHitTestBrowserTest,
testing::Combine(testing::ValuesIn(kHitTestOption), testing::Combine(testing::ValuesIn(kHitTestOption),
testing::ValuesIn(kOneScale))); testing::ValuesIn(kOneScale)));
INSTANTIATE_TEST_SUITE_P(/* no prefix */,
SitePerProcessUserActivationHitTestBrowserTest,
testing::Combine(testing::ValuesIn(kHitTestOption),
testing::ValuesIn(kOneScale)));
// TODO(wjmaclean): Since the next two test fixtures only differ in DSF // TODO(wjmaclean): Since the next two test fixtures only differ in DSF
// values, should we combine them into one using kMultiScale? This // values, should we combine them into one using kMultiScale? This
// approach would make it more difficult to disable individual scales on // approach would make it more difficult to disable individual scales on
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment