Consolidate Origin- and RegistrableDomain- FilterBuilder into one class

chrome/browser/chrome_content_browser_client_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chrome_content_browser_client.h"
 
 #include <list>
 #include <map>
 #include <memory>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/browsing_data/browsing_data_filter_builder.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_impl.h"
-#include "chrome/browser/browsing_data/origin_filter_builder.h"
-#include "chrome/browser/browsing_data/registrable_domain_filter_builder.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/variations/entropy_provider.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
@@ skipping 324 matching lines @@
   }
 
   void RemoveInternal(const base::Time& delete_begin,
                       const base::Time& delete_end,
                       int remove_mask,
                       int origin_type_mask,
                       std::unique_ptr<BrowsingDataFilterBuilder> filter_builder,
                       BrowsingDataRemover::Observer* observer) override {
     actual_calls_.emplace_back(delete_begin, delete_end, remove_mask,
                                origin_type_mask, std::move(filter_builder),
-                               UNKNOWN);
+                               true /* filter_is_important */);
 
     // |observer| is not recorded in |actual_calls_| to be compared with
     // expectations, because it's created internally in ClearSiteData() and
     // it's unknown to this. However, it is tested implicitly, because we use
     // it for the completion callback, so an incorrect |observer| will fail
     // the test by waiting for the callback forever.
     DCHECK(observer);
     observer->OnBrowsingDataRemoverDone();
   }
 
   void ExpectCall(
       const base::Time& delete_begin,
       const base::Time& delete_end,
       int remove_mask,
       int origin_type_mask,
-      std::unique_ptr<RegistrableDomainFilterBuilder> filter_builder) {
+      std::unique_ptr<BrowsingDataFilterBuilder> filter_builder) {
     expected_calls_.emplace_back(delete_begin, delete_end, remove_mask,
                                  origin_type_mask, std::move(filter_builder),
-                                 REGISTRABLE_DOMAIN_FILTER_BUILDER);
-  }
-
-  void ExpectCall(const base::Time& delete_begin,
-                  const base::Time& delete_end,
-                  int remove_mask,
-                  int origin_type_mask,
-                  std::unique_ptr<OriginFilterBuilder> filter_builder) {
-    expected_calls_.emplace_back(delete_begin, delete_end, remove_mask,
-                                 origin_type_mask, std::move(filter_builder),
-                                 ORIGIN_FILTER_BUILDER);
+                                 true /* filter_is_important */);
   }
 
   void ExpectCallDontCareAboutFilterBuilder(const base::Time& delete_begin,
                                             const base::Time& delete_end,
                                             int remove_mask,
                                             int origin_type_mask) {
     expected_calls_.emplace_back(delete_begin, delete_end, remove_mask,
                                  origin_type_mask,
                                  std::unique_ptr<BrowsingDataFilterBuilder>(),
-                                 DONT_CARE);
+                                 false /* filter_is_important */);
   }
 
   void VerifyAndClearExpectations() {
     EXPECT_EQ(expected_calls_, actual_calls_);
     expected_calls_.clear();
     actual_calls_.clear();
   }
 
  private:
-  // Used to further specify the type and intention behind the passed
-  // std::unique_ptr<BrowsingDataFilterBuilder>. This is needed for comparison
-  // between the expected and actual call parameters.
-  enum FilterBuilderType {
-    REGISTRABLE_DOMAIN_FILTER_BUILDER,  // RegistrableDomainFilterBuilder
-    ORIGIN_FILTER_BUILDER,              // OriginFilterBuilder
-    UNKNOWN,                            // can't static_cast<>
-    DONT_CARE                           // don't have to compare for equality
-  };
-
   class CallParameters {
    public:
     CallParameters(const base::Time& delete_begin,
                    const base::Time& delete_end,
                    int remove_mask,
                    int origin_type_mask,
                    std::unique_ptr<BrowsingDataFilterBuilder> filter_builder,
-                   FilterBuilderType type)
+                   bool filter_is_important)

[Mike West, 2017/01/24 08:37:27]

Nit: What does "important" mean? How about "should_compare_filter"?
"filter_is_not_relevant_to_this_test"?

[msramek, 2017/01/24 11:52:31]

Yeah, good point. Let's call it "should_compare_filter".

         : delete_begin_(delete_begin),
           delete_end_(delete_end),
           remove_mask_(remove_mask),
           origin_type_mask_(origin_type_mask),
           filter_builder_(std::move(filter_builder)),
-          type_(type) {}
+          filter_is_important_(filter_is_important) {}
     ~CallParameters() {}
 
     bool operator==(const CallParameters& other) const {
       const CallParameters& a = *this;
       const CallParameters& b = other;
 
       if (a.delete_begin_ != b.delete_begin_ ||
           a.delete_end_ != b.delete_end_ ||
           a.remove_mask_ != b.remove_mask_ ||
           a.origin_type_mask_ != b.origin_type_mask_) {
         return false;
       }
 
-      if (a.type_ == DONT_CARE || b.type_ == DONT_CARE)
+      if (!a.filter_is_important_ || !b.filter_is_important_)
         return true;
-      if (a.type_ == UNKNOWN && b.type_ == UNKNOWN)
-        return false;
-      if (a.type_ != UNKNOWN && b.type_ != UNKNOWN && a.type_ != b.type_)
-        return false;
-
-      FilterBuilderType resolved_type =
-          (a.type_ != UNKNOWN) ? a.type_ : b.type_;
-
-      DCHECK(resolved_type == ORIGIN_FILTER_BUILDER ||
-             resolved_type == REGISTRABLE_DOMAIN_FILTER_BUILDER);
-
-      if (resolved_type == ORIGIN_FILTER_BUILDER) {
-        return *static_cast<OriginFilterBuilder*>(a.filter_builder_.get()) ==
-               *static_cast<OriginFilterBuilder*>(b.filter_builder_.get());
-      } else if (resolved_type == REGISTRABLE_DOMAIN_FILTER_BUILDER) {
-        return *static_cast<RegistrableDomainFilterBuilder*>(
-                   a.filter_builder_.get()) ==
-               *static_cast<RegistrableDomainFilterBuilder*>(
-                   b.filter_builder_.get());
-      }
-
-      NOTREACHED();
-      return false;
+      return *a.filter_builder_ == *b.filter_builder_;
     }
 
    private:
     base::Time delete_begin_;
     base::Time delete_end_;
     int remove_mask_;
     int origin_type_mask_;
     std::unique_ptr<BrowsingDataFilterBuilder> filter_builder_;
-    FilterBuilderType type_;
+    bool filter_is_important_;
   };
 
   std::list<CallParameters> actual_calls_;
   std::list<CallParameters> expected_calls_;
 };
 
 // Tests for ChromeContentBrowserClient::ClearSiteData().
 class ChromeContentBrowserClientClearSiteDataTest : public testing::Test {
  public:
   void SetUp() override {
@@ skipping 141 matching lines @@
 
       // Scheme and port don't matter.
       {"http://example.com", "example.com"},
       {"http://example.com:8080", "example.com"},
       {"https://example.com:4433", "example.com"},
   };
 
   for (const TestCase& test_case : test_cases) {
     SCOPED_TRACE(test_case.origin);
 
-    std::unique_ptr<RegistrableDomainFilterBuilder>
-        registrable_domain_filter_builder(new RegistrableDomainFilterBuilder(
+    std::unique_ptr<BrowsingDataFilterBuilder>
+        registrable_domain_filter_builder(BrowsingDataFilterBuilder::Create(
             BrowsingDataFilterBuilder::WHITELIST));
     registrable_domain_filter_builder->AddRegisterableDomain(test_case.domain);
 
     remover()->ExpectCall(
         base::Time(), base::Time::Max(),
         BrowsingDataRemover::REMOVE_COOKIES |
             BrowsingDataRemover::REMOVE_CHANNEL_IDS |
             BrowsingDataRemover::REMOVE_PLUGIN_DATA,
         BrowsingDataHelper::ALL, std::move(registrable_domain_filter_builder));
 
-    std::unique_ptr<OriginFilterBuilder> origin_filter_builder(
-        new OriginFilterBuilder(BrowsingDataFilterBuilder::WHITELIST));
+    std::unique_ptr<BrowsingDataFilterBuilder> origin_filter_builder(
+        BrowsingDataFilterBuilder::Create(
+            BrowsingDataFilterBuilder::WHITELIST));
     origin_filter_builder->AddOrigin(url::Origin(GURL(test_case.origin)));
 
     remover()->ExpectCall(
         base::Time(), base::Time::Max(),
         BrowsingDataRemover::REMOVE_CACHE, BrowsingDataHelper::ALL,
         std::move(origin_filter_builder));
 
     SetClearingFinished(false);
     client.ClearSiteData(
         profile(), url::Origin(GURL(test_case.origin)), true /* cookies */,
@@ skipping 48 matching lines @@
   client.ClearSiteData(
       profile(), origin, true /* cookies */, false /* storage */,
       true /* cache */,
       base::Bind(
           &ChromeContentBrowserClientClearSiteDataTest::SetClearingFinished,
           base::Unretained(this), true));
   EXPECT_TRUE(IsClearingFinished());
 }
 
 }  // namespace