Call CSD whitelist checking on UI thread and record UMA

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
new file mode 100644
index 0000000000000000000000000000000000000000..69ac2e4a1ebb68e41e5bb4a08797556c01b7c4f3
--- /dev/null
+++ b/components/safe_browsing/password_protection/password_protection_service_unittest.cc
@@ -0,0 +1,97 @@
+// 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/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/test/histogram_tester.h"
+#include "components/safe_browsing_db/test_database_manager.h"
+#include "content/public/test/test_browser_thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const char kPasswordReuseMatchWhitelistHistogramName[] =
+    "PasswordManager.PasswordReuse.MainFrameMatchCsdWhitelist";
+const char kWhitelistedUrl[] = "http://inwhitelist.com";
+const char kNoneWhitelistedUrl[] = "http://notinwhitelist.com";
+
+}  // namespace
+
+namespace safe_browsing {
+
+class MockSafeBrowsingDatabaseManager : public TestSafeBrowsingDatabaseManager {
+ public:
+  MockSafeBrowsingDatabaseManager() {}
+
+  MOCK_METHOD1(MatchCsdWhitelistUrl, bool(const GURL&));
+
+ protected:
+  ~MockSafeBrowsingDatabaseManager() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockSafeBrowsingDatabaseManager);
+};
+
+class PasswordProtectionServiceTest : public testing::Test {
+ public:
+  PasswordProtectionServiceTest()
+      : ui_thread_(content::BrowserThread::UI, &message_loop_),
+        io_thread_(content::BrowserThread::IO, &message_loop_){};
+
+  void SetUp() override {
+    database_manager_ = new MockSafeBrowsingDatabaseManager();
+    password_protection_service_ =
+        new PasswordProtectionService(database_manager_);
+  }
+
+  void TearDown() override {
+    // Run all pending tasks or else some threads hold on to the message loop
+    // and prevent it from being deleted.
+    base::RunLoop().RunUntilIdle();
+    database_manager_ = nullptr;

[Nathan Parker, 2017/03/01 22:59:22]

Do we need to null these, or can we rely on the destructor de-ref'ing them?

[Jialiu Lin, 2017/03/02 00:53:07]

Right. it is not necessary. 

+    password_protection_service_ = nullptr;
+  }
+
+ protected:
+  base::MessageLoopForUI message_loop_;

[vakh (use Gerrit instead), 2017/03/01 23:25:20]

Not sure how message_loop_, ui_thread_, and io_thread_ are being used.

Are they?
If yes, could you please document how?

[Jialiu Lin, 2017/03/02 00:53:07]

Just realize ThreatBrowserThread is deprecated. Changed into
TestBrowserThreadBundle instead.

+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread io_thread_;
+  scoped_refptr<MockSafeBrowsingDatabaseManager> database_manager_;
+  scoped_refptr<PasswordProtectionService> password_protection_service_;
+};
+
+TEST_F(PasswordProtectionServiceTest, TestPasswordReuseMatchWhitlistHistogram) {

[vakh (use Gerrit instead), 2017/03/01 23:25:20]

Typo: whitelist

[Jialiu Lin, 2017/03/02 00:53:07]

Good eye!

+  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));
+  base::HistogramTester histograms;
+  histograms.ExpectTotalCount(kPasswordReuseMatchWhitelistHistogramName, 0);
+
+  // Empty url should not increment metric.
+  password_protection_service_->RecordPasswordReuse(GURL());
+  base::RunLoop().RunUntilIdle();
+  histograms.ExpectTotalCount(kPasswordReuseMatchWhitelistHistogramName, 0);
+
+  // Whitelisted url should increase "True" bucket by 1.
+  password_protection_service_->RecordPasswordReuse(GURL(kWhitelistedUrl));

[vakh (use Gerrit instead), 2017/03/01 23:25:20]

Use whitelisted_url instead

[Jialiu Lin, 2017/03/02 00:53:07]

Done.

+  base::RunLoop().RunUntilIdle();
+  EXPECT_THAT(
+      histograms.GetAllSamples(kPasswordReuseMatchWhitelistHistogramName),
+      testing::ElementsAre(base::Bucket(1, 1)));
+
+  // Non-whitelisted url should increase "False" bucket by 1.
+  password_protection_service_->RecordPasswordReuse(GURL(kNoneWhitelistedUrl));

[vakh (use Gerrit instead), 2017/03/01 23:25:20]

Use not_whitelisted_url instead

[Jialiu Lin, 2017/03/02 00:53:07]

Done.

+  base::RunLoop().RunUntilIdle();
+  EXPECT_THAT(
+      histograms.GetAllSamples(kPasswordReuseMatchWhitelistHistogramName),
+      testing::ElementsAre(base::Bucket(0, 1), base::Bucket(1, 1)));
+}
+
+}  // namespace safe_browsing