Don't drop debug_info when posting to sync server fails

sync/engine/download.cc

 // Copyright 2013 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.
 
 #include "sync/engine/download.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "sync/engine/process_updates_util.h"
@@ skipping 76 matching lines @@
   message->set_share(session->context()->account_name());
   message->set_message_contents(sync_pb::ClientToServerMessage::GET_UPDATES);
 
   sync_pb::GetUpdatesMessage* get_updates = message->mutable_get_updates();
 
   // We want folders for our associated types, always.  If we were to set
   // this to false, the server would send just the non-container items
   // (e.g. Bookmark URLs but not their containing folders).
   get_updates->set_fetch_folders(true);
 
-  sync_pb::DebugInfo* debug_info = message->mutable_debug_info();
-  AppendClientDebugInfoIfNeeded(session->context()->debug_info_getter(),
-                                session->mutable_status_controller(),
-                                debug_info);
-
   get_updates->set_create_mobile_bookmarks_folder(
       create_mobile_bookmarks_folder);
   bool need_encryption_key = ShouldRequestEncryptionKey(session->context());
   get_updates->set_need_encryption_key(need_encryption_key);
 
   // Set legacy GetUpdatesMessage.GetUpdatesCallerInfo information.
   get_updates->mutable_caller_info()->set_notifications_enabled(
       session->context()->notifications_enabled());
 }
 
@@ skipping 225 matching lines @@
 }
 
 SyncerError ExecuteDownloadUpdates(
     ModelTypeSet request_types,
     SyncSession* session,
     sync_pb::ClientToServerMessage* msg) {
   sync_pb::ClientToServerResponse update_response;
   StatusController* status = session->mutable_status_controller();
   bool need_encryption_key = ShouldRequestEncryptionKey(session->context());
 
+  if (session->context()->debug_info_getter()) {
+    sync_pb::DebugInfo* debug_info = msg->mutable_debug_info();
+    CopyClientDebugInfo(session->context()->debug_info_getter(), debug_info);
+  }
+
   SyncerError result = SyncerProtoUtil::PostClientToServerMessage(
       msg,
       &update_response,
       session);
 
   DVLOG(2) << SyncerProtoUtil::ClientToServerResponseDebugString(
       update_response);
 
   if (result != SYNCER_OK) {
     status->mutable_updates_response()->Clear();
     LOG(ERROR) << "PostClientToServerMessage() failed during GetUpdates";
     return result;
   }
 
   status->mutable_updates_response()->CopyFrom(update_response);
 
   DVLOG(1) << "GetUpdates "
            << " returned " << update_response.get_updates().entries_size()
            << " updates and indicated "
            << update_response.get_updates().changes_remaining()
            << " updates left on server.";
 
+  if (session->context()->debug_info_getter()) {
+    // Clear debug info now that we have successfully sent it to the server.
+    DVLOG(1) << "Clearing client debug info.";
+    session->context()->debug_info_getter()->ClearDebugInfo();
+  }
+
   if (need_encryption_key ||
       update_response.get_updates().encryption_keys_size() > 0) {
     syncable::Directory* dir = session->context()->directory();
     status->set_last_get_key_result(
         HandleGetEncryptionKeyResponse(update_response, dir));
   }
 
   const sync_pb::GetUpdatesResponse& gu_response =
       update_response.get_updates();
   status->increment_num_updates_downloaded_by(gu_response.entries_size());
   DCHECK(gu_response.has_changes_remaining());
   status->set_num_server_changes_remaining(gu_response.changes_remaining());
 
   const ModelTypeSet proto_request_types =
       Intersection(request_types, ProtocolTypes());
 
   if (!ProcessUpdateResponseMessage(gu_response,
                                     proto_request_types,
                                     session->context()->update_handler_map(),
                                     status)) {
     return SERVER_RESPONSE_VALIDATION_FAILED;
   } else {
     return result;
   }
 }
 
-void AppendClientDebugInfoIfNeeded(
+void CopyClientDebugInfo(
     sessions::DebugInfoGetter* debug_info_getter,
-    StatusController* status,
     sync_pb::DebugInfo* debug_info) {
-  // We want to send the debug info only once per sync cycle. Check if it has
-  // already been sent.
-  if (!status->debug_info_sent()) {
-    DVLOG(1) << "Sending client debug info ...";
-    // Could be null in some unit tests.
-    if (debug_info_getter) {
-      debug_info_getter->GetAndClearDebugInfo(debug_info);
-    }
-    status->set_debug_info_sent();
+  // Could be null in some unit tests.
+  if (debug_info_getter) {

[Nicolas Zea, 2013/11/19 23:43:04]

nit: you're already checking debug_info_getter before calling this. One of these
checks is unnecessary.

[maniscalco, 2013/11/20 01:03:32]

Done.  Now relying on caller's check.

+    DVLOG(1) << "Copying client debug info to send.";
+    debug_info_getter->GetDebugInfo(debug_info);
   }
 }
 
 }  // namespace download
 
 }  // namespace syncer