Safety Tips: Gracefully handle invalid flag types from the component.

This CL updates the safety tip logic to ignore elements in the component
blocklist that use unexpected flags. This allows us to gradually rollout
new flag types used in future versions of Chrome without breaking this
version (and without segmenting the component release).

Bug: 1001885
Change-Id: I9addd8d9eecafcde2014292554c34d9d71c1bd28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796113
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695438}

chrome/browser/lookalikes/safety_tips/reputation_service.cc

@@ -139,8 +139,9 @@ security_state::SafetyTipStatus FlagTypeToSafetyTipStatus(
   switch (type) {
     case FlaggedPage::FlagType::FlaggedPage_FlagType_UNKNOWN:
     case FlaggedPage::FlagType::FlaggedPage_FlagType_YOUNG_DOMAIN:
-      NOTREACHED();
-      break;
+      // Reached if component includes these flags, which might happen to
+      // support newer Chrome releases.
+      return security_state::SafetyTipStatus::kNone;
     case FlaggedPage::FlagType::FlaggedPage_FlagType_BAD_REP:
       return security_state::SafetyTipStatus::kBadReputation;
   }
@@ -264,8 +265,15 @@ security_state::SafetyTipStatus GetUrlBlockType(const GURL& url) {
           return a.pattern() < b.pattern();
         });
 
-    if (lower != flagged_pages.end() && pattern == lower->pattern()) {
-      return FlagTypeToSafetyTipStatus(lower->type());
+    while (lower != flagged_pages.end() && pattern == lower->pattern()) {
+      // Skip over sites with unexpected flag types and keep looking for other
+      // matches. This allows components to include flag types not handled by
+      // this release.
+      auto type = FlagTypeToSafetyTipStatus(lower->type());
+      if (type != security_state::SafetyTipStatus::kNone) {
+        return type;
+      }
+      ++lower;
     }
   }
 

chrome/browser/lookalikes/safety_tips/safety_tip_test_utils.cc

@@ -4,10 +4,11 @@
 
 #include "chrome/browser/lookalikes/safety_tips/safety_tip_test_utils.h"
 
-#include "chrome/browser/lookalikes/safety_tips/safety_tips.pb.h"
 #include "chrome/browser/lookalikes/safety_tips/safety_tips_config.h"
 
-void SetSafetyTipBadRepPatterns(std::vector<std::string> patterns) {
+void SetSafetyTipPatternsWithFlagType(
+    std::vector<std::string> patterns,
+    chrome_browser_safety_tips::FlaggedPage::FlagType type) {
   std::unique_ptr<chrome_browser_safety_tips::SafetyTipsConfig> config_proto =
       std::make_unique<chrome_browser_safety_tips::SafetyTipsConfig>();
   // Any version ID will do.
@@ -17,8 +18,13 @@ void SetSafetyTipBadRepPatterns(std::vector<std::string> patterns) {
     chrome_browser_safety_tips::FlaggedPage* page =
         config_proto->add_flagged_page();
     page->set_pattern(pattern);
-    page->set_type(chrome_browser_safety_tips::FlaggedPage::BAD_REP);
+    page->set_type(type);
   }
 
   safety_tips::SetRemoteConfigProto(std::move(config_proto));
 }
+
+void SetSafetyTipBadRepPatterns(std::vector<std::string> patterns) {
+  SetSafetyTipPatternsWithFlagType(
+      patterns, chrome_browser_safety_tips::FlaggedPage::BAD_REP);
+}

chrome/browser/lookalikes/safety_tips/safety_tip_test_utils.h

@@ -7,7 +7,15 @@
 
 #include <string>
 
-// Sets the patterns to trigger a bad-reputation Safety Tip for tests.
+#include "chrome/browser/lookalikes/safety_tips/safety_tips.pb.h"
+
+// Sets the patterns included in component with the given flag type for tests.
+void SetSafetyTipPatternsWithFlagType(
+    std::vector<std::string> pattern,
+    chrome_browser_safety_tips::FlaggedPage::FlagType type);
+
+// Sets the patterns to trigger a bad-reputation Safety Tip for tests. This just
+// calls SetSafetyTipPatternsWithFlagType with BAD_REPUTATION as the type.
 void SetSafetyTipBadRepPatterns(std::vector<std::string> pattern);
 
 #endif  // CHROME_BROWSER_LOOKALIKES_SAFETY_TIPS_SAFETY_TIP_TEST_UTILS_H_

chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc

@@ -563,3 +563,35 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
     EXPECT_LE(kMinWarningTime.InMilliseconds(), samples.front().min);
   }
 }
+
+// Tests that Safety Tips aren't triggered on 'unknown' flag types from the
+// component updater. This permits us to add new flag types to the component
+// without breaking this release.
+IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
+                       NotShownOnUnknownFlag) {
+  auto kNavigatedUrl = GetURL("site1.com");
+  SetSafetyTipPatternsWithFlagType(
+      {"site1.com/"}, chrome_browser_safety_tips::FlaggedPage::UNKNOWN);
+
+  SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement);
+  NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
+  EXPECT_FALSE(IsUIShowing());
+
+  ASSERT_NO_FATAL_FAILURE(CheckPageInfoDoesNotShowSafetyTipInfo(browser()));
+}
+
+// Tests that Safety Tips aren't triggered on domains flagged as 'YOUNG_DOMAIN'
+// in the component. This permits us to use this flag in the component without
+// breaking this release.
+IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
+                       NotShownOnYoungDomain) {
+  auto kNavigatedUrl = GetURL("site1.com");
+  SetSafetyTipPatternsWithFlagType(
+      {"site1.com/"}, chrome_browser_safety_tips::FlaggedPage::YOUNG_DOMAIN);
+
+  SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement);
+  NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
+  EXPECT_FALSE(IsUIShowing());
+
+  ASSERT_NO_FATAL_FAILURE(CheckPageInfoDoesNotShowSafetyTipInfo(browser()));
+}