Certificate Transparency: Start tracking logs' state

net/cert/ct_observer.h

+// 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.
+
+#ifndef NET_CERT_CT_OBSERVER_H_
+#define NET_CERT_CT_OBSERVER_H_
+
+#include "net/base/net_export.h"
+#include "net/cert/ct_verifier.h"
+#include "net/cert/sth_observer.h"
+
+namespace net {
+
+namespace ct {
+
+class NET_EXPORT CTObserver : public CTVerifier::Observer, public STHObserver {

[Ryan Sleevi, 2016/04/14 16:56:16]

Seems unnecessary. Why?

[Eran Messeri, 2016/04/18 22:02:12]

To abstract the TreeStateTracker from the IOThread / ProfileIOData.
As far as the IOThread/ProfileIOData are concerned, the ct_observer they hold
cares about STH & SCT notifications.
By creating this interface I avoid having the IOThread/ProfileIOData know about
TreeStateTracker and depending on its definition.

[Ryan Sleevi, 2016/04/21 14:05:15]

That's not true for IOThread -
https://codereview.chromium.org/1845113003/diff/240001/chrome/browser/io_thre...

It's not true for ProfileIOData -
https://codereview.chromium.org/1845113003/diff/240001/chrome/browser/profile...

So what are you trying to accomplish?

[Eran Messeri, 2016/04/25 14:50:59]

The main reasons I've created the CTObserver interface under net/cert rather
than directly use the TreeStateTracker class:
- Not create a dependency on the components/certificate_transparency code
everywhere I have to pass the object that receives and distributes STH
notifications (depending on net/cert seems more natural).
- Clearly document what's the interface client code should care about - so
client code never relies on implementation details of the TreeStateTracker.
- Facilitate testing - that interface could be implemented in test code to test
correct interaction with the IOThread, for example.

If you think all of these reasons are invalid I could remove this interface, but
having it used throughout the code helps me reason about the role(s) of the
TreeStateTracker.

[Ryan Sleevi, 2016/04/27 23:05:58]

Why would you have to? If they're both pure virtual interfaces, and the
certificate_transparency code implements both, then you just pass the object as
the base pointer relevant to the place you're passing it to.
I don't understand this?
I don't understand this?
It's not that I think they're invalid, it's that I'm having trouble following
the logic. To me, this class serves no purpose but to say "These two interfaces
might be coupled" - but the whole point of separate interfaces is because you're
saying "These are distinct functions". If you're having to pass something to
code as a "CTObserver*", it sounds like the dependencies are broken or there's a
short-circuit being taken somewhere. But as I mentioned, I could be
misunderstanding.

+ public:
+  ~CTObserver() override {}
+};
+
+}  // namespace ct
+
+}  // namespace net
+
+#endif  // NET_CERT_CT_OBSERVER_H_