Don't clear debug info until after it has been sent to the server.

Replaced DebugInfoGetter::GetAndClearDebugInfo with GetDebugInfo and ClearDebugInfo so we can clear the debug info only after we have successfully sent it to the server.

Moved MockDebugInfoGetter into its own file so it can be used by other tests.

BUG=319937

Review URL: https://codereview.chromium.org/61213009

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

5 files changed, 38 insertions, 20 deletions

diff --git a/sync/internal_api/debug_info_event_listener.cc b/sync/internal_api/debug_info_event_listener.cc
index 7875ed9..f46c4ee 100644
--- a/sync/internal_api/debug_info_event_listener.cc
+++ b/sync/internal_api/debug_info_event_listener.cc
@@ -144,22 +144,28 @@ void DebugInfoEventListener::OnIncomingNotification(
   AddEventToQueue(event_info);
 }
 
-void DebugInfoEventListener::GetAndClearDebugInfo(
-    sync_pb::DebugInfo* debug_info) {
+void DebugInfoEventListener::GetDebugInfo(sync_pb::DebugInfo* debug_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_LE(events_.size(), kMaxEntries);
-  while (!events_.empty()) {
+
+  for (DebugEventInfoQueue::const_iterator iter = events_.begin();
+       iter != events_.end();
+       ++iter) {
     sync_pb::DebugEventInfo* event_info = debug_info->add_events();
-    const sync_pb::DebugEventInfo& debug_event_info = events_.front();
-    event_info->CopyFrom(debug_event_info);
-    events_.pop();
+    event_info->CopyFrom(*iter);
   }
 
   debug_info->set_events_dropped(events_dropped_);
   debug_info->set_cryptographer_ready(cryptographer_ready_);
   debug_info->set_cryptographer_has_pending_keys(
       cryptographer_has_pending_keys_);
+}
+
+void DebugInfoEventListener::ClearDebugInfo() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_LE(events_.size(), kMaxEntries);
 
+  events_.clear();
   events_dropped_ = false;
 }
 
@@ -255,10 +261,10 @@ void DebugInfoEventListener::AddEventToQueue(
     DVLOG(1) << "DebugInfoEventListener::AddEventToQueue Dropping an old event "
              << "because of full queue";
 
-    events_.pop();
+    events_.pop_front();
     events_dropped_ = true;
   }
-  events_.push(event_info);
+  events_.push_back(event_info);
 }
 
 }  // namespace syncer
diff --git a/sync/internal_api/debug_info_event_listener.h b/sync/internal_api/debug_info_event_listener.h
index ee12c54..15cc0a6 100644
--- a/sync/internal_api/debug_info_event_listener.h
+++ b/sync/internal_api/debug_info_event_listener.h
@@ -5,7 +5,7 @@
 #ifndef SYNC_INTERNAL_API_DEBUG_INFO_EVENT_LISTENER_H_
 #define SYNC_INTERNAL_API_DEBUG_INFO_EVENT_LISTENER_H_
 
-#include <queue>
+#include <deque>
 #include <string>
 
 #include "base/compiler_specific.h"
@@ -75,7 +75,10 @@ class SYNC_EXPORT_PRIVATE DebugInfoEventListener
   void OnIncomingNotification(const ObjectIdInvalidationMap& invalidations);
 
   // DebugInfoGetter implementation.
-  virtual void GetAndClearDebugInfo(sync_pb::DebugInfo* debug_info) OVERRIDE;
+  virtual void GetDebugInfo(sync_pb::DebugInfo* debug_info) OVERRIDE;
+
+  // DebugInfoGetter implementation.
+  virtual void ClearDebugInfo() OVERRIDE;
 
   // DataTypeDebugInfoListener implementation.
   virtual void OnDataTypeConfigureComplete(
@@ -88,11 +91,14 @@ class SYNC_EXPORT_PRIVATE DebugInfoEventListener
  private:
   FRIEND_TEST_ALL_PREFIXES(DebugInfoEventListenerTest, VerifyEventsAdded);
   FRIEND_TEST_ALL_PREFIXES(DebugInfoEventListenerTest, VerifyQueueSize);
-  FRIEND_TEST_ALL_PREFIXES(DebugInfoEventListenerTest, VerifyGetAndClearEvents);
+  FRIEND_TEST_ALL_PREFIXES(DebugInfoEventListenerTest, VerifyGetEvents);
+  FRIEND_TEST_ALL_PREFIXES(DebugInfoEventListenerTest, VerifyClearEvents);
 
   void AddEventToQueue(const sync_pb::DebugEventInfo& event_info);
   void CreateAndAddEvent(sync_pb::DebugEventInfo::SingletonEventType type);
-  std::queue<sync_pb::DebugEventInfo> events_;
+
+  typedef std::deque<sync_pb::DebugEventInfo> DebugEventInfoQueue;
+  DebugEventInfoQueue events_;
 
   // True indicates we had to drop one or more events to keep our limit of
   // |kMaxEntries|.
diff --git a/sync/internal_api/debug_info_event_listener_unittest.cc b/sync/internal_api/debug_info_event_listener_unittest.cc
index 131728b..31f16f8 100644
--- a/sync/internal_api/debug_info_event_listener_unittest.cc
+++ b/sync/internal_api/debug_info_event_listener_unittest.cc
@@ -29,23 +29,33 @@ TEST_F(DebugInfoEventListenerTest, VerifyQueueSize) {
         sync_pb::DebugEventInfo::ENCRYPTION_COMPLETE);
   }
   sync_pb::DebugInfo debug_info;
-  debug_info_event_listener.GetAndClearDebugInfo(&debug_info);
+  debug_info_event_listener.GetDebugInfo(&debug_info);
+  debug_info_event_listener.ClearDebugInfo();
   ASSERT_TRUE(debug_info.events_dropped());
   ASSERT_EQ(static_cast<int>(kMaxEntries), debug_info.events_size());
 }
 
-TEST_F(DebugInfoEventListenerTest, VerifyGetAndClearEvents) {
+TEST_F(DebugInfoEventListenerTest, VerifyGetEvents) {
   DebugInfoEventListener debug_info_event_listener;
   debug_info_event_listener.CreateAndAddEvent(
       sync_pb::DebugEventInfo::ENCRYPTION_COMPLETE);
   ASSERT_EQ(debug_info_event_listener.events_.size(), 1U);
   sync_pb::DebugInfo debug_info;
-  debug_info_event_listener.GetAndClearDebugInfo(&debug_info);
-  ASSERT_EQ(debug_info_event_listener.events_.size(), 0U);
+  debug_info_event_listener.GetDebugInfo(&debug_info);
+  ASSERT_EQ(debug_info_event_listener.events_.size(), 1U);
   ASSERT_EQ(debug_info.events_size(), 1);
   ASSERT_TRUE(debug_info.events(0).has_singleton_event());
   ASSERT_EQ(debug_info.events(0).singleton_event(),
             sync_pb::DebugEventInfo::ENCRYPTION_COMPLETE);
 }
 
+TEST_F(DebugInfoEventListenerTest, VerifyClearEvents) {
+  DebugInfoEventListener debug_info_event_listener;
+  debug_info_event_listener.CreateAndAddEvent(
+      sync_pb::DebugEventInfo::ENCRYPTION_COMPLETE);
+  ASSERT_EQ(debug_info_event_listener.events_.size(), 1U);
+  debug_info_event_listener.ClearDebugInfo();
+  ASSERT_EQ(debug_info_event_listener.events_.size(), 0U);
+}
+
 }  // namespace syncer
diff --git a/sync/internal_api/public/sessions/model_neutral_state.cc b/sync/internal_api/public/sessions/model_neutral_state.cc
index b1a1706..fa2b019 100644
--- a/sync/internal_api/public/sessions/model_neutral_state.cc
+++ b/sync/internal_api/public/sessions/model_neutral_state.cc
@@ -23,7 +23,6 @@ ModelNeutralState::ModelNeutralState()
       last_download_updates_result(UNSET),
       commit_result(UNSET),
       items_committed(false),
-      debug_info_sent(false),
       num_server_changes_remaining(0) {
 }
 
diff --git a/sync/internal_api/public/sessions/model_neutral_state.h b/sync/internal_api/public/sessions/model_neutral_state.h
index 3a9d744..ee9c974 100644
--- a/sync/internal_api/public/sessions/model_neutral_state.h
+++ b/sync/internal_api/public/sessions/model_neutral_state.h
@@ -63,9 +63,6 @@ struct SYNC_EXPORT ModelNeutralState {
   // Set to true by PostCommitMessageCommand if any commits were successful.
   bool items_committed;
 
-  // True indicates debug info has been sent once this session.
-  bool debug_info_sent;
-
   // Number of changes remaining, according to the server.
   // Take it as an estimate unless it's value is zero, in which case there
   // really is nothing more to download.