Commit 8521a09f authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

[Safety Tips] Better handle same-document navigations.

This CL prevents same-document navigations such as history.pushState and
fragment navigations from resulting in a closed Safety Tip.

Fixed: 1137661
Change-Id: Id0125055147d50c936c26a8763839e1f47244f8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468189Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816832}
parent 00982d27
...@@ -198,12 +198,20 @@ ReputationWebContentsObserver::~ReputationWebContentsObserver() = default; ...@@ -198,12 +198,20 @@ ReputationWebContentsObserver::~ReputationWebContentsObserver() = default;
void ReputationWebContentsObserver::DidFinishNavigation( void ReputationWebContentsObserver::DidFinishNavigation(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
if (!navigation_handle->IsInMainFrame() || if (!navigation_handle->IsInMainFrame() ||
navigation_handle->IsSameDocument() ||
!navigation_handle->HasCommitted() || navigation_handle->IsErrorPage()) { !navigation_handle->HasCommitted() || navigation_handle->IsErrorPage()) {
MaybeCallReputationCheckCallback(false); MaybeCallReputationCheckCallback(false);
return; return;
} }
// Same doc navigations keep the same status as their predecessor. Update last
// navigation entry so that GetSafetyTipInfoForVisibleNavigation() works.
if (navigation_handle->IsSameDocument()) {
last_safety_tip_navigation_entry_id_ =
web_contents()->GetController().GetLastCommittedEntry()->GetUniqueID();
MaybeCallReputationCheckCallback(false);
return;
}
last_navigation_safety_tip_info_ = {security_state::SafetyTipStatus::kUnknown, last_navigation_safety_tip_info_ = {security_state::SafetyTipStatus::kUnknown,
GURL()}; GURL()};
last_safety_tip_navigation_entry_id_ = 0; last_safety_tip_navigation_entry_id_ = 0;
......
...@@ -18,7 +18,8 @@ class GURL; ...@@ -18,7 +18,8 @@ class GURL;
// Represents the different user interactions with a Safety Tip dialog. // Represents the different user interactions with a Safety Tip dialog.
// //
// These values are persisted to logs. Entries should not be renumbered and // These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused. Keep in sync with SafetyTipStatus. // numeric values should never be reused. Keep in sync with
// SafetyTipInteraction in enums.xml.
enum class SafetyTipInteraction { enum class SafetyTipInteraction {
// The user dismissed the safety tip. Every time the user dismisses the // The user dismissed the safety tip. Every time the user dismisses the
// dialog, a histogram will be recorded once with this value, and again with a // dialog, a histogram will be recorded once with this value, and again with a
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "chrome/grit/theme_resources.h" #include "chrome/grit/theme_resources.h"
#include "components/security_state/core/security_state.h" #include "components/security_state/core/security_state.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "content/public/browser/navigation_handle.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
...@@ -250,6 +251,17 @@ void SafetyTipPageInfoBubbleView::OpenHelpCenter() { ...@@ -250,6 +251,17 @@ void SafetyTipPageInfoBubbleView::OpenHelpCenter() {
OpenHelpCenterFromSafetyTip(web_contents()); OpenHelpCenterFromSafetyTip(web_contents());
} }
void SafetyTipPageInfoBubbleView::DidStartNavigation(
content::NavigationHandle* handle) {
if (handle->IsInMainFrame() && !handle->IsSameDocument()) {
GetWidget()->CloseWithReason(views::Widget::ClosedReason::kUnspecified);
}
}
void SafetyTipPageInfoBubbleView::DidChangeVisibleSecurityState() {
// Do nothing. (Base class closes the bubble.)
}
void ShowSafetyTipDialog( void ShowSafetyTipDialog(
content::WebContents* web_contents, content::WebContents* web_contents,
security_state::SafetyTipStatus safety_tip_status, security_state::SafetyTipStatus safety_tip_status,
......
...@@ -50,12 +50,15 @@ class SafetyTipPageInfoBubbleView : public PageInfoBubbleViewBase { ...@@ -50,12 +50,15 @@ class SafetyTipPageInfoBubbleView : public PageInfoBubbleViewBase {
private: private:
friend class SafetyTipPageInfoBubbleViewBrowserTest; friend class SafetyTipPageInfoBubbleViewBrowserTest;
void OpenHelpCenter();
void ExecuteLeaveCommand(); void ExecuteLeaveCommand();
void OpenHelpCenter();
views::Button* GetLeaveButtonForTesting() { return leave_button_; } views::Button* GetLeaveButtonForTesting() { return leave_button_; }
// WebContentsObserver:
void DidStartNavigation(content::NavigationHandle* handle) override;
void DidChangeVisibleSecurityState() override;
const security_state::SafetyTipStatus safety_tip_status_; const security_state::SafetyTipStatus safety_tip_status_;
// The URL of the page the Safety Tip suggests you intended to go to, when // The URL of the page the Safety Tip suggests you intended to go to, when
......
...@@ -532,6 +532,26 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, ...@@ -532,6 +532,26 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
GURL())); GURL()));
} }
// Ensure same-document navigations don't close the Safety Tip.
// Regression test for crbug.com/1137661
IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
StillShowAfterSameDocNav) {
auto kNavigatedUrl = GetURL("site1.com");
SetSafetyTipBadRepPatterns({"site1.com/"});
// Generate a Safety Tip.
NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(IsUIShowingOrSuspiciousSitesDisabled());
// Now generate a same-document navigation and verify the tip is still there.
NavigateToURL(browser(), GURL(kNavigatedUrl.spec() + "#fragment"),
WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(IsUIShowingOrSuspiciousSitesDisabled());
ASSERT_NO_FATAL_FAILURE(CheckPageInfoShowsSafetyTipInfo(
browser(), security_state::SafetyTipStatus::kBadReputation, GURL()));
}
// Ensure explicitly-allowed sites don't get blocked when the site is otherwise // Ensure explicitly-allowed sites don't get blocked when the site is otherwise
// blocked server-side. // blocked server-side.
IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
......
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