Certificate Transparency: Notify STH Observers of known STHs

net/cert/sth_distributor.cc

Index: net/cert/sth_distributor.cc
diff --git a/net/cert/sth_distributor.cc b/net/cert/sth_distributor.cc
index 0b8df2606660cd0e60a47b9756bc7d3d3b5a95fa..6423c343db4ce10f8aba8223e5da8c0d2a14efc0 100644
--- a/net/cert/sth_distributor.cc
+++ b/net/cert/sth_distributor.cc
@@ -18,12 +18,42 @@ namespace net {
 
 namespace ct {
 
+namespace {
+
+// Compares SignedTreeHeads by their log ID, to facilitate creation of a
+// map of STHs where there's only one STH for each log.

[Ryan Sleevi, 2016/05/13 15:41:12]

Documentation isn't correct because this isn't a map

[Eran Messeri, 2016/05/16 13:55:14]

Removed the struct so the documentation is no longer there.

+struct STHLogIDComparator {
+  STHLogIDComparator(const std::string& log_id) : log_id_(log_id) {}
+  bool operator()(const SignedTreeHead& other) {
+    return other.log_id == log_id_;
+  }
+
+ private:
+  std::string log_id_;
+};
+
+}  // namespace
+
 STHDistributor::STHDistributor()
     : observer_list_(base::ObserverList<STHObserver>::NOTIFY_EXISTING_ONLY) {}
 
 STHDistributor::~STHDistributor() {}
 
 void STHDistributor::NewSTHObserved(const SignedTreeHead& sth) {
+  auto it = std::find_if(observed_sths_.begin(), observed_sths_.end(),
+                         STHLogIDComparator(sth.log_id));

[Ryan Sleevi, 2016/05/13 15:41:12]

auto it = std::find_if(observed_sths_.begin(), observed_sths_.end(),
[&sth](const SignedTreeHead& other) { return sth.log_id == other.log_id });

[Eran Messeri, 2016/05/16 13:55:14]

Thanks, I wasn't aware lambdas were allowed in Chromium code.

+
+  if (it == observed_sths_.end())
+    observed_sths_.push_back(sth);
+  else
+    *it = sth;
+
+  for (auto observed_sth : observed_sths_) {
+    if (observed_sth.log_id == sth.log_id) {
+      observed_sth = sth;
+    }
+  }

[Ryan Sleevi, 2016/05/13 15:41:12]

Why do you do this for loop? It seems entirely redundant with 46-49?

Also, I feel like I've pointed out a few times before, but please read
https://chromium-cpp.appspot.com/ on Range-based for loops, and see the rule of
thumb.

[Eran Messeri, 2016/05/16 13:55:14]

My bad - leftover code. Removed. You have pointed out chromium-cpp resource
about Range-based for loops in the past. Removed.

+
   FOR_EACH_OBSERVER(STHObserver, observer_list_, NewSTHObserved(sth));
 
   if (sth.log_id.compare(0, sth.log_id.size(), kPilotLogID,
@@ -38,6 +68,8 @@ void STHDistributor::NewSTHObserved(const SignedTreeHead& sth) {
 
 void STHDistributor::RegisterObserver(STHObserver* observer) {
   observer_list_.AddObserver(observer);
+  for (const auto& sth : observed_sths_)
+    observer->NewSTHObserved(sth);
 }
 
 void STHDistributor::UnregisterObserver(STHObserver* observer) {