[BrowserSwitcher] Add URL Checker to internals page

It's a text input field. When the user enters a URL in that field,
a JSON object is displayed, explaining the decision + reason LBS
would make for that URL.

https://i.imgur.com/geKj4Gh.png

Bug: 959379
Change-Id: Ia79577f6135f9203832123b13a4f77f1b4b4f31b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1601645Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658549}

chrome/browser/browser_switcher/browser_switcher_navigation_throttle_unittest.cc

@@ -39,7 +39,7 @@ class MockBrowserSwitcherSitelist : public BrowserSwitcherSitelist {
   MockBrowserSwitcherSitelist() = default;
   ~MockBrowserSwitcherSitelist() override = default;
 
-  MOCK_CONST_METHOD1(ShouldSwitch, bool(const GURL&));
+  MOCK_CONST_METHOD1(GetDecision, Decision(const GURL&));
   MOCK_METHOD1(SetIeemSitelist, void(ParsedXml&&));
   MOCK_METHOD1(SetExternalSitelist, void(ParsedXml&&));
   MOCK_CONST_METHOD0(GetIeemSitelist, const RuleSet*());
@@ -82,8 +82,11 @@ class BrowserSwitcherNavigationThrottleTest
   MockBrowserSwitcherSitelist* sitelist_;
 };
 
+Decision STAY = {kStay, kDefault, ""};
+Decision GO = {kGo, kSitelist, "example.com"};
+
 TEST_F(BrowserSwitcherNavigationThrottleTest, ShouldIgnoreNavigation) {
-  EXPECT_CALL(*sitelist(), ShouldSwitch(_)).WillOnce(Return(false));
+  EXPECT_CALL(*sitelist(), GetDecision(_)).WillOnce(Return(STAY));
   std::unique_ptr<MockNavigationHandle> handle =
       CreateMockNavigationHandle(GURL("https://example.com/"));
   std::unique_ptr<NavigationThrottle> throttle =
@@ -92,7 +95,7 @@ TEST_F(BrowserSwitcherNavigationThrottleTest, ShouldIgnoreNavigation) {
 }
 
 TEST_F(BrowserSwitcherNavigationThrottleTest, LaunchesOnStartRequest) {
-  EXPECT_CALL(*sitelist(), ShouldSwitch(_)).WillOnce(Return(true));
+  EXPECT_CALL(*sitelist(), GetDecision(_)).WillOnce(Return(GO));
   std::unique_ptr<MockNavigationHandle> handle =
       CreateMockNavigationHandle(GURL("https://example.com/"));
   std::unique_ptr<NavigationThrottle> throttle =
@@ -103,9 +106,9 @@ TEST_F(BrowserSwitcherNavigationThrottleTest, LaunchesOnStartRequest) {
 }
 
 TEST_F(BrowserSwitcherNavigationThrottleTest, LaunchesOnRedirectRequest) {
-  EXPECT_CALL(*sitelist(), ShouldSwitch(_))
-      .WillOnce(Return(false))
-      .WillOnce(Return(true));
+  EXPECT_CALL(*sitelist(), GetDecision(_))
+      .WillOnce(Return(STAY))
+      .WillOnce(Return(GO));
   std::unique_ptr<MockNavigationHandle> handle =
       CreateMockNavigationHandle(GURL("https://yahoo.com/"));
   std::unique_ptr<NavigationThrottle> throttle =

chrome/browser/browser_switcher/browser_switcher_sitelist.cc

@@ -160,29 +160,47 @@ void CanonicalizeRule(std::string* pattern) {
   *pattern = base::StrCat({prefix, spec.as_string()});
 }
 
+Decision::Decision(Action action_,
+                   Reason reason_,
+                   base::StringPiece matching_rule_)
+    : action(action_), reason(reason_), matching_rule(matching_rule_) {}
+
+Decision::Decision() = default;
+Decision::Decision(Decision&) = default;
+Decision::Decision(Decision&&) = default;
+
+bool Decision::operator==(const Decision& that) const {
+  return (action == that.action && reason == that.reason &&
+          matching_rule == that.matching_rule);
+}
+
 BrowserSwitcherSitelist::~BrowserSwitcherSitelist() = default;
 
+bool BrowserSwitcherSitelist::ShouldSwitch(const GURL& url) const {
+  return GetDecision(url).action == kGo;
+}
+
 BrowserSwitcherSitelistImpl::BrowserSwitcherSitelistImpl(
     const BrowserSwitcherPrefs* prefs)
     : prefs_(prefs) {}
 
 BrowserSwitcherSitelistImpl::~BrowserSwitcherSitelistImpl() = default;
 
-bool BrowserSwitcherSitelistImpl::ShouldSwitch(const GURL& url) const {
+Decision BrowserSwitcherSitelistImpl::GetDecision(const GURL& url) const {
   // Don't record metrics for LBS non-users.
   if (!IsActive())
-    return false;
+    return {kStay, kDisabled, ""};
 
-  bool should_switch = ShouldSwitchImpl(url);
-  UMA_HISTOGRAM_BOOLEAN("BrowserSwitcher.Decision", should_switch);
-  return should_switch;
+  Decision decision = GetDecisionImpl(url);
+  UMA_HISTOGRAM_BOOLEAN("BrowserSwitcher.Decision", decision.action == kGo);
+  return decision;
 }
 
-bool BrowserSwitcherSitelistImpl::ShouldSwitchImpl(const GURL& url) const {
+Decision BrowserSwitcherSitelistImpl::GetDecisionImpl(const GURL& url) const {
   SCOPED_UMA_HISTOGRAM_TIMER("BrowserSwitcher.DecisionTime");
 
   if (!url.SchemeIsHTTPOrHTTPS() && !url.SchemeIsFile()) {
-    return false;
+    return {kStay, kProtocol, ""};
   }
 
   std::string url_host = url.host();
@@ -197,9 +215,10 @@ bool BrowserSwitcherSitelistImpl::ShouldSwitchImpl(const GURL& url) const {
       StringSizeCompare);
 
   // If sitelists don't match, no need to check the greylists.
-  if (reason_to_go.empty() || IsInverted(reason_to_go)) {
-    return false;
-  }
+  if (reason_to_go.empty())
+    return {kStay, kDefault, ""};
+  if (IsInverted(reason_to_go))
+    return {kStay, kSitelist, reason_to_go};
 
   base::StringPiece reason_to_stay = std::max(
       {
@@ -210,9 +229,12 @@ bool BrowserSwitcherSitelistImpl::ShouldSwitchImpl(const GURL& url) const {
       StringSizeCompare);
 
   if (reason_to_go == "*" && !reason_to_stay.empty())
-    return false;
+    return {kStay, kGreylist, reason_to_stay};
 
-  return reason_to_go.size() >= reason_to_stay.size();
+  if (reason_to_go.size() >= reason_to_stay.size())
+    return {kGo, kSitelist, reason_to_go};
+  else
+    return {kStay, kGreylist, reason_to_stay};
 }
 
 void BrowserSwitcherSitelistImpl::SetIeemSitelist(ParsedXml&& parsed_xml) {

chrome/browser/browser_switcher/browser_switcher_sitelist.h

@@ -20,6 +20,40 @@ class ParsedXml;
 // having to convert uppercase/lowercase for every rule at every navigation.
 void CanonicalizeRule(std::string* rule);
 
+enum Action {
+  kStay = 0,
+  kGo = 1,
+};
+
+enum Reason {
+  // BrowserSwitcher is globally disabled.
+  kDisabled = 0,
+  // Protocol is not HTTP, HTTPS or FILE.
+  kProtocol = 1,
+  // A sitelist rule (either positive or negative) matched.
+  kSitelist = 2,
+  // A greylist rule matched.
+  kGreylist = 3,
+  // No rule matched, so default to STAY.
+  kDefault = 4,
+};
+
+struct Decision {
+  Decision(Action, Reason, base::StringPiece matching_rule);
+
+  Decision();
+  Decision(Decision&);
+  Decision(Decision&&);
+
+  bool operator==(const Decision&) const;
+
+  Action action;
+  Reason reason;
+  // If reason is kSitelist or kGreylist, this is the rule that caused the
+  // decision.
+  base::StringPiece matching_rule;
+};
+
 // Interface that decides whether a navigation should trigger a browser
 // switch.
 class BrowserSwitcherSitelist {
@@ -27,7 +61,11 @@ class BrowserSwitcherSitelist {
   virtual ~BrowserSwitcherSitelist();
 
   // Returns true if the given URL should be open in an alternative browser.
-  virtual bool ShouldSwitch(const GURL& url) const = 0;
+  bool ShouldSwitch(const GURL& url) const;
+
+  // Same as ShouldSwitch(), but returns a struct instead of a bool, also
+  // containing the reason why this decision was made.
+  virtual Decision GetDecision(const GURL& url) const = 0;
 
   // Set the Internet Explorer Enterprise Mode sitelist to use, in addition to
   // Chrome's sitelist/greylist policies. Consumes the object, and performs no
@@ -52,7 +90,7 @@ class BrowserSwitcherSitelistImpl : public BrowserSwitcherSitelist {
   ~BrowserSwitcherSitelistImpl() override;
 
   // BrowserSwitcherSitelist
-  bool ShouldSwitch(const GURL& url) const override;
+  Decision GetDecision(const GURL& url) const override;
   void SetIeemSitelist(ParsedXml&& sitelist) override;
   void SetExternalSitelist(ParsedXml&& sitelist) override;
   const RuleSet* GetIeemSitelist() const override;
@@ -62,7 +100,7 @@ class BrowserSwitcherSitelistImpl : public BrowserSwitcherSitelist {
   // Returns true if there are any rules configured.
   bool IsActive() const;
 
-  bool ShouldSwitchImpl(const GURL& url) const;
+  Decision GetDecisionImpl(const GURL& url) const;
 
   RuleSet ieem_sitelist_;
   RuleSet external_sitelist_;

chrome/browser/browser_switcher/browser_switcher_sitelist_unittest.cc

@@ -41,10 +41,11 @@ std::unique_ptr<base::Value> StringArrayToValue(
 class BrowserSwitcherSitelistTest : public testing::Test {
  public:
   void Initialize(const std::vector<const char*>& url_list,
-                  const std::vector<const char*>& url_greylist) {
+                  const std::vector<const char*>& url_greylist,
+                  bool enabled = true) {
     BrowserSwitcherPrefs::RegisterProfilePrefs(prefs_backend_.registry());
     prefs_backend_.SetManagedPref(prefs::kEnabled,
-                                  std::make_unique<base::Value>(true));
+                                  std::make_unique<base::Value>(enabled));
     prefs_backend_.SetManagedPref(prefs::kUrlList,
                                   StringArrayToValue(url_list));
     prefs_backend_.SetManagedPref(prefs::kUrlGreylist,
@@ -54,6 +55,7 @@ class BrowserSwitcherSitelistTest : public testing::Test {
   }
 
   bool ShouldSwitch(const GURL& url) { return sitelist_->ShouldSwitch(url); }
+  Decision GetDecision(const GURL& url) { return sitelist_->GetDecision(url); }
 
   sync_preferences::TestingPrefServiceSyncable* prefs_backend() {
     return &prefs_backend_;
@@ -262,4 +264,28 @@ TEST_F(BrowserSwitcherSitelistTest, All3Sources) {
   EXPECT_FALSE(ShouldSwitch(GURL("http://finance.yahoo.com/")));
 }
 
+TEST_F(BrowserSwitcherSitelistTest, BrowserSwitcherDisabled) {
+  Initialize({"example.com"}, {}, false);
+  EXPECT_FALSE(ShouldSwitch(GURL("http://example.com/")));
+  EXPECT_EQ(Decision(kStay, kDisabled, ""),
+            GetDecision(GURL("http://example.com/")));
+}
+
+TEST_F(BrowserSwitcherSitelistTest, CheckReason) {
+  Initialize({"foo.invalid.com", "!example.com"},
+             {"//foo.invalid.com/foobar", "invalid.com"});
+  EXPECT_EQ(Decision(kStay, kProtocol, ""),
+            GetDecision(GURL("ftp://example.com/")));
+  EXPECT_EQ(Decision(kStay, kDefault, ""),
+            GetDecision(GURL("http://google.com/")));
+  EXPECT_EQ(Decision(kStay, kDefault, ""),
+            GetDecision(GURL("http://bar.invalid.com/")));
+  EXPECT_EQ(Decision(kStay, kSitelist, "!example.com"),
+            GetDecision(GURL("http://example.com/")));
+  EXPECT_EQ(Decision(kGo, kSitelist, "foo.invalid.com"),
+            GetDecision(GURL("http://foo.invalid.com/")));
+  EXPECT_EQ(Decision(kStay, kGreylist, "//foo.invalid.com/foobar"),
+            GetDecision(GURL("http://foo.invalid.com/foobar")));
+}
+
 }  // namespace browser_switcher

chrome/browser/resources/browser_switch/internals/browser_switch_internals.html

@@ -14,6 +14,10 @@
     table {
       border-collapse: collapse;
       font-family: monospace;
+    }
+
+    pre,
+    table {
       font-size: 1.25em;
     }
 
@@ -32,14 +36,37 @@
     .url {
       text-align: start;
     }
+
+    #url-checker-input {
+      width: 400px;
+    }
   </style>
 </head>
 <body>
   <h1>Legacy Browser Support Internals</h1>
 
+  <p>
+    If an XML sitelist is configured, wait 1 minute after browser
+    startup for it to download, and then refresh this page.
+  </p>
+
   <!-- TODO(crbug/959379): Hide all this and show a message if
                            BrowserSwitcherEnabled is false. -->
 
+  <h2>URL Checker</h2>
+
+  <p>
+    Enter a complete URL (with the http:// part) to see what LBS would do
+    with it.
+  </p>
+
+  <p>
+    <input type="text" id="url-checker-input"
+        placeholder="http://example.com/">
+  </p>
+
+  <pre id=output></pre>
+
   <h2>Sitelist</h2>
   <table id="sitelist"></table>
 

chrome/browser/resources/browser_switch/internals/browser_switch_internals.js

@@ -81,3 +81,24 @@ function updateTables(rulesets) {
 }
 
 cr.sendWithPromise('getAllRulesets').then(updateTables);
+
+function checkUrl() {
+  const url = $('url-checker-input').value;
+  if (!url) {
+    $('output').innerText = '';
+    return;
+  }
+  cr.sendWithPromise('getDecision', url)
+      .then(decision => {
+        // URL is valid.
+        $('output').innerText = JSON.stringify(decision, null, 2);
+      })
+      .catch(err => {
+        // URL is invalid.
+        $('output').innerText =
+            'Invalid URL. Make sure it is formatted properly.';
+      });
+}
+
+$('url-checker-input').addEventListener('input', checkUrl);
+checkUrl();

chrome/browser/ui/webui/browser_switch/browser_switch_ui.cc

@@ -186,6 +186,16 @@ class BrowserSwitchHandler : public content::WebUIMessageHandler {
   // }
   void HandleGetAllRulesets(const base::ListValue* args);
 
+  // Resolves a promise with a JSON object describing the decision for a URL
+  // (stay/go) + reason. The result is formatted like this:
+  //
+  // {
+  //   "action": ("stay"|"go"),
+  //   "reason": ("globally_disabled"|"protocol"|"sitelist"|...),
+  //   "matching_rule": (string|undefined)
+  // }
+  void HandleGetDecision(const base::ListValue* args);
+
   DISALLOW_COPY_AND_ASSIGN(BrowserSwitchHandler);
 };
 
@@ -206,6 +216,10 @@ void BrowserSwitchHandler::RegisterMessages() {
       "getAllRulesets",
       base::BindRepeating(&BrowserSwitchHandler::HandleGetAllRulesets,
                           base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "getDecision",
+      base::BindRepeating(&BrowserSwitchHandler::HandleGetDecision,
+                          base::Unretained(this)));
 }
 
 void BrowserSwitchHandler::HandleLaunchAlternativeBrowserAndCloseTab(
@@ -274,6 +288,53 @@ void BrowserSwitchHandler::HandleGetAllRulesets(const base::ListValue* args) {
   ResolveJavascriptCallback(args->GetList()[0], retval);
 }
 
+void BrowserSwitchHandler::HandleGetDecision(const base::ListValue* args) {
+  DCHECK(args);
+  AllowJavascript();
+
+  GURL url = GURL(args->GetList()[1].GetString());
+  if (!url.is_valid()) {
+    RejectJavascriptCallback(args->GetList()[0], base::Value());
+    return;
+  }
+
+  auto* service = GetBrowserSwitcherService(web_ui());
+  browser_switcher::Decision decision = service->sitelist()->GetDecision(url);
+
+  base::DictionaryValue retval;
+
+  base::StringPiece action_name =
+      (decision.action == browser_switcher::kStay) ? "stay" : "go";
+  retval.Set("action", std::make_unique<base::Value>(action_name));
+
+  base::StringPiece reason_name;
+  switch (decision.reason) {
+    case browser_switcher::kDisabled:
+      reason_name = "globally_disabled";
+      break;
+    case browser_switcher::kProtocol:
+      reason_name = "protocol";
+      break;
+    case browser_switcher::kSitelist:
+      reason_name = "sitelist";
+      break;
+    case browser_switcher::kGreylist:
+      reason_name = "greylist";
+      break;
+    case browser_switcher::kDefault:
+      reason_name = "default";
+      break;
+  }
+  retval.Set("reason", std::make_unique<base::Value>(reason_name));
+
+  if (!decision.matching_rule.empty()) {
+    retval.Set("matching_rule",
+               std::make_unique<base::Value>(decision.matching_rule));
+  }
+
+  ResolveJavascriptCallback(args->GetList()[0], retval);
+}
+
 }  // namespace
 
 BrowserSwitchUI::BrowserSwitchUI(content::WebUI* web_ui)