Commit e7de93ac authored by Giovanni Ortuño Urquidi's avatar Giovanni Ortuño Urquidi Committed by Commit Bot

Revert "[SB Delayed Warnings] Allow non-character keys such as Escape"

This reverts commit e00873cb.

Reason for revert: Causing a lot of
SafeBrowsingBlockingPageDelayedWarningBrowserTest tests to fail:

https://ci.chromium.org/p/chromium/builders/ci/Mac%20ASan%2064%20Tests%20%281%29/63042

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8878647092646084928/+/steps/browser_tests/0/logs/Deterministic_failure:_SafeBrowsingBlockingPageWithDelayedWarningsBrowserTest__x2f_SafeBrowsingBlockingPageDelayedWarningBrowserTest.KeyPress_ESCAndCharacterKey_WarningShown__x2f_1__status_CRASH_/0


Original change's description:
> [SB Delayed Warnings] Allow non-character keys such as Escape
> 
> Users may sometimes press non character keys such as ESC when on a
> webpage (e.g. to close popups or other prompts). This CL ignores
> these cases and does not undelay the warning.
> 
> Bug: 1057157
> Change-Id: I4cfc2c621e4c0a84f46fdaa8f800e6843ed6b4b4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225361
> Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
> Reviewed-by: Varun Khaneja <vakh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#773833}

TBR=meacer@chromium.org,vakh@chromium.org

Change-Id: I660f1e2b2b05472df75fbe34600672d53594e95e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1057157
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225682Reviewed-by: default avatarGiovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774033}
parent 34a84164
...@@ -2028,71 +2028,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageDelayedWarningBrowserTest, ...@@ -2028,71 +2028,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageDelayedWarningBrowserTest,
DelayedWarningEvent::kWarningShownOnKeypress, 1); DelayedWarningEvent::kWarningShownOnKeypress, 1);
} }
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageDelayedWarningBrowserTest,
KeyPress_ESC_WarningNotShown) {
base::HistogramTester histograms;
NavigateAndAssertNoInterstitial();
// Press ESC key. The interstitial should not be shown.
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
content::NativeWebKeyboardEvent event(
blink::WebKeyboardEvent::Type::kRawKeyDown,
blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::GetStaticTimeStampForTests());
event.windows_key_code = ui::VKEY_ESCAPE;
contents->GetRenderViewHost()->GetWidget()->ForwardKeyboardEvent(event);
AssertNoInterstitial(browser(), false);
// Navigate to about:blank twice to "flush" metrics, if any. The delayed
// warning user interaction observer may not have been deleted after the first
// navigation.
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
histograms.ExpectTotalCount(kDelayedWarningsHistogram, 2);
histograms.ExpectBucketCount(kDelayedWarningsHistogram,
DelayedWarningEvent::kPageLoaded, 1);
histograms.ExpectBucketCount(kDelayedWarningsHistogram,
DelayedWarningEvent::kWarningNotShown, 1);
}
// Similar to KeyPress_ESC_WarningNotShown, but a character key is pressed after
// ESC. The warning should be shown.
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageDelayedWarningBrowserTest,
KeyPress_ESCAndCharacterKey_WarningShown) {
base::HistogramTester histograms;
NavigateAndAssertNoInterstitial();
// Press ESC key. The interstitial should not be shown.
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
content::NativeWebKeyboardEvent event(
blink::WebKeyboardEvent::Type::kRawKeyDown,
blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::GetStaticTimeStampForTests());
event.windows_key_code = ui::VKEY_ESCAPE;
contents->GetRenderViewHost()->GetWidget()->ForwardKeyboardEvent(event);
base::RunLoop().RunUntilIdle();
AssertNoInterstitial(browser(), false);
histograms.ExpectTotalCount(kDelayedWarningsHistogram, 1);
histograms.ExpectBucketCount(kDelayedWarningsHistogram,
DelayedWarningEvent::kPageLoaded, 1);
// Now type something. The interstitial should be shown.
EXPECT_TRUE(TypeAndWaitForInterstitial(browser()));
EXPECT_TRUE(ClickAndWaitForDetach(browser(), "primary-button"));
AssertNoInterstitial(browser(), false); // Assert the interstitial is gone
EXPECT_EQ(GURL(url::kAboutBlankURL), // Back to "about:blank"
browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
histograms.ExpectTotalCount(kDelayedWarningsHistogram, 2);
histograms.ExpectBucketCount(kDelayedWarningsHistogram,
DelayedWarningEvent::kPageLoaded, 1);
histograms.ExpectBucketCount(kDelayedWarningsHistogram,
DelayedWarningEvent::kWarningShownOnKeypress, 1);
}
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageDelayedWarningBrowserTest, IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageDelayedWarningBrowserTest,
Fullscreen_WarningShown) { Fullscreen_WarningShown) {
base::HistogramTester histograms; base::HistogramTester histograms;
......
...@@ -220,11 +220,6 @@ void SafeBrowsingUserInteractionObserver::OnDesktopCaptureRequest() { ...@@ -220,11 +220,6 @@ void SafeBrowsingUserInteractionObserver::OnDesktopCaptureRequest() {
bool SafeBrowsingUserInteractionObserver::HandleKeyPress( bool SafeBrowsingUserInteractionObserver::HandleKeyPress(
const content::NativeWebKeyboardEvent& event) { const content::NativeWebKeyboardEvent& event) {
// Allow non-character keys such as ESC. These can be used to exit fullscreen,
// for example.
if (!event.IsCharacterKey()) {
return false;
}
ShowInterstitial(DelayedWarningEvent::kWarningShownOnKeypress); ShowInterstitial(DelayedWarningEvent::kWarningShownOnKeypress);
// DO NOT add code past this point. |this| is destroyed. // DO NOT add code past this point. |this| is destroyed.
return true; return true;
......
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