[BrowserSwitcher] Update sitelist v1 logic

This improves the compatibility with actual IEEM sitelists, especially
the doNotTransition attribute.

Previously:
- "exclude" meant "invert this rule"
- "doNotTransition" meant "apply to greylist instead of sitelist"

Now:
- "exclude" means "this rule does nothing (but can have sub-rules)"
- "doNotTransition" means "invert this rule"

Bug: 933577
Change-Id: I84f516ad2cc7d6e0f400b8ae26bf81e470b55816
Reviewed-on: https://chromium-review.googlesource.com/c/1481796Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634797}

chrome/browser/browser_switcher/ieem_sitelist_parser.cc

@@ -45,10 +45,10 @@ std::vector<const base::Value*> GetChildrenWithTag(const base::Value& node,
 struct Entry {
   // URL or path concerned.
   std::string text;
-  // Whether to include or exclude the URL.
+  // True if the exclude attribute is "true".
   bool exclude;
-  // List affected by this rule (sitelist or greylist).
-  std::vector<std::string>* list;
+  // True if the doNotTransition attribute is "true".
+  bool do_not_transition;
 };
 
 Entry ParseDomainOrPath(const base::Value& node, ParsedXml* result) {
@@ -63,9 +63,7 @@ Entry ParseDomainOrPath(const base::Value& node, ParsedXml* result) {
 
   std::string do_not_transition_attrib =
       GetXmlElementAttribute(node, kSchema1DoNotTransitionAttribute);
-  entry.list =
-      ((do_not_transition_attrib == kSchema1TrueValue) ? &result->greylist
-                                                       : &result->sitelist);
+  entry.do_not_transition = (do_not_transition_attrib == kSchema1TrueValue);
 
   GetXmlElementText(node, &entry.text);
   base::TrimWhitespaceASCII(entry.text, base::TRIM_ALL, &entry.text);
@@ -87,17 +85,17 @@ void ParseIeFileVersionOne(const base::Value& xml, ParsedXml* result) {
     for (const base::Value* domain_node :
          GetChildrenWithTag(node, kSchema1DomainElement)) {
       Entry domain = ParseDomainOrPath(*domain_node, result);
-      if (!domain.text.empty()) {
-        std::string prefix = (domain.exclude ? "!" : "");
-        domain.list->push_back(prefix + domain.text);
+      if (!domain.text.empty() && !domain.exclude) {
+        std::string prefix = (domain.do_not_transition ? "!" : "");
+        result->sitelist.push_back(prefix + domain.text);
       }
       // Loop over <path> elements.
       for (const base::Value* path_node :
            GetChildrenWithTag(*domain_node, kSchema1PathElement)) {
         Entry path = ParseDomainOrPath(*path_node, result);
-        if (!path.text.empty() && !domain.text.empty()) {
-          std::string prefix = (path.exclude ? "!" : "");
-          path.list->push_back(prefix + domain.text + path.text);
+        if (!path.text.empty() && !domain.text.empty() && !path.exclude) {
+          std::string prefix = (path.do_not_transition ? "!" : "");
+          result->sitelist.push_back(prefix + domain.text + path.text);
         }
       }
     }

chrome/browser/browser_switcher/ieem_sitelist_parser_browsertest.cc

@@ -101,49 +101,41 @@ IN_PROC_BROWSER_TEST_F(IeemSitelistParserTest, V1Full) {
       "exclude=\"false\">/r6</path></domain><domain docMode=\"5\" "
       "exclude=\"false\">howmanydomainz.com<path docMode=\"5\">/r8</path><path "
       "docMode=\"5\" exclude=\"true\">/r9</path><path docMode=\"5\" "
-      "exclude=\"false\">/r10</path></domain><domain doNotTransition=\"true\">"
-      "notransition.com<path>/yestransition</path><path exclude=\"true\" "
-      "doNotTransition=\"true\">/guessnot</path></domain></docMode></rules>";
+      "exclude=\"false\">/r10</path></domain><domain exclude=\"true\" "
+      "doNotTransition=\"true\">maybe.com<path>/yestransition</path>"
+      "<path doNotTransition=\"true\">/guessnot</path></domain><domain>"
+      "yes.com<path doNotTransition=\"true\">/actuallyno</path></domain>"
+      "<domain doNotTransition=\"true\">no.com</domain></docMode></rules>";
   std::vector<std::string> expected_sitelist = {
       "inside.com",
       "inside.com/in_domain",
       "google.com",
-      "!good.com",
       "more.com",
       "e100.com",
       "e100.com/path1",
-      "!e100.com/pa2",
       "e100.com/path3",
-      "!e200.com",
       "e200.com/path1",
-      "!e200.com/pth2",
       "e200.com/path3",
       "e300.com",
       "e300.com/path1",
-      "!e300.com/pt2",
       "e300.com/path3",
-      "!random.com",
-      "!random.com/path1/",
       "random.com/path2",
       "moredomains.com",
       "evenmore.com",
       "evenmore.com/r1",
       "evenmore.com/r2",
-      "!domainz.com",
       "domainz.com/r2",
-      "!domainz.com/r5",
       "domainz.com/r6",
       "howmanydomainz.com",
       "howmanydomainz.com/r8",
-      "!howmanydomainz.com/r9",
       "howmanydomainz.com/r10",
-      "notransition.com/yestransition",
+      "maybe.com/yestransition",
+      "!maybe.com/guessnot",
+      "yes.com",
+      "!yes.com/actuallyno",
+      "!no.com",
   };
-  std::vector<std::string> expected_greylist = {
-      "notransition.com", "!notransition.com/guessnot",
-  };
-  TestParseXml(xml, ParsedXml(std::move(expected_sitelist),
-                              std::move(expected_greylist), base::nullopt));
+  TestParseXml(xml, ParsedXml(std::move(expected_sitelist), {}, base::nullopt));
 }
 
 IN_PROC_BROWSER_TEST_F(IeemSitelistParserTest, V2Full) {