Commit b024da66 authored by Charlie Harrison's avatar Charlie Harrison Committed by Commit Bot

Remove OpenURLParam logic from Safe Browsing popup blocker

This logic is pulled out and looked at by the popup_blocker.cc, since
we never want to block these types of popups even if we add additional
popup policy.

While we're at it, rename ShouldApplyStrongPopupBlocker to
ShouldApplyAbusivePopupBlocker, which is more precise.

Bug: None
Change-Id: Id9f31522f697b4ead95e8e23b89da325c0960f3e
Reviewed-on: https://chromium-review.googlesource.com/c/1311260Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604421}
parent 3973180d
...@@ -121,7 +121,7 @@ TEST_P(SubresourceFilterAbusiveTest, ConfigCombination) { ...@@ -121,7 +121,7 @@ TEST_P(SubresourceFilterAbusiveTest, ConfigCombination) {
SimulateNavigateAndCommit(url, main_rfh()); SimulateNavigateAndCommit(url, main_rfh());
bool disallow_requests = !CreateAndNavigateDisallowedSubframe(main_rfh()); bool disallow_requests = !CreateAndNavigateDisallowedSubframe(main_rfh());
bool disallow_popups = popup_blocker_->ShouldApplyStrongPopupBlocker(nullptr); bool disallow_popups = popup_blocker_->ShouldApplyAbusivePopupBlocker();
bool any_activation_enforce = bool any_activation_enforce =
bas_level_ == METADATA_ENFORCE || bas_level_ == METADATA_ENFORCE ||
......
...@@ -50,10 +50,18 @@ PopupBlockType ShouldBlockPopup(content::WebContents* web_contents, ...@@ -50,10 +50,18 @@ PopupBlockType ShouldBlockPopup(content::WebContents* web_contents,
if (!user_gesture) if (!user_gesture)
return PopupBlockType::kNoGesture; return PopupBlockType::kNoGesture;
// This is trusted user action (e.g. shift-click), so make sure it is not
// blocked.
if (open_url_params &&
open_url_params->triggering_event_info !=
blink::WebTriggeringEventInfo::kFromUntrustedEvent) {
return PopupBlockType::kNotBlocked;
}
auto* safe_browsing_blocker = auto* safe_browsing_blocker =
SafeBrowsingTriggeredPopupBlocker::FromWebContents(web_contents); SafeBrowsingTriggeredPopupBlocker::FromWebContents(web_contents);
if (safe_browsing_blocker && if (safe_browsing_blocker &&
safe_browsing_blocker->ShouldApplyStrongPopupBlocker(open_url_params)) { safe_browsing_blocker->ShouldApplyAbusivePopupBlocker()) {
return PopupBlockType::kAbusive; return PopupBlockType::kAbusive;
} }
return PopupBlockType::kNotBlocked; return PopupBlockType::kNotBlocked;
......
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
#include "components/safe_browsing/db/v4_protocol_manager_util.h" #include "components/safe_browsing/db/v4_protocol_manager_util.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h" #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/page_navigator.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/common/console_message_level.h" #include "content/public/common/console_message_level.h"
...@@ -89,27 +88,19 @@ void SafeBrowsingTriggeredPopupBlocker::MaybeCreate( ...@@ -89,27 +88,19 @@ void SafeBrowsingTriggeredPopupBlocker::MaybeCreate(
SafeBrowsingTriggeredPopupBlocker::~SafeBrowsingTriggeredPopupBlocker() = SafeBrowsingTriggeredPopupBlocker::~SafeBrowsingTriggeredPopupBlocker() =
default; default;
bool SafeBrowsingTriggeredPopupBlocker::ShouldApplyStrongPopupBlocker( bool SafeBrowsingTriggeredPopupBlocker::ShouldApplyAbusivePopupBlocker() {
const content::OpenURLParams* open_url_params) {
LogAction(Action::kConsidered); LogAction(Action::kConsidered);
if (!current_page_data_->is_triggered()) if (!current_page_data_->is_triggered())
return false; return false;
bool should_block = true;
if (open_url_params) {
should_block = open_url_params->triggering_event_info ==
blink::WebTriggeringEventInfo::kFromUntrustedEvent;
}
if (!IsEnabled(web_contents())) if (!IsEnabled(web_contents()))
return false; return false;
if (should_block) { LogAction(Action::kBlocked);
LogAction(Action::kBlocked); current_page_data_->inc_num_popups_blocked();
current_page_data_->inc_num_popups_blocked(); web_contents()->GetMainFrame()->AddMessageToConsole(
web_contents()->GetMainFrame()->AddMessageToConsole( content::CONSOLE_MESSAGE_LEVEL_ERROR, kAbusiveEnforceMessage);
content::CONSOLE_MESSAGE_LEVEL_ERROR, kAbusiveEnforceMessage); return true;
}
return should_block;
} }
SafeBrowsingTriggeredPopupBlocker::SafeBrowsingTriggeredPopupBlocker( SafeBrowsingTriggeredPopupBlocker::SafeBrowsingTriggeredPopupBlocker(
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
namespace content { namespace content {
struct OpenURLParams;
class WebContents; class WebContents;
} // namespace content } // namespace content
...@@ -77,8 +76,7 @@ class SafeBrowsingTriggeredPopupBlocker ...@@ -77,8 +76,7 @@ class SafeBrowsingTriggeredPopupBlocker
static void MaybeCreate(content::WebContents* web_contents); static void MaybeCreate(content::WebContents* web_contents);
~SafeBrowsingTriggeredPopupBlocker() override; ~SafeBrowsingTriggeredPopupBlocker() override;
bool ShouldApplyStrongPopupBlocker( bool ShouldApplyAbusivePopupBlocker();
const content::OpenURLParams* open_url_params);
private: private:
// The |web_contents| and |observer_manager| are expected to be // The |web_contents| and |observer_manager| are expected to be
......
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