Commit 896fcc61 authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

[Safety Tips] Don't go to NTP when Safety Tip shown in CCT.

The New Tab Page in CCTs doesn't work quite like you'd expect (since
they don't happen in organically), so this CL prevents using the NTP
as the default "safe page" for Safety Tips when a better
recommendation isn't available.

Instead, this CL either navigates back (if possible), or closes the CCT
(if not possible). This isn't an ideal solution either, but there
probably isn't a magical option.

Bug: 1007029
Change-Id: I63f53e361d734f4b26480467f74b182612dd210b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2334591
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794672}
parent 1d4538f1
...@@ -74,7 +74,7 @@ bool SafetyTipInfoBarDelegate::Accept() { ...@@ -74,7 +74,7 @@ bool SafetyTipInfoBarDelegate::Accept() {
action_taken_ = SafetyTipInteraction::kLeaveSite; action_taken_ = SafetyTipInteraction::kLeaveSite;
auto url = safety_tip_status_ == security_state::SafetyTipStatus::kLookalike auto url = safety_tip_status_ == security_state::SafetyTipStatus::kLookalike
? suggested_url_ ? suggested_url_
: GURL(kSafetyTipLeaveSiteUrl); : GURL();
LeaveSiteFromSafetyTip(web_contents_, url); LeaveSiteFromSafetyTip(web_contents_, url);
return true; return true;
} }
......
...@@ -14,8 +14,20 @@ ...@@ -14,8 +14,20 @@
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#if defined(OS_ANDROID)
#include "chrome/browser/android/tab_android.h"
#include "chrome/browser/ui/android/tab_model/tab_model.h"
#include "chrome/browser/ui/android/tab_model/tab_model_list.h"
#include "content/public/browser/navigation_controller.h"
#endif
namespace {
// URL that the "leave site" button aborts to by default.
const char kSafetyTipLeaveSiteUrl[] = "chrome://newtab"; const char kSafetyTipLeaveSiteUrl[] = "chrome://newtab";
} // namespace
void RecordSafetyTipInteractionHistogram(content::WebContents* web_contents, void RecordSafetyTipInteractionHistogram(content::WebContents* web_contents,
SafetyTipInteraction interaction) { SafetyTipInteraction interaction) {
SecurityStateTabHelper* helper = SecurityStateTabHelper* helper =
...@@ -32,8 +44,32 @@ void LeaveSiteFromSafetyTip(content::WebContents* web_contents, ...@@ -32,8 +44,32 @@ void LeaveSiteFromSafetyTip(content::WebContents* web_contents,
const GURL& safe_url) { const GURL& safe_url) {
RecordSafetyTipInteractionHistogram(web_contents, RecordSafetyTipInteractionHistogram(web_contents,
SafetyTipInteraction::kLeaveSite); SafetyTipInteraction::kLeaveSite);
auto navigated_to = safe_url;
if (navigated_to.is_empty()) {
navigated_to = GURL(kSafetyTipLeaveSiteUrl);
#if defined(OS_ANDROID)
if (TabAndroid::FromWebContents(web_contents)->IsCustomTab()) {
auto& controller = web_contents->GetController();
// For CCTs, just go back if we can...
if (controller.CanGoBack()) {
controller.GoBack();
return;
}
// ... or close the CCT otherwise.
auto* tab_model = TabModelList::GetTabModelForWebContents(web_contents);
if (tab_model) {
tab_model->CloseTabAt(tab_model->GetActiveIndex());
return;
}
// (And if we don't have a tab model for some reason, just navigate away
// someplace at least slightly. To my knowledge, this shouldn't happen.)
}
#endif
}
content::OpenURLParams params( content::OpenURLParams params(
safe_url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB, navigated_to, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_AUTO_TOPLEVEL, false /* is_renderer_initiated */); ui::PAGE_TRANSITION_AUTO_TOPLEVEL, false /* is_renderer_initiated */);
params.should_replace_current_entry = true; params.should_replace_current_entry = true;
web_contents->OpenURL(params); web_contents->OpenURL(params);
......
...@@ -14,16 +14,14 @@ namespace content { ...@@ -14,16 +14,14 @@ namespace content {
class WebContents; class WebContents;
} }
// URL that the "leave site" button aborts to by default.
extern const char kSafetyTipLeaveSiteUrl[];
// Records a histogram for a user's interaction with a Safety Tip in the given // Records a histogram for a user's interaction with a Safety Tip in the given
// |web_contents|. // |web_contents|.
void RecordSafetyTipInteractionHistogram(content::WebContents* web_contents, void RecordSafetyTipInteractionHistogram(content::WebContents* web_contents,
SafetyTipInteraction interaction); SafetyTipInteraction interaction);
// Invokes action when 'leave site' button is clicked, and records a histogram. // Invokes action when 'leave site' button is clicked, and records a histogram.
// Navigates to a safe URL, replacing the current page in the process. // Navigates to either |safe_url| (when !is_empty()), or a safe default
// otherwise, replacing the current page in the process.
void LeaveSiteFromSafetyTip(content::WebContents* web_contents, void LeaveSiteFromSafetyTip(content::WebContents* web_contents,
const GURL& safe_url); const GURL& safe_url);
......
...@@ -251,7 +251,7 @@ void SafetyTipPageInfoBubbleView::ButtonPressed(views::Button* button, ...@@ -251,7 +251,7 @@ void SafetyTipPageInfoBubbleView::ButtonPressed(views::Button* button,
web_contents(), web_contents(),
safety_tip_status_ == security_state::SafetyTipStatus::kLookalike safety_tip_status_ == security_state::SafetyTipStatus::kLookalike
? suggested_url_ ? suggested_url_
: GURL(kSafetyTipLeaveSiteUrl)); : GURL());
} }
void SafetyTipPageInfoBubbleView::StyledLabelLinkClicked( void SafetyTipPageInfoBubbleView::StyledLabelLinkClicked(
......
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