Link PasswordProtectionService to Profile and SB Service

components/safe_browsing/password_protection/password_protection_service_unittest.cc

Index: components/safe_browsing/password_protection/password_protection_service_unittest.cc
diff --git a/components/safe_browsing/password_protection/password_protection_service_unittest.cc b/components/safe_browsing/password_protection/password_protection_service_unittest.cc
index 04c0d0f75ce238091db8fa63586f88d3f40f3f34..5f88e35bffde29575406e2d9dde2a019e1832d77 100644
--- a/components/safe_browsing/password_protection/password_protection_service_unittest.cc
+++ b/components/safe_browsing/password_protection/password_protection_service_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/null_task_runner.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/safe_browsing/password_protection/password_protection_request.h"
 #include "components/safe_browsing_db/test_database_manager.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
@@ -66,12 +67,10 @@ class TestPasswordProtectionService : public PasswordProtectionService {
       const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
       scoped_refptr<net::URLRequestContextGetter> request_context_getter,
       scoped_refptr<HostContentSettingsMap> content_setting_map)
-      : PasswordProtectionService(database_manager, request_context_getter),
-        content_setting_map_(content_setting_map) {}
-
-  HostContentSettingsMap* GetSettingMapForActiveProfile() override {
-    return content_setting_map_.get();
-  }
+      : PasswordProtectionService(database_manager,
+                                  request_context_getter,
+                                  nullptr,
+                                  content_setting_map.get()) {}
 
   void RequestFinished(
       PasswordProtectionRequest* request,
@@ -79,13 +78,23 @@ class TestPasswordProtectionService : public PasswordProtectionService {
     latest_response_ = std::move(response);
   }
 
+  // Intentionally do nothing.
+  void FillReferrerChain(const GURL& event_url,
+                         int event_tab_id,
+                         LoginReputationClientRequest::Frame* frame) override {}
+
+  bool IsExtendedReporting() override { return true; }
+
+  bool IsIncognito() override { return false; }

[Nathan Parker, 2017/03/30 21:38:12]

Do you test anywhere that these get used correctly?  Like if IsIncognito returns
false?

[Jialiu Lin, 2017/03/30 23:23:13]

Yes, these are just default returns. Situations like incognito, non-SBER, are
covered in 
TestNoRequestSentForIncognito, TestNoRequestSentForNonExtendedReporting.
I have refactored the testing code a little to make it easier to read.

+
+  bool IsPingingEnabled() override { return true; }
+
   LoginReputationClientResponse* latest_response() {
     return latest_response_.get();
   }
 
  private:
   std::unique_ptr<LoginReputationClientResponse> latest_response_;
-  scoped_refptr<HostContentSettingsMap> content_setting_map_;
   DISALLOW_COPY_AND_ASSIGN(TestPasswordProtectionService);
 };
 
@@ -162,8 +171,8 @@ class PasswordProtectionServiceTest : public testing::Test {
                     const base::Time& verdict_received_time) {
     LoginReputationClientResponse response(CreateVerdictProto(
         verdict, cache_duration_sec, cache_expression, exact_match));
-    password_protection_service_->CacheVerdict(
-        url, &response, verdict_received_time, content_setting_map_.get());
+    password_protection_service_->CacheVerdict(url, &response,
+                                               verdict_received_time);
   }
 
   size_t GetStoredVerdictCount() {
@@ -341,7 +350,6 @@ TEST_F(PasswordProtectionServiceTest, TestCachedVerdicts) {
   LoginReputationClientResponse out_verdict;
   EXPECT_EQ(LoginReputationClientResponse::PHISHING,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(),
                 GURL("http://www.test.com/foo/index2.html"), &out_verdict));
 
   // Cache another verdict with the same origin but different cache_expression
@@ -377,21 +385,18 @@ TEST_F(PasswordProtectionServiceTest, TestGetCachedVerdicts) {
   LoginReputationClientResponse actual_verdict;
   EXPECT_EQ(LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(), GURL("http://www.unknown.com/"),
-                &actual_verdict));
+                GURL("http://www.unknown.com/"), &actual_verdict));
 
   // Return VERDICT_TYPE_UNSPECIFIED if look up for a URL with http://test.com
   // origin, but doesn't match any known cache_expression.
   EXPECT_EQ(LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(), GURL("http://test.com/xyz/foo.jsp"),
-                &actual_verdict));
+                GURL("http://test.com/xyz/foo.jsp"), &actual_verdict));
 
   // Return VERDICT_TYPE_UNSPECIFIED if look up for a URL whose variants match
   // test.com/def, since corresponding entry is expired.
   EXPECT_EQ(LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(),
                 GURL("http://test.com/def/ghi/index.html"), &actual_verdict));
 
   // Return VERDICT_TYPE_UNSPECIFIED if look up for a URL whose variants match
@@ -399,26 +404,21 @@ TEST_F(PasswordProtectionServiceTest, TestGetCachedVerdicts) {
   // test.com.
   EXPECT_EQ(LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(),
                 GURL("http://test.com/ghi/index.html"), &actual_verdict));
   EXPECT_EQ(LoginReputationClientResponse::SAFE,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(),
                 GURL("http://test.com/term_of_service.html"), &actual_verdict));
 
   // Return LOW_REPUTATION if look up for a URL whose variants match
   // test.com/abc.
   EXPECT_EQ(LoginReputationClientResponse::LOW_REPUTATION,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(), GURL("http://test.com/abc/"),
-                &actual_verdict));
+                GURL("http://test.com/abc/"), &actual_verdict));
   EXPECT_EQ(LoginReputationClientResponse::LOW_REPUTATION,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(), GURL("http://test.com/abc/bar.jsp"),
-                &actual_verdict));
+                GURL("http://test.com/abc/bar.jsp"), &actual_verdict));
   EXPECT_EQ(LoginReputationClientResponse::LOW_REPUTATION,
             password_protection_service_->GetCachedVerdict(
-                content_setting_map_.get(),
                 GURL("http://test.com/abc/foo/bar.html"), &actual_verdict));
 }
 
@@ -440,18 +440,17 @@ TEST_F(PasswordProtectionServiceTest, TestCleanUpCachedVerdicts) {
   history::URLRows deleted_urls;
   deleted_urls.push_back(history::URLRow(GURL("http://bar.com")));
   password_protection_service_->RemoveContentSettingsOnURLsDeleted(
-      false /* all_history */, deleted_urls, content_setting_map_.get());
+      false /* all_history */, deleted_urls);
   EXPECT_EQ(1U, GetStoredVerdictCount());
   LoginReputationClientResponse actual_verdict;
-  EXPECT_EQ(
-      LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
-      password_protection_service_->GetCachedVerdict(
-          content_setting_map_.get(), GURL("http://bar.com"), &actual_verdict));
+  EXPECT_EQ(LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
+            password_protection_service_->GetCachedVerdict(
+                GURL("http://bar.com"), &actual_verdict));
 
   // If delete all history. All password protection content settings should be
   // gone.
   password_protection_service_->RemoveContentSettingsOnURLsDeleted(
-      true /* all_history */, history::URLRows(), content_setting_map_.get());
+      true /* all_history */, history::URLRows());
   EXPECT_EQ(0U, GetStoredVerdictCount());
 }
 
@@ -589,8 +588,7 @@ TEST_F(PasswordProtectionServiceTest, TestTearDownWithPendingRequests) {
   EXPECT_CALL(*database_manager_.get(), MatchCsdWhitelistUrl(target_url))
       .WillRepeatedly(testing::Return(false));
   password_protection_service_->StartRequest(
-      target_url, LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE,
-      true /* extended_reporting */, false /* incognito */);
+      target_url, LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
 
   // Destroy password_protection_service_ while there is one request pending.
   password_protection_service_.reset();