Restructure SDCH layering to allow more separation (observer/1->[0,n]

net/base/sdch_observer.h

Index: net/base/sdch_observer.h
diff --git a/net/base/sdch_observer.h b/net/base/sdch_observer.h
new file mode 100644
index 0000000000000000000000000000000000000000..59c3c77f343dc5ccd2cb7865bccd73be505c7cfb
--- /dev/null
+++ b/net/base/sdch_observer.h
@@ -0,0 +1,47 @@
+// Copyright 2014 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.
+
+// Observer interface for SDCH.  Observers can register with
+// the SdchManager to receive notifications of various SDCH events.
+
+#ifndef NET_BASE_SDCH_OBSERVER_H_
+#define NET_BASE_SDCH_OBSERVER_H_
+
+#include "net/base/net_export.h"
+
+class GURL;
+
+namespace net {
+
+class SdchManager;
+class URLRequest;
+
+class NET_EXPORT SdchObserver {
+ public:
+  // Notification that SDCH has seen a "Get-Dictionary" header.
+  virtual void OnGetDictionary(SdchManager* manager,
+                               const GURL& request_url,
+                               const GURL& dictionary_url);

[Ryan Sleevi, 2014/10/20 22:42:04]

Why aren't these pure virtual methods, if this is meant to be an interface base
class?

[Randy Smith (Not in Mondays), 2014/10/21 19:05:37]

Ah, this issue :-}.  So I'm following where John Abd-El Malek ended up with the
content refactoring, which is that making things easy for the subclass is the
overarching virtue.  There may be subclasses of SdchObserver that care about
OnGetDictionary but not about OnClearDictionaries, or the reverse, and it's
certainly possible that there'll be subclasses that don't care about
"OnDictionaryUsed()" (which I'll be adding for doing prioritization, and
which'll fire a lot).  More abstractly: It isn't functionally necessary for an
observer to implement any particular interface, since this isn't a delegation
relationship; observers are subscribing for notifications they are interested
in, and not implementing a particular method is how a lack of interest is
indicated.

The counter-argument that I see is that in the current design, we really only
expect there to be a single SdchObserver, and it might be useful to get a
compilation error if it doesn't implement an observer interface, since that
might mean we can get rid of that observer interface.  

+
+  // Notification that SDCH has received a request to clear all
+  // its dictionaries.
+  virtual void OnClearDictionaries(SdchManager* manager);
+
+  virtual ~SdchObserver();
+  SdchObserver();

[Ryan Sleevi, 2014/10/20 22:42:04]

STYLE: Why isn't this ctor, dtor, methods?

[Randy Smith (Not in Mondays), 2014/10/21 19:05:36]

No clue; maybe my fingers slipped? :-}.  Fixed.  

+
+ private:
+  // Reference count implementation to confirm no reference to
+  // this class is left on the SdchManager after class destruction.
+  friend class SdchManager;
+
+  void AddRef();
+  void RemoveRef();
+
+  int reference_count_;

[Ryan Sleevi, 2014/10/20 22:42:04]

This is all sorts of danger zone for me. I need a solid explanation why you're
inventing ref-count semantics here, for a class with a public destructor.

[Randy Smith (Not in Mondays), 2014/10/21 19:05:36]

See response in SdchManager to the dual of this question.

+};
+
+}  // namespace net
+
+#endif  // NET_BASE_SDCH_MANAGER_H_