Reduce dependency of TiclInvalidationService on Profile

sync/notifier/invalidation_state_tracker.h

Index: sync/notifier/invalidation_state_tracker.h
diff --git a/sync/notifier/invalidation_state_tracker.h b/sync/notifier/invalidation_state_tracker.h
index 81a07eae5c51555792a4b42c7c9d04272cdb3aff..0eff6eee33b6d76c9f7a17b8ec203562647a405e 100644
--- a/sync/notifier/invalidation_state_tracker.h
+++ b/sync/notifier/invalidation_state_tracker.h
@@ -18,6 +18,7 @@
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "google/cacheinvalidation/include/types.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/base/invalidation.h"
@@ -32,12 +33,18 @@ namespace syncer {
 
 class InvalidationStateTracker {
  public:
-  InvalidationStateTracker() {}
+  InvalidationStateTracker() : weak_factory_(this) {}
+  virtual ~InvalidationStateTracker() {}
+
+  base::WeakPtr<InvalidationStateTracker> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();

[dcheng, 2014/04/14 21:39:59]

I don't think we should be publicly exposing weak pointers in the interface.
It's generally considered an antipattern, and ajwong points out that this could
result in if(ptr) returning true for a partially destructed object.

Not sure of a good solution yet though... this is an issue that's been in sync
for a long time, but previously, it was at least a hidden implementation detail
inside chrome/.

A simple (but not necessarily correct fix) might be to make this ref counted...
I'm not sure how I feel about that, but it's definitely better than having this
interface return weak pointers.

[bartfab (slow), 2014/04/14 22:06:01]

Actually, based on discussions we had on chromium-dev, an explicit GetWeakPtr()
method is preferable to tbe implicit SupportsWeakPtr, which is now deprecated
because it makes the risk of accessing a partially destroyed object even worse.

Ref counting is not a solution to the same problem. With weak pointers, you have
explicit ownership and lifetime. Whoever has a naked pointer owns the object.
Everyone else has weak pointers that may become invalid at any point, when the
object is destroyed. With ref counting, ownership is shared and nobody can be
sure when exactly the object gets destroyed. Whoever happens to drop the last
reference will trigger the destructor. This makes it impossible to clean up and
e.g. unregister as an observer in a well-defined manner.

If you are worried about weak pointers being used to access a partially
destroyed object here, I implemented a simple solution: The derived class the
destructor is the very first thing that gets called when an object goes out of
scope. If that destructor calls weak_factory_.InvalidateWeakPtrs(); as the very
first thing, you are guaranteed that nobody can access a partially destructed
object.

+  }
 
   // The per-client unique ID used to register the invalidation client with the
   // server.  This is used to squelch invalidation notifications that originate
-  // from changes made by this client.
-  virtual void SetInvalidatorClientId(const std::string& data) = 0;
+  // from changes made by this client.  Setting the client ID clears all other
+  // state.
+  virtual void ClearAndSetNewClientId(const std::string& data) = 0;
   virtual std::string GetInvalidatorClientId() const = 0;
 
   // Used by invalidation::InvalidationClient for persistence. |data| is an
@@ -56,8 +63,8 @@ class InvalidationStateTracker {
   // Erases invalidation versions, client ID, and state stored on disk.
   virtual void Clear() = 0;
 
- protected:
-  virtual ~InvalidationStateTracker() {}
+ private:
+  base::WeakPtrFactory<InvalidationStateTracker> weak_factory_;
 };
 
 }  // namespace syncer