Certificate Transparency: Notify STH Observers of known STHs

net/cert/sth_distributor_unittest.cc

Index: net/cert/sth_distributor_unittest.cc
diff --git a/net/cert/sth_distributor_unittest.cc b/net/cert/sth_distributor_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0eeba852a0ba566fdddf614c07c3082b4f9a1a8a
--- /dev/null
+++ b/net/cert/sth_distributor_unittest.cc
@@ -0,0 +1,76 @@
+// Copyright 2016 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 "net/cert/sth_distributor.h"
+
+#include <memory>
+#include <string>
+
+#include "base/test/histogram_tester.h"
+#include "crypto/sha2.h"
+#include "net/cert/signed_tree_head.h"
+#include "net/test/ct_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace ct {
+
+namespace {
+const char kAnotherLogName[] = "another log";

[Ryan Sleevi, 2016/05/12 19:55:06]

This doesn't need to be file-level, does it?

[Eran Messeri, 2016/05/13 09:04:09]

Done.

+const char kPilotSTHAgeHistogram[] = "Net.CertificateTransparency.PilotSTHAge";
+const char kPilotLogID[33] =
+    "\xa4\xb9\x09\x90\xb4\x18\x58\x14\x87\xbb\x13\xa2\xcc\x67\x70\x0a\x3c\x35"
+    "\x98\x04\xf9\x1b\xdf\xb8\xe3\x77\xcd\x0e\xc8\x0d\xdc\x10";

[Ryan Sleevi, 2016/05/12 19:55:06]

The first two variables don't need to be file-level, do they?

[Eran Messeri, 2016/05/13 09:04:09]

Done.

+}
+
+class StoringSTHObserver : public net::ct::STHObserver {

[Ryan Sleevi, 2016/05/12 19:55:06]

Document

[Ryan Sleevi, 2016/05/12 19:55:06]

IWYU: STHObserver

[Eran Messeri, 2016/05/13 09:04:08]

Done.

[Eran Messeri, 2016/05/13 09:04:09]

Done.

+ public:
+  void NewSTHObserved(const net::ct::SignedTreeHead& sth) override {
+    sths[sth.log_id] = sth;
+  }
+
+  std::map<std::string, net::ct::SignedTreeHead> sths;

[Ryan Sleevi, 2016/05/12 19:55:06]

IWYU: map

[Eran Messeri, 2016/05/13 09:04:09]

Done.

+};
+
+class STHDistributorTest : public ::testing::Test {
+ public:
+  STHDistributorTest() {}
+
+  void SetUp() override {
+    distributor_.reset(new STHDistributor());

[Ryan Sleevi, 2016/05/12 19:55:06]

There's no need to do this in SetUp versus the constructor, is there? Is there a
reason the STHDistributor isn't just a member?

[Eran Messeri, 2016/05/13 09:04:09]

Done.

+    ASSERT_TRUE(GetSampleSignedTreeHead(&sample_sth_));
+  }
+
+ protected:
+  std::unique_ptr<STHDistributor> distributor_;
+  SignedTreeHead sample_sth_;
+};
+
+TEST_F(STHDistributorTest, NotifiesOfExistingSTHs) {
+  SignedTreeHead second_sth(sample_sth_);
+  second_sth.log_id = std::string(kAnotherLogName);

[Ryan Sleevi, 2016/05/12 19:55:06]

Doesn't need to be a constant, does it?

[Eran Messeri, 2016/05/13 09:04:08]

Done.

+
+  distributor_->NewSTHObserved(sample_sth_);
+  distributor_->NewSTHObserved(second_sth);
+
+  StoringSTHObserver observer;
+  distributor_->RegisterObserver(&observer);
+
+  EXPECT_EQ(2u, observer.sths.size());
+  EXPECT_EQ(1u, observer.sths.count(kAnotherLogName));

[Ryan Sleevi, 2016/05/12 19:55:06]

Document what you're testing, and why. You should be clear about what the
expectations are trying to test.

[Eran Messeri, 2016/05/13 09:04:09]

Done.

+}
+
+TEST_F(STHDistributorTest, LogsUMAForPilotSTH) {
+  base::HistogramTester histograms;
+  histograms.ExpectTotalCount(kPilotSTHAgeHistogram, 0);
+  sample_sth_.log_id = std::string(kPilotLogID, crypto::kSHA256Length);
+
+  distributor_->NewSTHObserved(sample_sth_);
+  histograms.ExpectTotalCount(kPilotSTHAgeHistogram, 1);
+}
+
+}  // namespace ct
+
+}  // namespace net