Improve PasswordProtectionService and PasswordProtectionRequest

components/safe_browsing/password_protection/password_protection_service_unittest.cc

 // Copyright 2017 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 "components/safe_browsing/password_protection/password_protection_service.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/null_task_runner.h"
@@ skipping 62 matching lines @@
                                   content_setting_map.get()),
         is_extended_reporting_(true),
         is_incognito_(false) {}
 
   void RequestFinished(
       PasswordProtectionRequest* request,
       std::unique_ptr<LoginReputationClientResponse> response) override {
     latest_response_ = std::move(response);
   }
 
-  // Intentionally do nothing.
+  // Since referrer chain logic has been thoroughly tested in
+  // SBNavigationObserverBrowserTest class, we intentionally leave this function
+  // as a no-op here.
   void FillReferrerChain(const GURL& event_url,
                          int event_tab_id,
                          LoginReputationClientRequest::Frame* frame) override {}
 
   bool IsExtendedReporting() override { return is_extended_reporting_; }
 
   bool IsIncognito() override { return is_incognito_; }
 
   void set_extended_reporting(bool enabled) {
     is_extended_reporting_ = enabled;
   }
 
   void set_incognito(bool enabled) { is_incognito_ = enabled; }
 
   bool IsPingingEnabled() override { return true; }
 
   LoginReputationClientResponse* latest_response() {
     return latest_response_.get();
   }
 
+  ~TestPasswordProtectionService() override {}
+
+  size_t GetPendingRequestsCount() { return requests_.size(); }
+
  private:
   bool is_extended_reporting_;
   bool is_incognito_;
   std::unique_ptr<LoginReputationClientResponse> latest_response_;
   DISALLOW_COPY_AND_ASSIGN(TestPasswordProtectionService);
 };
 
 class PasswordProtectionServiceTest : public testing::Test {
  public:
   PasswordProtectionServiceTest(){};
@@ skipping 22 matching lines @@
   }
 
   void TearDown() override { content_setting_map_->ShutdownOnUIThread(); }
 
   // Sets up |database_manager_| and |requests_| as needed.
   void InitializeAndStartRequest(bool match_whitelist, int timeout_in_ms) {
     GURL target_url(kTargetUrl);
     EXPECT_CALL(*database_manager_.get(), MatchCsdWhitelistUrl(target_url))
         .WillRepeatedly(testing::Return(match_whitelist));
 
-    request_ = base::MakeUnique<PasswordProtectionRequest>(
+    request_ = new PasswordProtectionRequest(
         target_url, LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE,
-        password_protection_service_->IsExtendedReporting(),
-        password_protection_service_->IsIncognito(),
-        password_protection_service_->GetWeakPtr(), timeout_in_ms);
+        nullptr, password_protection_service_.get(), timeout_in_ms);
     request_->Start();
   }
 
   bool PathVariantsMatchCacheExpression(const GURL& url,
                                         const std::string& cache_expression) {
     std::vector<std::string> paths;
     PasswordProtectionService::GeneratePathVariantsWithoutQuery(url, &paths);
     return PasswordProtectionService::PathVariantsMatchCacheExpression(
         paths,
         PasswordProtectionService::GetCacheExpressionPath(cache_expression));
@@ skipping 16 matching lines @@
 
  protected:
   // |thread_bundle_| is needed here because this test involves both UI and IO
   // threads.
   content::TestBrowserThreadBundle thread_bundle_;
   scoped_refptr<MockSafeBrowsingDatabaseManager> database_manager_;
   sync_preferences::TestingPrefServiceSyncable test_pref_service_;
   scoped_refptr<HostContentSettingsMap> content_setting_map_;
   scoped_refptr<DummyURLRequestContextGetter> dummy_request_context_getter_;
   std::unique_ptr<TestPasswordProtectionService> password_protection_service_;
-  std::unique_ptr<PasswordProtectionRequest> request_;
+  scoped_refptr<PasswordProtectionRequest> request_;
   base::HistogramTester histograms_;
 };
 
 TEST_F(PasswordProtectionServiceTest,
        TestPasswordReuseMatchWhitelistHistogram) {
   const GURL whitelisted_url(kWhitelistedUrl);
   const GURL not_whitelisted_url(kNoneWhitelistedUrl);
   EXPECT_CALL(*database_manager_.get(), MatchCsdWhitelistUrl(whitelisted_url))
       .WillOnce(testing::Return(true));
   EXPECT_CALL(*database_manager_.get(),
               MatchCsdWhitelistUrl(not_whitelisted_url))
       .WillOnce(testing::Return(false));
   histograms_.ExpectTotalCount(kPasswordReuseMatchWhitelistHistogramName, 0);
 
-  // Empty url should not increment metric.
+  // Empty url should increase "True" bucket by 1.
   password_protection_service_->RecordPasswordReuse(GURL());
   base::RunLoop().RunUntilIdle();
-  histograms_.ExpectTotalCount(kPasswordReuseMatchWhitelistHistogramName, 0);
+  EXPECT_THAT(
+      histograms_.GetAllSamples(kPasswordReuseMatchWhitelistHistogramName),
+      testing::ElementsAre(base::Bucket(1, 1)));
 
   // Whitelisted url should increase "True" bucket by 1.
   password_protection_service_->RecordPasswordReuse(whitelisted_url);
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(
       histograms_.GetAllSamples(kPasswordReuseMatchWhitelistHistogramName),
-      testing::ElementsAre(base::Bucket(1, 1)));
+      testing::ElementsAre(base::Bucket(1, 2)));
 
   // Non-whitelisted url should increase "False" bucket by 1.
   password_protection_service_->RecordPasswordReuse(not_whitelisted_url);
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(
       histograms_.GetAllSamples(kPasswordReuseMatchWhitelistHistogramName),
-      testing::ElementsAre(base::Bucket(0, 1), base::Bucket(1, 1)));
+      testing::ElementsAre(base::Bucket(0, 1), base::Bucket(1, 2)));
 }
 
 TEST_F(PasswordProtectionServiceTest, TestParseInvalidVerdictEntry) {
   std::unique_ptr<base::DictionaryValue> invalid_verdict_entry =
       base::MakeUnique<base::DictionaryValue>();
   invalid_verdict_entry->SetString("cache_creation_time", "invalid_time");
 
   int cache_creation_time;
   LoginReputationClientResponse response;
   // ParseVerdictEntry fails if input is empty.
@@ skipping 156 matching lines @@
   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());
   EXPECT_EQ(0U, GetStoredVerdictCount());
 }
+TEST_F(PasswordProtectionServiceTest,
+       TestNoRequestCreatedIfMainFrameURLIsNotValid) {
+  ASSERT_EQ(0u, password_protection_service_->GetPendingRequestsCount());
+  password_protection_service_->MaybeStartLowReputationRequest(GURL(), nullptr);
+  EXPECT_EQ(0u, password_protection_service_->GetPendingRequestsCount());
+}
+
+TEST_F(PasswordProtectionServiceTest,
+       TestNoRequestCreatedIfMainFrameURLIsNotHttpOrHttps) {
+  ASSERT_EQ(0u, password_protection_service_->GetPendingRequestsCount());
+  // If main frame url is data url, don't create request.
+  password_protection_service_->MaybeStartLowReputationRequest(
+      GURL("data:text/html, <p>hellow"), nullptr);
+  EXPECT_EQ(0u, password_protection_service_->GetPendingRequestsCount());
+
+  // If main frame url is ftp, don't create request.
+  password_protection_service_->MaybeStartLowReputationRequest(
+      GURL("ftp://foo.com:21"), nullptr);
+  EXPECT_EQ(0u, password_protection_service_->GetPendingRequestsCount());
+}
 
 TEST_F(PasswordProtectionServiceTest, TestNoRequestSentForIncognito) {
   histograms_.ExpectTotalCount(kRequestOutcomeHistogramName, 0);
   password_protection_service_->set_incognito(true);
   password_protection_service_->StartRequest(
-      GURL(kTargetUrl), LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
+      GURL(kTargetUrl), LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE,
+      nullptr);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(nullptr, password_protection_service_->latest_response());
   EXPECT_THAT(histograms_.GetAllSamples(kRequestOutcomeHistogramName),
               testing::ElementsAre(base::Bucket(7 /* INCOGNITO */, 1)));
 }
 
 TEST_F(PasswordProtectionServiceTest,
        TestNoRequestSentForNonExtendedReporting) {
   histograms_.ExpectTotalCount(kRequestOutcomeHistogramName, 0);
   password_protection_service_->set_extended_reporting(false);
   password_protection_service_->StartRequest(
-      GURL(kTargetUrl), LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
+      GURL(kTargetUrl), LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE,
+      nullptr);
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(nullptr, password_protection_service_->latest_response());
   EXPECT_THAT(
       histograms_.GetAllSamples(kRequestOutcomeHistogramName),
       testing::ElementsAre(base::Bucket(6 /* NO_EXTENDED_REPORTING */, 1)));
 }
 
 TEST_F(PasswordProtectionServiceTest, TestNoRequestSentForWhitelistedURL) {
   histograms_.ExpectTotalCount(kRequestOutcomeHistogramName, 0);
@@ skipping 89 matching lines @@
   EXPECT_EQ(expected_response.cache_duration_sec(),
             actual_response->cache_duration_sec());
 }
 
 TEST_F(PasswordProtectionServiceTest, TestTearDownWithPendingRequests) {
   histograms_.ExpectTotalCount(kRequestOutcomeHistogramName, 0);
   GURL target_url(kTargetUrl);
   EXPECT_CALL(*database_manager_.get(), MatchCsdWhitelistUrl(target_url))
       .WillRepeatedly(testing::Return(false));
   password_protection_service_->StartRequest(
-      target_url, LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
+      target_url, LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, nullptr);
 
   // Destroy password_protection_service_ while there is one request pending.
   password_protection_service_.reset();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_THAT(histograms_.GetAllSamples(kRequestOutcomeHistogramName),
               testing::ElementsAre(base::Bucket(2 /* CANCELED */, 1)));
 }
 }  // namespace safe_browsing