Domain Reliability: Default to enabled

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

Index: chrome/browser/browsing_data/browsing_data_remover_unittest.cc
diff --git a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
index 7d1e71ad131274625d302ed07cbbd9c962c156a8..914293e86c83bfcd6b1cc0784e11cc7ca1fa3c83 100644
--- a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
@@ -703,7 +703,9 @@ class ClearDomainReliabilityTester {
 
 class BrowsingDataRemoverTest : public testing::Test {
  public:
-  BrowsingDataRemoverTest() : profile_(new TestingProfile()) {
+  BrowsingDataRemoverTest()
+      : profile_(new TestingProfile()),
+        clear_domain_reliability_tester_(GetProfile()) {
     callback_subscription_ =
         BrowsingDataRemover::RegisterOnBrowsingDataRemovedCallback(
             base::Bind(&BrowsingDataRemoverTest::NotifyWithDetails,
@@ -724,6 +726,8 @@ class BrowsingDataRemoverTest : public testing::Test {
     // Otherwise we leak memory.
     profile_.reset();
     base::MessageLoop::current()->RunUntilIdle();
+
+    TestingBrowserProcess::GetGlobal()->SetLocalState(NULL);
   }
 
   void BlockUntilBrowsingDataRemoved(BrowsingDataRemover::TimePeriod period,
@@ -846,6 +850,11 @@ class BrowsingDataRemoverTest : public testing::Test {
 
   BrowsingDataRemover::CallbackSubscription callback_subscription_;
 
+ protected:
+  // Needed to mock out DomainReliabilityService, even for unrelated tests.
+  ClearDomainReliabilityTester clear_domain_reliability_tester_;

[davidben, 2015/02/03 19:09:23]

I don't think we ever interleave private/protected/private like that. If the
ordering matters, how about leave it private and add an accessor? Or stuff it in
a scoped_ptr so you can initialize it later.

[Deprecated (see juliatuttle), 2015/02/03 20:31:39]

Done.

+
+ private:
   DISALLOW_COPY_AND_ASSIGN(BrowsingDataRemoverTest);
 };
 
@@ -1772,13 +1781,13 @@ TEST_F(BrowsingDataRemoverTest, ContentProtectionPlatformKeysRemoval) {
 #endif
 
 TEST_F(BrowsingDataRemoverTest, DomainReliability_Null) {
-  ClearDomainReliabilityTester tester(GetProfile());
+  ClearDomainReliabilityTester& tester = clear_domain_reliability_tester_;

[davidben, 2015/02/03 19:09:23]

Nit: do we ever use non-const references? Pointer maybe.

[Deprecated (see juliatuttle), 2015/02/03 20:31:38]

Obsolete; I made it a const reference instead.

 
   EXPECT_EQ(0u, tester.clear_count());
 }
 
 TEST_F(BrowsingDataRemoverTest, DomainReliability_Beacons) {
-  ClearDomainReliabilityTester tester(GetProfile());
+  ClearDomainReliabilityTester& tester = clear_domain_reliability_tester_;
 
   BlockUntilBrowsingDataRemoved(
       BrowsingDataRemover::EVERYTHING,
@@ -1788,7 +1797,7 @@ TEST_F(BrowsingDataRemoverTest, DomainReliability_Beacons) {
 }
 
 TEST_F(BrowsingDataRemoverTest, DomainReliability_Contexts) {
-  ClearDomainReliabilityTester tester(GetProfile());
+  ClearDomainReliabilityTester& tester = clear_domain_reliability_tester_;
 
   BlockUntilBrowsingDataRemoved(
       BrowsingDataRemover::EVERYTHING,
@@ -1798,7 +1807,7 @@ TEST_F(BrowsingDataRemoverTest, DomainReliability_Contexts) {
 }
 
 TEST_F(BrowsingDataRemoverTest, DomainReliability_ContextsWin) {
-  ClearDomainReliabilityTester tester(GetProfile());
+  ClearDomainReliabilityTester& tester = clear_domain_reliability_tester_;
 
   BlockUntilBrowsingDataRemoved(
       BrowsingDataRemover::EVERYTHING,
@@ -1809,7 +1818,7 @@ TEST_F(BrowsingDataRemoverTest, DomainReliability_ContextsWin) {
 }
 
 TEST_F(BrowsingDataRemoverTest, DomainReliability_ProtectedOrigins) {
-  ClearDomainReliabilityTester tester(GetProfile());
+  ClearDomainReliabilityTester& tester = clear_domain_reliability_tester_;
 
   BlockUntilBrowsingDataRemoved(
       BrowsingDataRemover::EVERYTHING,
@@ -1818,7 +1827,10 @@ TEST_F(BrowsingDataRemoverTest, DomainReliability_ProtectedOrigins) {
   EXPECT_EQ(CLEAR_CONTEXTS, tester.last_clear_mode());
 }
 
-TEST_F(BrowsingDataRemoverTest, DomainReliability_NoMonitor) {
+// TODO(ttuttle): This isn't actually testing the no-monitor case, since
+// BrowsingDataRemoverTest now creates one unconditionally, since it's needed
+// for some unrelated test cases.
+TEST_F(BrowsingDataRemoverTest, DISABLED_DomainReliability_NoMonitor) {
   BlockUntilBrowsingDataRemoved(
       BrowsingDataRemover::EVERYTHING,
       BrowsingDataRemover::REMOVE_HISTORY |