Fix bug wth delayed warnings Safety Tips on lookalike sites

This CL fixes a bug where the delayed warnings Safety Tip feature
caused non-blocklisted lookalike sites to show "Suspicious site"
Safety Tips instead of "Did you mean..." Safety Tips. This was due to
passing |web_contents| as the |has_delayed_warning| parameter to
GetReputationStatusWithEngagedSites.

As a follow-up, it would be good to parameterize all the Safety Tips
tests to run with the delayed warnings Safety Tips feature enabled,
but I'm not doing that now because lookalike Safety Tips tests are
currently disabled due to flakiness.

Bug: 1146471
Change-Id: I369cf4570cade19ab81a921299025b755b0cc6b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531119Reviewed-by: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826565}

chrome/browser/reputation/reputation_service.cc

@@ -111,19 +111,22 @@ void ReputationService::GetReputationStatus(const GURL& url,
                                             ReputationCheckCallback callback) {
   DCHECK(url.SchemeIsHTTPOrHTTPS());
 
+  bool has_delayed_warning =
+      !!safe_browsing::SafeBrowsingUserInteractionObserver::FromWebContents(
+          web_contents);
+
   LookalikeUrlService* service = LookalikeUrlService::Get(profile_);
   if (service->EngagedSitesNeedUpdating()) {
-    service->ForceUpdateEngagedSites(base::BindOnce(
-        &ReputationService::GetReputationStatusWithEngagedSites,
-        weak_factory_.GetWeakPtr(), url,
-        !!safe_browsing::SafeBrowsingUserInteractionObserver::FromWebContents(
-            web_contents),
+    service->ForceUpdateEngagedSites(
+        base::BindOnce(&ReputationService::GetReputationStatusWithEngagedSites,
+                       weak_factory_.GetWeakPtr(), url, has_delayed_warning,
                        std::move(callback)));
     // If the engaged sites need updating, there's nothing to do until callback.
     return;
   }
 
-  GetReputationStatusWithEngagedSites(url, web_contents, std::move(callback),
+  GetReputationStatusWithEngagedSites(url, has_delayed_warning,
+                                      std::move(callback),
                                       service->GetLatestEngagedSites());
 }
 

chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

@@ -43,7 +43,10 @@
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/location_bar/location_icon_view.h"
 #include "chrome/browser/ui/views/page_info/page_info_bubble_view_base.h"
+#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
@@ -56,6 +59,7 @@
 #include "components/policy/core/common/policy_types.h"
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_service.h"
+#include "components/reputation/core/safety_tip_test_utils.h"
 #include "components/safe_browsing/content/browser/threat_details.h"
 #include "components/safe_browsing/content/common/safe_browsing.mojom.h"
 #include "components/safe_browsing/content/renderer/threat_dom_details.h"
@@ -152,6 +156,23 @@ content::RenderFrameHost* GetRenderFrameHost(Browser* browser) {
   return browser->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
 }
 
+class ClickEvent : public ui::Event {
+ public:
+  ClickEvent() : ui::Event(ui::ET_UNKNOWN, base::TimeTicks(), 0) {}
+};
+
+views::BubbleDialogDelegateView* OpenPageInfo(Browser* browser) {
+  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
+  LocationIconView* location_icon_view =
+      browser_view->toolbar()->location_bar()->location_icon_view();
+  ClickEvent event;
+  location_icon_view->ShowBubble(event);
+  views::BubbleDialogDelegateView* page_info =
+      PageInfoBubbleViewBase::GetPageInfoBubbleForTesting();
+  page_info->set_close_on_deactivate(false);
+  return page_info;
+}
+
 bool WaitForReady(Browser* browser) {
   WebContents* contents = browser->tab_strip_model()->GetActiveWebContents();
   if (!content::WaitForRenderFrameReady(contents->GetMainFrame())) {
@@ -2635,6 +2656,11 @@ class SafeBrowsingBlockingPageDelayedWarningWithSafetyTipBrowserTest
       delete;
 
  protected:
+  void SetUp() override {
+    reputation::InitializeSafetyTipConfig();
+    SafeBrowsingBlockingPageDelayedWarningBrowserTest::SetUp();
+  }
+
   void GetAdditionalFeatures(
       std::vector<base::Feature>* enabled_features,
       std::vector<base::Feature>* disabled_features) override {
@@ -2669,12 +2695,17 @@ IN_PROC_BROWSER_TEST_P(
   ui_test_utils::NavigateToURL(browser(), url);
   AssertNoInterstitial(browser(), true);
 
-  // Check that a Safety Tip is showing.
+  // Check that a "Suspicious site" Safety Tip is showing.
   EXPECT_EQ(PageInfoBubbleViewBase::BUBBLE_SAFETY_TIP,
             PageInfoBubbleViewBase::GetShownBubbleType());
   histograms.ExpectUniqueSample("Security.SafetyTips.SafetyTipShown",
                                 security_state::SafetyTipStatus::kBadReputation,
                                 1);
+  auto* page_info = OpenPageInfo(browser());
+  ASSERT_TRUE(page_info);
+  EXPECT_EQ(
+      page_info->GetWindowTitle(),
+      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE));
 
   // When the Safety Tip is showing, the security level should be downgraded, as
   // with a normal Safety Tip.
@@ -2732,6 +2763,65 @@ INSTANTIATE_TEST_SUITE_P(
         testing::Values(false, true), /* IsolateAllSitesForTesting */
         testing::Values(false, true) /* Show warning on mouse click */));
 
+// This test fixture has both regular Safety Tips and delayed warnings Safety
+// Tips enabled, to test that delayed warnings Safety Tips don't interfere with
+// regular Safety Tips on non-blocklisted pages.
+class SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest
+    : public SafeBrowsingBlockingPageDelayedWarningBrowserTest {
+ public:
+  SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest() =
+      default;
+  SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest(
+      const SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest&) =
+      delete;
+  SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest&
+  operator=(
+      const SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest&) =
+      delete;
+
+ protected:
+  void SetUp() override {
+    reputation::InitializeSafetyTipConfig();
+    SafeBrowsingBlockingPageDelayedWarningBrowserTest::SetUp();
+  }
+
+  void GetAdditionalFeatures(
+      std::vector<base::Feature>* enabled_features,
+      std::vector<base::Feature>* disabled_features) override {
+    enabled_features->push_back(
+        security_state::features::kSafetyTipUIOnDelayedWarning);
+    enabled_features->push_back(security_state::features::kSafetyTipUI);
+  }
+};
+
+// Tests that the delayed warnings Safety Tips feature does not interfere with
+// normal lookalike safety tips.
+IN_PROC_BROWSER_TEST_P(
+    SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest,
+    LookalikeSafetyTip) {
+  // Use a domain that is 1 edit distance away from a top 500 domain.
+  const GURL lookalike_url =
+      embedded_test_server()->GetURL("gooogle.com", "/iframe.html");
+  ui_test_utils::NavigateToURL(browser(), lookalike_url);
+  AssertNoInterstitial(browser(), true);
+
+  // Check that a "Did you mean..." Safety Tip is showing.
+  EXPECT_EQ(PageInfoBubbleViewBase::BUBBLE_SAFETY_TIP,
+            PageInfoBubbleViewBase::GetShownBubbleType());
+  auto* page_info = OpenPageInfo(browser());
+  ASSERT_TRUE(page_info);
+  EXPECT_EQ(page_info->GetWindowTitle(),
+            l10n_util::GetStringFUTF16(IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_TITLE,
+                                       base::ASCIIToUTF16("google.com")));
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest,
+    SafeBrowsingBlockingPageDelayedWarningWithLookalikeSafetyTipBrowserTest,
+    testing::Combine(
+        testing::Values(false, true), /* IsolateAllSitesForTesting */
+        testing::Values(false, true) /* Show warning on mouse click */));
+
 // Test that SafeBrowsingBlockingPage properly decodes IDN URLs that are
 // displayed.
 class SafeBrowsingBlockingPageIDNTest