Fix sending multiple feedback reports within short durations of each other

chrome/browser/extensions/api/feedback_private/feedback_service.cc

Index: chrome/browser/extensions/api/feedback_private/feedback_service.cc
diff --git a/chrome/browser/extensions/api/feedback_private/feedback_service.cc b/chrome/browser/extensions/api/feedback_private/feedback_service.cc
index 28a72c6b9329ac0d3cbefbe64b148e5a48487a93..91647802135f5b735275bf0fa44c87279ad31272 100644
--- a/chrome/browser/extensions/api/feedback_private/feedback_service.cc
+++ b/chrome/browser/extensions/api/feedback_private/feedback_service.cc
@@ -8,6 +8,7 @@
 
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
+#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
@@ -17,10 +18,14 @@
 using content::BrowserThread;
 using feedback::FeedbackData;
 
+namespace extensions {
+
 namespace {
 
-void PopulateSystemInfo(
-    extensions::SystemInformationList* sys_info_list,
+// Give unique IDs to each send report request.
+FeedbackService::RequestId g_last_send_feedback_request_Id = 0;
+
+void PopulateSystemInfo(SystemInformationList* sys_info_list,
     const std::string& key,
     const std::string& value) {
   base::DictionaryValue sys_info_value;
@@ -35,7 +40,13 @@ void PopulateSystemInfo(
 
 }  // namespace
 
-namespace extensions {
+// Represents a request to send a feedback report. It is used to be able to
+// handle multiple send requests.
+struct FeedbackService::SendRequest {
+  RequestId request_id;
+  scoped_refptr<feedback::FeedbackData> feedback_data;
+  SendFeedbackCallback on_sent_callback;
+};
 
 FeedbackService::FeedbackService() {
 }
@@ -47,75 +58,83 @@ void FeedbackService::SendFeedback(
     Profile* profile,
     scoped_refptr<FeedbackData> feedback_data,
     const SendFeedbackCallback& callback) {
-  send_feedback_callback_ = callback;
-  feedback_data_ = feedback_data;
-  feedback_data_->set_locale(g_browser_process->GetApplicationLocale());
-  feedback_data_->set_user_agent(GetUserAgent());
+  // Give every send request a unique Id.
+  const RequestId id = ++g_last_send_feedback_request_Id;
+  SendRequest& request = send_feedback_requests_[id];
 
-  if (!feedback_data_->attached_file_uuid().empty()) {
+  request.request_id = id;
+  request.on_sent_callback = callback;
+  request.feedback_data = feedback_data;
+  request.feedback_data->set_locale(g_browser_process->GetApplicationLocale());
+  request.feedback_data->set_user_agent(GetUserAgent());
+
+  if (!request.feedback_data->attached_file_uuid().empty()) {
     // Self-deleting object.
     BlobReader* attached_file_reader = new BlobReader(
-        profile, feedback_data_->attached_file_uuid(),
-        base::Bind(&FeedbackService::AttachedFileCallback, AsWeakPtr()));
+        profile, request.feedback_data->attached_file_uuid(),
+        base::Bind(&FeedbackService::AttachedFileCallback, AsWeakPtr(), id));
     attached_file_reader->Start();
   }
 
-  if (!feedback_data_->screenshot_uuid().empty()) {
+  if (!request.feedback_data->screenshot_uuid().empty()) {
     // Self-deleting object.
     BlobReader* screenshot_reader = new BlobReader(
-        profile, feedback_data_->screenshot_uuid(),
-        base::Bind(&FeedbackService::ScreenshotCallback, AsWeakPtr()));
+        profile, request.feedback_data->screenshot_uuid(),
+        base::Bind(&FeedbackService::ScreenshotCallback, AsWeakPtr(), id));
     screenshot_reader->Start();
   }
 
-  CompleteSendFeedback();
+  CompleteSendFeedback(id);
 }
 
-void FeedbackService::AttachedFileCallback(scoped_ptr<std::string> data,
+void FeedbackService::GetSystemInformation(
+    const GetSystemInformationCallback& callback) {
+  system_logs::ScrubbedSystemLogsFetcher* fetcher =
+      new system_logs::ScrubbedSystemLogsFetcher();
+  fetcher->Fetch(base::Bind(&FeedbackService::OnSystemLogsFetchComplete,
+                            AsWeakPtr(),
+                            callback));
+}
+
+void FeedbackService::AttachedFileCallback(RequestId request_id,
+                                           scoped_ptr<std::string> data,
                                            int64_t /* total_blob_length */) {
-  feedback_data_->set_attached_file_uuid(std::string());
+  DCHECK(ContainsKey(send_feedback_requests_, request_id));
+
+  SendRequest& request = send_feedback_requests_[request_id];
+  request.feedback_data->set_attached_file_uuid(std::string());
   if (data)
-    feedback_data_->AttachAndCompressFileData(std::move(data));
+    request.feedback_data->AttachAndCompressFileData(std::move(data));
 
-  CompleteSendFeedback();
+  CompleteSendFeedback(request_id);
 }
 
-void FeedbackService::ScreenshotCallback(scoped_ptr<std::string> data,
+void FeedbackService::ScreenshotCallback(RequestId request_id,
+                                         scoped_ptr<std::string> data,
                                          int64_t /* total_blob_length */) {
-  feedback_data_->set_screenshot_uuid(std::string());
-  if (data)
-    feedback_data_->set_image(std::move(data));
+  DCHECK(ContainsKey(send_feedback_requests_, request_id));
 
-  CompleteSendFeedback();
-}
-
-void FeedbackService::GetSystemInformation(
-    const GetSystemInformationCallback& callback) {
-  system_information_callback_ = callback;
+  SendRequest& request = send_feedback_requests_[request_id];
+  request.feedback_data->set_screenshot_uuid(std::string());
+  if (data)
+    request.feedback_data->set_image(std::move(data));
 
-  system_logs::ScrubbedSystemLogsFetcher* fetcher =
-      new system_logs::ScrubbedSystemLogsFetcher();
-  fetcher->Fetch(
-      base::Bind(&FeedbackService::OnSystemLogsFetchComplete, AsWeakPtr()));
+  CompleteSendFeedback(request_id);
 }
 
-
 void FeedbackService::OnSystemLogsFetchComplete(
+    const GetSystemInformationCallback& callback,
     scoped_ptr<system_logs::SystemLogsResponse> sys_info_map) {
   SystemInformationList sys_info_list;
-  if (!sys_info_map.get()) {
-    system_information_callback_.Run(sys_info_list);
-    return;
+  if (sys_info_map.get()) {
+    for (const auto& itr : *sys_info_map)
+      PopulateSystemInfo(&sys_info_list, itr.first, itr.second);
   }
 
-  for (system_logs::SystemLogsResponse::iterator it = sys_info_map->begin();
-       it != sys_info_map->end(); ++it)
-    PopulateSystemInfo(&sys_info_list, it->first, it->second);
-
-  system_information_callback_.Run(sys_info_list);
+  callback.Run(sys_info_list);
 }
 
-void FeedbackService::CompleteSendFeedback() {
+void FeedbackService::CompleteSendFeedback(RequestId request_id) {
   // A particular data collection is considered completed if,
   // a.) The blob URL is invalid - this will either happen because we never had
   //     a URL and never needed to read this data, or that the data read failed
@@ -123,16 +142,25 @@ void FeedbackService::CompleteSendFeedback() {
   // b.) The associated data object exists, meaning that the data has been read
   //     and the read callback has updated the associated data on the feedback
   //     object.
-  bool attached_file_completed = feedback_data_->attached_file_uuid().empty();
-  bool screenshot_completed = feedback_data_->screenshot_uuid().empty();
+  DCHECK(ContainsKey(send_feedback_requests_, request_id));
+
+  SendRequest& request = send_feedback_requests_[request_id];
+  const bool attached_file_completed =
+      request.feedback_data->attached_file_uuid().empty();
+  const bool screenshot_completed =
+      request.feedback_data->screenshot_uuid().empty();
 
   if (screenshot_completed && attached_file_completed) {
     // Signal the feedback object that the data from the feedback page has been
     // filled - the object will manage sending of the actual report.
-    feedback_data_->OnFeedbackPageDataComplete();
+    request.feedback_data->OnFeedbackPageDataComplete();
+
     // TODO(rkc): Change this once we have FeedbackData/Util refactored to
     // report the status of the report being sent.
-    send_feedback_callback_.Run(true);
+    request.on_sent_callback.Run(true);
+
+    // We're done with this request.
+    send_feedback_requests_.erase(request_id);
   }
 }