[Sync] Added some plumbing needed for notifications persistence support

BUG=58556
TEST=ChromeSystemResources unit tests

Review URL: http://codereview.chromium.org/3610015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62155 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 173 insertions, 42 deletions

diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index 08b3885..cdb14ac 100644
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -34,6 +34,7 @@
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_thread.h"
 #include "chrome/browser/sync/notifier/server_notifier_thread.h"
+#include "chrome/browser/sync/notifier/state_writer.h"
 #include "chrome/browser/sync/protocol/app_specifics.pb.h"
 #include "chrome/browser/sync/protocol/autofill_specifics.pb.h"
 #include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
@@ -910,6 +911,7 @@ class BridgedGaiaAuthenticator : public gaia::GaiaAuthenticator {
 class SyncManager::SyncInternal
     : public net::NetworkChangeNotifier::Observer,
       public TalkMediator::Delegate,
+      public sync_notifier::StateWriter,
       public browser_sync::ChannelEventHandler<syncable::DirectoryChangeEvent>,
       public browser_sync::ChannelEventHandler<SyncerEvent>{
   static const int kDefaultNudgeDelayMilliseconds;
@@ -996,6 +998,9 @@ class SyncManager::SyncInternal
 
   virtual void OnOutgoingNotification();
 
+  // sync_notifier::StateWriter implementation.
+  virtual void WriteState(const std::string& state);
+
   // Accessors for the private members.
   DirectoryManager* dir_manager() { return share_.dir_manager.get(); }
   SyncAPIServerConnectionManager* connection_manager() {
@@ -1328,22 +1333,30 @@ bool SyncManager::SyncInternal::Init(
   core_message_loop_->PostTask(FROM_HERE,
       method_factory_.NewRunnableMethod(&SyncInternal::CheckServerReachable));
 
-  // NOTIFICATION_SERVER uses a substantially different notification method, so
-  // it has its own MediatorThread implementation.  Everything else just uses
-  // MediatorThreadImpl.
-  notifier::MediatorThread* mediator_thread =
-      (notifier_options_.notification_method == notifier::NOTIFICATION_SERVER) ?
-      new sync_notifier::ServerNotifierThread(notifier_options) :
-      new notifier::MediatorThreadImpl(notifier_options);
-  talk_mediator_.reset(new TalkMediatorImpl(mediator_thread, false));
-  if (notifier_options_.notification_method != notifier::NOTIFICATION_LEGACY &&
-      notifier_options_.notification_method != notifier::NOTIFICATION_SERVER) {
-    if (notifier_options_.notification_method ==
-        notifier::NOTIFICATION_TRANSITIONAL) {
-      talk_mediator_->AddSubscribedServiceUrl(
-          browser_sync::kSyncLegacyServiceUrl);
+  if (notifier_options_.notification_method ==
+      notifier::NOTIFICATION_SERVER) {
+    // TODO(akalin): Grab persisted state from storage and then
+    // immediately erase it (This simplifies things for
+    // ChromeSystemResources).
+    std::string state;
+    sync_notifier::ServerNotifierThread* server_notifier_thread =
+        new sync_notifier::ServerNotifierThread(
+            notifier_options, state, this);
+    talk_mediator_.reset(
+        new TalkMediatorImpl(server_notifier_thread, false));
+  } else {
+    notifier::MediatorThread* mediator_thread =
+        new notifier::MediatorThreadImpl(notifier_options);
+    talk_mediator_.reset(new TalkMediatorImpl(mediator_thread, false));
+    if (notifier_options_.notification_method !=
+        notifier::NOTIFICATION_LEGACY) {
+      if (notifier_options_.notification_method ==
+          notifier::NOTIFICATION_TRANSITIONAL) {
+        talk_mediator_->AddSubscribedServiceUrl(
+            browser_sync::kSyncLegacyServiceUrl);
+      }
+      talk_mediator_->AddSubscribedServiceUrl(browser_sync::kSyncServiceUrl);
     }
-    talk_mediator_->AddSubscribedServiceUrl(browser_sync::kSyncServiceUrl);
   }
 
   // Listen to TalkMediator events ourselves
@@ -2008,6 +2021,11 @@ void SyncManager::SyncInternal::OnOutgoingNotification() {
   allstatus_.IncrementNotificationsSent();
 }
 
+void SyncManager::SyncInternal::WriteState(const std::string& state) {
+  // TODO(akalin): Write state to persistent storage.
+  NOTIMPLEMENTED();
+}
+
 SyncManager::Status::Summary SyncManager::GetStatusSummary() const {
   return data_->ComputeAggregatedStatusSummary();
 }
diff --git a/chrome/browser/sync/notifier/chrome_invalidation_client.cc b/chrome/browser/sync/notifier/chrome_invalidation_client.cc
index c2505c2..606eb4f 100644
--- a/chrome/browser/sync/notifier/chrome_invalidation_client.cc
+++ b/chrome/browser/sync/notifier/chrome_invalidation_client.cc
@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 
+#include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "chrome/browser/sync/notifier/cache_invalidation_packet_handler.h"
 #include "chrome/browser/sync/notifier/invalidation_util.h"
@@ -19,7 +20,9 @@ namespace sync_notifier {
 ChromeInvalidationClient::Listener::~Listener() {}
 
 ChromeInvalidationClient::ChromeInvalidationClient()
-    : listener_(NULL) {
+    : chrome_system_resources_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      listener_(NULL),
+      state_writer_(NULL) {
   DCHECK(non_thread_safe_.CalledOnValidThread());
 }
 
@@ -27,10 +30,12 @@ ChromeInvalidationClient::~ChromeInvalidationClient() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   Stop();
   DCHECK(!listener_);
+  DCHECK(!state_writer_);
 }
 
 void ChromeInvalidationClient::Start(
-    const std::string& client_id, Listener* listener,
+    const std::string& client_id, const std::string& state,
+    Listener* listener, StateWriter* state_writer,
     base::WeakPtr<talk_base::Task> base_task) {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   DCHECK(base_task.get());
@@ -39,7 +44,11 @@ void ChromeInvalidationClient::Start(
   chrome_system_resources_.StartScheduler();
 
   DCHECK(!listener_);
+  DCHECK(listener);
   listener_ = listener;
+  DCHECK(!state_writer_);
+  DCHECK(state_writer);
+  state_writer_ = state_writer;
 
   invalidation::ClientType client_type;
   client_type.set_type(invalidation::ClientType::CHROME_SYNC);
@@ -50,12 +59,10 @@ void ChromeInvalidationClient::Start(
   // replies we get.
   client_config.max_registrations_per_message = 20;
   client_config.max_ops_per_message = 40;
-  // TODO(akalin): Grab |persisted_state| from persistent storage.
-  std::string persisted_state;
   invalidation_client_.reset(
       new invalidation::InvalidationClientImpl(
           &chrome_system_resources_, client_type, client_id,
-          persisted_state, client_config, this));
+          state, client_config, this));
   cache_invalidation_packet_handler_.reset(
       new CacheInvalidationPacketHandler(base_task,
                                          invalidation_client_.get()));
@@ -76,6 +83,7 @@ void ChromeInvalidationClient::Stop() {
   registration_manager_.reset();
   cache_invalidation_packet_handler_.reset();
   invalidation_client_.reset();
+  state_writer_ = NULL;
   listener_ = NULL;
 }
 
@@ -144,4 +152,8 @@ void ChromeInvalidationClient::RegistrationLost(
   RunAndDeleteClosure(callback);
 }
 
+void ChromeInvalidationClient::WriteState(const std::string& state) {
+  state_writer_->WriteState(state);
+}
+
 }  // namespace sync_notifier
diff --git a/chrome/browser/sync/notifier/chrome_invalidation_client.h b/chrome/browser/sync/notifier/chrome_invalidation_client.h
index 6d87d70..7cbc3f8 100644
--- a/chrome/browser/sync/notifier/chrome_invalidation_client.h
+++ b/chrome/browser/sync/notifier/chrome_invalidation_client.h
@@ -16,6 +16,7 @@
 #include "base/scoped_ptr.h"
 #include "base/weak_ptr.h"
 #include "chrome/browser/sync/notifier/chrome_system_resources.h"
+#include "chrome/browser/sync/notifier/state_writer.h"
 #include "chrome/browser/sync/syncable/model_type.h"
 #include "google/cacheinvalidation/invalidation-client.h"
 
@@ -31,7 +32,9 @@ class RegistrationManager;
 // TODO(akalin): Hook this up to a NetworkChangeNotifier so we can
 // properly notify invalidation_client_.
 
-class ChromeInvalidationClient : public invalidation::InvalidationListener {
+class ChromeInvalidationClient
+    : public invalidation::InvalidationListener,
+      public StateWriter {
  public:
   class Listener {
    public:
@@ -47,10 +50,11 @@ class ChromeInvalidationClient : public invalidation::InvalidationListener {
   // Calls Stop().
   virtual ~ChromeInvalidationClient();
 
-  // Does not take ownership of |listener|.  |base_task| must still be
-  // non-NULL.
+  // Does not take ownership of |listener| or |state_writer|.
+  // |base_task| must still be non-NULL.
   void Start(
-      const std::string& client_id, Listener* listener,
+      const std::string& client_id, const std::string& state,
+      Listener* listener, StateWriter* state_writer,
       base::WeakPtr<talk_base::Task> base_task);
 
   void Stop();
@@ -74,10 +78,14 @@ class ChromeInvalidationClient : public invalidation::InvalidationListener {
   virtual void RegistrationLost(const invalidation::ObjectId& object_id,
                                 invalidation::Closure* callback);
 
+  // StateWriter implementation.
+  virtual void WriteState(const std::string& state);
+
  private:
   NonThreadSafe non_thread_safe_;
   ChromeSystemResources chrome_system_resources_;
   Listener* listener_;
+  StateWriter* state_writer_;
   scoped_ptr<invalidation::InvalidationClient> invalidation_client_;
   scoped_ptr<CacheInvalidationPacketHandler>
       cache_invalidation_packet_handler_;
diff --git a/chrome/browser/sync/notifier/chrome_system_resources.cc b/chrome/browser/sync/notifier/chrome_system_resources.cc
index 2f851b6..b26874b 100644
--- a/chrome/browser/sync/notifier/chrome_system_resources.cc
+++ b/chrome/browser/sync/notifier/chrome_system_resources.cc
@@ -16,8 +16,10 @@
 
 namespace sync_notifier {
 
-ChromeSystemResources::ChromeSystemResources() {
+ChromeSystemResources::ChromeSystemResources(StateWriter* state_writer)
+    : state_writer_(state_writer) {
   DCHECK(non_thread_safe_.CalledOnValidThread());
+  DCHECK(state_writer_);
 }
 
 ChromeSystemResources::~ChromeSystemResources() {
@@ -111,8 +113,12 @@ void ChromeSystemResources::Log(
 void ChromeSystemResources::WriteState(
     const invalidation::string& state,
     invalidation::StorageCallback* callback) {
-  // TODO(akalin): Write the given state to persistent storage.
-  callback->Run(false);
+  CHECK(state_writer_);
+  state_writer_->WriteState(state);
+  // According to the cache invalidation API folks, we can do this as
+  // long as we make sure to clear the persistent state that we start
+  // up the cache invalidation client with.
+  callback->Run(true);
   delete callback;
 }
 
diff --git a/chrome/browser/sync/notifier/chrome_system_resources.h b/chrome/browser/sync/notifier/chrome_system_resources.h
index c9e79b7..4631e31 100644
--- a/chrome/browser/sync/notifier/chrome_system_resources.h
+++ b/chrome/browser/sync/notifier/chrome_system_resources.h
@@ -16,13 +16,14 @@
 #include "base/non_thread_safe.h"
 #include "base/scoped_ptr.h"
 #include "base/task.h"
+#include "chrome/browser/sync/notifier/state_writer.h"
 #include "google/cacheinvalidation/invalidation-client.h"
 
 namespace sync_notifier {
 
 class ChromeSystemResources : public invalidation::SystemResources {
  public:
-  ChromeSystemResources();
+  explicit ChromeSystemResources(StateWriter* state_writer);
 
   ~ChromeSystemResources();
 
@@ -55,6 +56,7 @@ class ChromeSystemResources : public invalidation::SystemResources {
       scoped_runnable_method_factory_;
   // Holds all posted tasks that have not yet been run.
   std::set<invalidation::Closure*> posted_tasks_;
+  StateWriter* state_writer_;
 
   // If the scheduler has been started, inserts |task| into
   // |posted_tasks_| and returns a Task* to post.  Otherwise,
diff --git a/chrome/browser/sync/notifier/chrome_system_resources_unittest.cc b/chrome/browser/sync/notifier/chrome_system_resources_unittest.cc
index 0ee480b..12b4080 100644
--- a/chrome/browser/sync/notifier/chrome_system_resources_unittest.cc
+++ b/chrome/browser/sync/notifier/chrome_system_resources_unittest.cc
@@ -4,13 +4,24 @@
 
 #include "chrome/browser/sync/notifier/chrome_system_resources.h"
 
+#include <string>
+
 #include "base/message_loop.h"
 #include "google/cacheinvalidation/invalidation-client.h"
+#include "chrome/browser/sync/notifier/state_writer.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace sync_notifier {
 namespace {
 
+using ::testing::_;
+
+class MockStateWriter : public StateWriter {
+ public:
+  MOCK_METHOD1(WriteState, void(const std::string&));
+};
+
 void ShouldNotRun() {
   ADD_FAILURE();
 }
@@ -23,12 +34,14 @@ class ChromeSystemResourcesTest : public testing::Test {
   }
 
   // Used as a callback.
-  void ExpectFalse(bool result) {
-    EXPECT_FALSE(result);
+  void ExpectTrue(bool result) {
+    EXPECT_TRUE(result);
   }
 
  protected:
-  ChromeSystemResourcesTest() : counter_(0) {}
+  ChromeSystemResourcesTest() :
+      chrome_system_resources_(&mock_state_writer_),
+      counter_(0) {}
 
   virtual ~ChromeSystemResourcesTest() {}
 
@@ -42,6 +55,7 @@ class ChromeSystemResourcesTest : public testing::Test {
 
   // Needed by |chrome_system_resources_|.
   MessageLoop message_loop_;
+  MockStateWriter mock_state_writer_;
   ChromeSystemResources chrome_system_resources_;
   int counter_;
 
@@ -120,11 +134,13 @@ TEST_F(ChromeSystemResourcesTest, ScheduleWithZeroDelay) {
 // TODO(akalin): Figure out how to test with a non-zero delay.
 
 TEST_F(ChromeSystemResourcesTest, WriteState) {
+  EXPECT_CALL(mock_state_writer_, WriteState(_));
+
   // Explicitness hack here to work around broken callback
   // implementations.
   ChromeSystemResourcesTest* run_object = this;
   void (ChromeSystemResourcesTest::*run_function)(bool) =
-      &ChromeSystemResourcesTest::ExpectFalse;
+      &ChromeSystemResourcesTest::ExpectTrue;
 
   chrome_system_resources_.WriteState(
       "state", invalidation::NewPermanentCallback(run_object, run_function));
diff --git a/chrome/browser/sync/notifier/server_notifier_thread.cc b/chrome/browser/sync/notifier/server_notifier_thread.cc
index b1566a8..5251255 100644
--- a/chrome/browser/sync/notifier/server_notifier_thread.cc
+++ b/chrome/browser/sync/notifier/server_notifier_thread.cc
@@ -16,10 +16,13 @@
 namespace sync_notifier {
 
 ServerNotifierThread::ServerNotifierThread(
-    const notifier::NotifierOptions& notifier_options)
-    : notifier::MediatorThreadImpl(notifier_options) {
+    const notifier::NotifierOptions& notifier_options,
+    const std::string& state, StateWriter* state_writer)
+    : notifier::MediatorThreadImpl(notifier_options),
+      state_(state), state_writer_(state_writer) {
   DCHECK_EQ(notifier::NOTIFICATION_SERVER,
             notifier_options.notification_method);
+  DCHECK(state_writer_);
 }
 
 ServerNotifierThread::~ServerNotifierThread() {}
@@ -43,6 +46,7 @@ void ServerNotifierThread::SubscribeForUpdates(
 
 void ServerNotifierThread::Logout() {
   DCHECK_EQ(MessageLoop::current(), parent_message_loop_);
+  state_writer_ = NULL;
   worker_message_loop()->PostTask(
       FROM_HERE,
       NewRunnableMethod(this,
@@ -85,6 +89,16 @@ void ServerNotifierThread::OnInvalidateAll() {
           &ServerNotifierThread::SignalIncomingNotification));
 }
 
+void ServerNotifierThread::WriteState(const std::string& state) {
+  DCHECK_EQ(MessageLoop::current(), worker_message_loop());
+  VLOG(1) << "WriteState";
+  parent_message_loop_->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &ServerNotifierThread::SignalWriteState, state));
+}
+
 void ServerNotifierThread::OnDisconnect() {
   DCHECK_EQ(MessageLoop::current(), worker_message_loop());
   StopInvalidationListener();
@@ -105,7 +119,9 @@ void ServerNotifierThread::StartInvalidationListener() {
   // make it so that we won't receive any notifications that were
   // generated from our own changes.
   const std::string kClientId = "server_notifier_thread";
-  chrome_invalidation_client_->Start(kClientId, this, base_task_);
+  chrome_invalidation_client_->Start(
+      kClientId, state_, this, this, base_task_);
+  state_.clear();
 }
 
 void ServerNotifierThread::RegisterTypesAndSignalSubscribed() {
@@ -139,6 +155,13 @@ void ServerNotifierThread::SignalIncomingNotification() {
   }
 }
 
+void ServerNotifierThread::SignalWriteState(const std::string& state) {
+  DCHECK_EQ(MessageLoop::current(), parent_message_loop_);
+  if (state_writer_) {
+    state_writer_->WriteState(state);
+  }
+}
+
 void ServerNotifierThread::StopInvalidationListener() {
   DCHECK_EQ(MessageLoop::current(), worker_message_loop());
   chrome_invalidation_client_.reset();
diff --git a/chrome/browser/sync/notifier/server_notifier_thread.h b/chrome/browser/sync/notifier/server_notifier_thread.h
index 4d66f42..5e48379 100644
--- a/chrome/browser/sync/notifier/server_notifier_thread.h
+++ b/chrome/browser/sync/notifier/server_notifier_thread.h
@@ -18,6 +18,7 @@
 
 #include "base/scoped_ptr.h"
 #include "chrome/browser/sync/notifier/chrome_invalidation_client.h"
+#include "chrome/browser/sync/notifier/state_writer.h"
 #include "chrome/browser/sync/syncable/model_type.h"
 #include "jingle/notifier/listener/mediator_thread_impl.h"
 
@@ -29,10 +30,14 @@ namespace sync_notifier {
 
 class ServerNotifierThread
     : public notifier::MediatorThreadImpl,
-      public ChromeInvalidationClient::Listener {
+      public ChromeInvalidationClient::Listener,
+      public StateWriter {
  public:
+  // Does not take ownership of |state_writer| (which may not
+  // be NULL).
   explicit ServerNotifierThread(
-      const notifier::NotifierOptions& notifier_options);
+      const notifier::NotifierOptions& notifier_options,
+      const std::string& state, StateWriter* state_writer);
 
   virtual ~ServerNotifierThread();
 
@@ -52,11 +57,12 @@ class ServerNotifierThread
   virtual void SendNotification(const OutgoingNotificationData& data);
 
   // ChromeInvalidationClient::Listener implementation.
-
   virtual void OnInvalidate(syncable::ModelType model_type);
-
   virtual void OnInvalidateAll();
 
+  // StateWriter implementation.
+  virtual void WriteState(const std::string& state);
+
  protected:
   virtual void OnDisconnect();
 
@@ -73,10 +79,16 @@ class ServerNotifierThread
   // Signal to the delegate that we have an incoming notification.
   void SignalIncomingNotification();
 
+  // Signal to the delegate to write the state.
+  void SignalWriteState(const std::string& state);
+
   // Called by StartInvalidationListener() and posted to the worker
   // thread by Stop().
   void StopInvalidationListener();
 
+  std::string state_;
+  // May be NULL.
+  StateWriter* state_writer_;
   scoped_ptr<ChromeInvalidationClient> chrome_invalidation_client_;
 };
 
diff --git a/chrome/browser/sync/notifier/state_writer.h b/chrome/browser/sync/notifier/state_writer.h
new file mode 100644
index 0000000..84ac16a
--- /dev/null
+++ b/chrome/browser/sync/notifier/state_writer.h
@@ -0,0 +1,24 @@
+// Copyright (c) 2010 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.
+//
+// Simple interface for something that persists state.
+
+#ifndef CHROME_BROWSER_SYNC_NOTIFIER_STATE_WRITER_H_
+#define CHROME_BROWSER_SYNC_NOTIFIER_STATE_WRITER_H_
+#pragma once
+
+#include <string>
+
+namespace sync_notifier {
+
+class StateWriter {
+ public:
+  virtual ~StateWriter() {}
+
+  virtual void WriteState(const std::string& state) = 0;
+};
+
+}  // sync_notifier
+
+#endif  // CHROME_BROWSER_SYNC_NOTIFIER_STATE_WRITER_H_
diff --git a/chrome/browser/sync/tools/sync_listen_notifications.cc b/chrome/browser/sync/tools/sync_listen_notifications.cc
index e436a81..5c86fa4 100644
--- a/chrome/browser/sync/tools/sync_listen_notifications.cc
+++ b/chrome/browser/sync/tools/sync_listen_notifications.cc
@@ -6,6 +6,7 @@
 #include <vector>
 
 #include "base/at_exit.h"
+#include "base/base64.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -157,7 +158,8 @@ class ChromeInvalidationListener
 
 // Delegate for server-side notifications.
 class CacheInvalidationNotifierDelegate
-    : public XmppNotificationClient::Observer {
+    : public XmppNotificationClient::Observer,
+      public sync_notifier::StateWriter {
  public:
   CacheInvalidationNotifierDelegate() {}
 
@@ -168,9 +170,10 @@ class CacheInvalidationNotifierDelegate
 
     // TODO(akalin): app_name should be per-client unique.
     const std::string kAppName = "cc_sync_listen_notifications";
-    chrome_invalidation_client_.Start(kAppName,
+    std::string state = "";
+    chrome_invalidation_client_.Start(kAppName, state,
                                       &chrome_invalidation_listener_,
-                                      base_task);
+                                      this, base_task);
     chrome_invalidation_client_.RegisterTypes();
   }
 
@@ -178,6 +181,12 @@ class CacheInvalidationNotifierDelegate
     chrome_invalidation_client_.Stop();
   }
 
+  virtual void WriteState(const std::string& state) {
+    std::string base64_state;
+    CHECK(base::Base64Encode(state, &base64_state));
+    LOG(INFO) << "Writing state: " << base64_state;
+  }
+
  private:
   ChromeInvalidationListener chrome_invalidation_listener_;
   sync_notifier::ChromeInvalidationClient chrome_invalidation_client_;
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 0e18479..572449d 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -1085,6 +1085,7 @@
         'browser/sync/notifier/registration_manager.h',
         'browser/sync/notifier/server_notifier_thread.cc',
         'browser/sync/notifier/server_notifier_thread.h',
+        'browser/sync/notifier/state_writer.h',
       ],
       'include_dirs': [
         '..',