Fixed invalid reads when XmppClient is destroyed.

BUG=49274
TEST=manual testing with code to trigger bug repro

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

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

3 files changed, 38 insertions, 5 deletions

diff --git a/chrome/browser/sync/notifier/server_notifier_thread.cc b/chrome/browser/sync/notifier/server_notifier_thread.cc
index 4fbfdc1..0e5be0c 100644
--- a/chrome/browser/sync/notifier/server_notifier_thread.cc
+++ b/chrome/browser/sync/notifier/server_notifier_thread.cc
@@ -14,7 +14,8 @@
 namespace sync_notifier {
 
 ServerNotifierThread::ServerNotifierThread(bool use_chrome_async_socket)
-    : notifier::MediatorThreadImpl(use_chrome_async_socket) {}
+    : notifier::MediatorThreadImpl(use_chrome_async_socket),
+      state_(notifier::STATE_CLOSED) {}
 
 ServerNotifierThread::~ServerNotifierThread() {}
 
@@ -72,8 +73,27 @@ void ServerNotifierThread::OnInvalidateAll() {
           &ServerNotifierThread::SignalIncomingNotification));
 }
 
+void ServerNotifierThread::OnClientStateChangeMessage(
+    notifier::LoginConnectionState state) {
+  DCHECK_EQ(MessageLoop::current(), worker_message_loop());
+  state_ = state;
+  if (state_ != notifier::STATE_OPENED) {
+    // Assume anything but an opened state invalidates xmpp_client().
+    StopInvalidationListener();
+  }
+  MediatorThreadImpl::OnClientStateChangeMessage(state);
+}
+
 void ServerNotifierThread::StartInvalidationListener() {
   DCHECK_EQ(MessageLoop::current(), worker_message_loop());
+  if (state_ != notifier::STATE_OPENED) {
+    return;
+  }
+  buzz::XmppClient* client = xmpp_client();
+  if (client == NULL) {
+    LOG(DFATAL) << "xmpp_client() unexpectedly NULL";
+    return;
+  }
 
   StopInvalidationListener();
   chrome_invalidation_client_.reset(new ChromeInvalidationClient());
@@ -83,11 +103,15 @@ 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, xmpp_client());
+  chrome_invalidation_client_->Start(kClientId, this, client);
 }
 
 void ServerNotifierThread::RegisterTypesAndSignalSubscribed() {
   DCHECK_EQ(MessageLoop::current(), worker_message_loop());
+  if (state_ != notifier::STATE_OPENED) {
+    return;
+  }
+
   chrome_invalidation_client_->RegisterTypes();
   parent_message_loop_->PostTask(
       FROM_HERE,
diff --git a/chrome/browser/sync/notifier/server_notifier_thread.h b/chrome/browser/sync/notifier/server_notifier_thread.h
index 4b5ec20..b3990a7 100644
--- a/chrome/browser/sync/notifier/server_notifier_thread.h
+++ b/chrome/browser/sync/notifier/server_notifier_thread.h
@@ -39,7 +39,7 @@ class ServerNotifierThread
   virtual void SubscribeForUpdates(
       const std::vector<std::string>& subscribed_services_list);
 
-  // Overridden to also stop listening to server notifications.
+  // Overridden to stop listening to server notifications.
   virtual void Logout();
 
   // Must not be called.
@@ -51,6 +51,11 @@ class ServerNotifierThread
 
   virtual void OnInvalidateAll();
 
+ protected:
+  // Overridden to know what state we're in.
+  virtual void OnClientStateChangeMessage(
+      notifier::LoginConnectionState state);
+
  private:
   // Posted to the worker thread by ListenForUpdates().
   void StartInvalidationListener();
@@ -68,6 +73,7 @@ class ServerNotifierThread
   // thread by Stop().
   void StopInvalidationListener();
 
+  notifier::LoginConnectionState state_;
   scoped_ptr<ChromeInvalidationClient> chrome_invalidation_client_;
 };
 
diff --git a/jingle/notifier/listener/mediator_thread_impl.h b/jingle/notifier/listener/mediator_thread_impl.h
index 6d4f997..519b2e8 100644
--- a/jingle/notifier/listener/mediator_thread_impl.h
+++ b/jingle/notifier/listener/mediator_thread_impl.h
@@ -81,9 +81,13 @@ class MediatorThreadImpl
   MessageLoop* worker_message_loop();
 
   // Should only be called after OnConnectionStateChange() is called
-  // on the delegate with true.
+  // on the delegate with true and before it is called with false.
   buzz::XmppClient* xmpp_client();
 
+  // This is virtual so that subclasses can also know when the
+  // connection state changes.
+  virtual void OnClientStateChangeMessage(LoginConnectionState state);
+
   Delegate* delegate_;
   MessageLoop* parent_message_loop_;
 
@@ -107,7 +111,6 @@ class MediatorThreadImpl
       const IncomingNotificationData& notification_data);
   void OnOutgoingNotification(bool success);
   void OnLoginFailureMessage(const notifier::LoginFailure& failure);
-  void OnClientStateChangeMessage(LoginConnectionState state);
   void OnSubscriptionStateChange(bool success);
 
   // Equivalents of the above functions called from the parent thread.