Commit 0b128880 authored by Charlie Harrison's avatar Charlie Harrison Committed by Commit Bot

Minor popup blocker cleanups

This CL:
1. Stops null-checking the WebContents when considering to block popups.
   All callers pass in a valid contents already.

2. Tests the case when a caller calls ShowBlockedPopup with an invalid
   id. This seemed like a more reasonable API to be defensive with, but
   it wasn't tested.

Bug: None
Change-Id: I3913ed540b06f57e7fd31cae1f7da7de4da37dab
Reviewed-on: https://chromium-review.googlesource.com/1073847Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562022}
parent e04ac9ce
......@@ -332,6 +332,11 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupPositionMetrics) {
static_cast<int>(ListItemPosition::kOnlyItem), 1);
tester.ExpectTotalCount(kClickThroughPosition, 4);
// Requests to show popups not on the list should do nothing.
EXPECT_FALSE(base::ContainsValue(ids, 5));
popup_blocker->ShowBlockedPopup(5, disposition);
tester.ExpectTotalCount(kClickThroughPosition, 4);
}
IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupMetrics) {
......
......@@ -112,14 +112,13 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
NavigateParams* params,
const content::OpenURLParams* open_url_params,
const blink::mojom::WindowFeatures& window_features) {
DCHECK(web_contents);
DCHECK(!open_url_params ||
open_url_params->user_gesture == params->user_gesture);
LogAction(Action::kInitiated);
const bool user_gesture = params->user_gesture;
if (!web_contents)
return false;
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisablePopupBlocking)) {
......
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