Fix sending multiple feedback reports within short durations of each other

chrome/browser/extensions/api/feedback_private/feedback_service.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 "chrome/browser/extensions/api/feedback_private/feedback_service.h"
 
 #include <utility>
 
 #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"
 #include "chrome/common/chrome_content_client.h"
 #include "content/public/browser/browser_thread.h"
 
 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;
   sys_info_value.Set("key", new base::StringValue(key));
   sys_info_value.Set("value", new base::StringValue(value));
 
   linked_ptr<SystemInformation> sys_info(new SystemInformation());
   SystemInformation::Populate(sys_info_value, sys_info.get());
 
   sys_info_list->push_back(sys_info);
 }
 
 }  // 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() {
 }
 
 FeedbackService::~FeedbackService() {
 }
 
 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();
-}
-
-void FeedbackService::AttachedFileCallback(scoped_ptr<std::string> data,
-                                           int64_t /* total_blob_length */) {
-  feedback_data_->set_attached_file_uuid(std::string());
-  if (data)
-    feedback_data_->AttachAndCompressFileData(std::move(data));
-
-  CompleteSendFeedback();
-}
-
-void FeedbackService::ScreenshotCallback(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));
-
-  CompleteSendFeedback();
+  CompleteSendFeedback(id);
 }
 
 void FeedbackService::GetSystemInformation(
     const GetSystemInformationCallback& callback) {
-  system_information_callback_ = callback;
-
   system_logs::ScrubbedSystemLogsFetcher* fetcher =
       new system_logs::ScrubbedSystemLogsFetcher();
-  fetcher->Fetch(
-      base::Bind(&FeedbackService::OnSystemLogsFetchComplete, AsWeakPtr()));
+  fetcher->Fetch(base::Bind(&FeedbackService::OnSystemLogsFetchComplete,
+                            AsWeakPtr(),
+                            callback));
 }
 
+void FeedbackService::AttachedFileCallback(RequestId request_id,
+                                           scoped_ptr<std::string> data,
+                                           int64_t /* total_blob_length */) {
+  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)
+    request.feedback_data->AttachAndCompressFileData(std::move(data));
+
+  CompleteSendFeedback(request_id);
+}
+
+void FeedbackService::ScreenshotCallback(RequestId request_id,
+                                         scoped_ptr<std::string> data,
+                                         int64_t /* total_blob_length */) {
+  DCHECK(ContainsKey(send_feedback_requests_, request_id));
+
+  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));
+
+  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
   //     and we set it to invalid in the data read callback.
   // 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);
   }
 }
 
 }  // namespace extensions