Fix feedback attach a file and system info code.

Attaching a file to feedback is crashing the app since we fail on the AttachedFile validate (since we don't actually send blobs over, the attachedFile.data field is always empty, which fails parameter validation on the Chrome side). The easiest way to fix it was to make the parameter optional, since we never read it anyway. It is only there to hold the data so that the feedback custom bindings can move it into attachedFileBlobUrl.

This CL also fixes the sys info, which wasn't being sent due to code that was ported over from the UI incorrectly. The new UI does not need to wait on system information collection, since that is now done via a separate call, making passing around the sys_info variable completely redundant, and in this case wrong.

R=asargent@chromium.org
BUG=285942,285938
TEST=We are able to attach a file and system information with a feedback report.

Review URL: https://chromiumcodereview.appspot.com/23458031

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

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

@@ -173,7 +173,7 @@ bool FeedbackPrivateSendFeedbackFunction::RunImpl() {
          it != sys_info->end(); ++it)
       (*sys_logs.get())[it->get()->key] = it->get()->value;
   }
-  feedback_data->set_sys_info(sys_logs.Pass());
+  feedback_data->SetAndCompressSystemInfo(sys_logs.Pass());
 
   FeedbackService* service = FeedbackPrivateAPI::GetFactoryInstance()->
       GetForProfile(profile())->GetService();

chrome/browser/feedback/feedback_data.cc

@@ -79,57 +79,49 @@ FeedbackData::FeedbackData() : profile_(NULL),
 FeedbackData::~FeedbackData() {
 }
 
-bool FeedbackData::IsDataComplete() {
-  return (syslogs_compression_complete_ || !sys_info_.get()) &&
-      feedback_page_data_complete_;
-}
-void FeedbackData::SendReport() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (IsDataComplete() && !report_sent_) {
-    report_sent_ = true;
-    feedback_util::SendReport(this);
-  }
-}
-
 void FeedbackData::OnFeedbackPageDataComplete() {
   feedback_page_data_complete_ = true;
   SendReport();
 }
 
-void FeedbackData::set_sys_info(
-    scoped_ptr<FeedbackData::SystemLogsMap> sys_info) {
-  if (sys_info.get())
-    CompressSyslogs(sys_info.Pass());
-}
-
-void FeedbackData::CompressSyslogs(
+void FeedbackData::SetAndCompressSystemInfo(
     scoped_ptr<FeedbackData::SystemLogsMap> sys_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // We get the pointer first since base::Passed will nullify the scoper, hence
-  // it's not safe to use <scoper>.get() as a parameter to PostTaskAndReply.
-  FeedbackData::SystemLogsMap* sys_info_ptr = sys_info.get();
-  std::string* compressed_logs_ptr = new std::string;
-  scoped_ptr<std::string> compressed_logs(compressed_logs_ptr);
-  BrowserThread::PostBlockingPoolTaskAndReply(
-      FROM_HERE,
-      base::Bind(&ZipLogs,
-                 sys_info_ptr,
-                 compressed_logs_ptr),
-      base::Bind(&FeedbackData::OnCompressLogsComplete,
-                 this,
-                 base::Passed(&sys_info),
-                 base::Passed(&compressed_logs)));
+  sys_info_ = sys_info.Pass();
+  if (sys_info_.get()) {
+    std::string* compressed_logs_ptr = new std::string;
+    scoped_ptr<std::string> compressed_logs(compressed_logs_ptr);
+    BrowserThread::PostBlockingPoolTaskAndReply(
+        FROM_HERE,
+        base::Bind(&ZipLogs,
+                   sys_info_.get(),
+                   compressed_logs_ptr),
+        base::Bind(&FeedbackData::OnCompressLogsComplete,
+                   this,
+                   base::Passed(&compressed_logs)));
+  }
 }
 
 void FeedbackData::OnCompressLogsComplete(
-    scoped_ptr<FeedbackData::SystemLogsMap> sys_info,
     scoped_ptr<std::string> compressed_logs) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  sys_info_ = sys_info.Pass();
   compressed_logs_ = compressed_logs.Pass();
   syslogs_compression_complete_ = true;
 
   SendReport();
 }
+
+bool FeedbackData::IsDataComplete() {
+  return (syslogs_compression_complete_ || !sys_info_.get()) &&
+      feedback_page_data_complete_;
+}
+
+void FeedbackData::SendReport() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (IsDataComplete() && !report_sent_) {
+    report_sent_ = true;
+    feedback_util::SendReport(this);
+  }
+}

chrome/browser/feedback/feedback_data.h

@@ -27,15 +27,15 @@ class FeedbackData : public base::RefCountedThreadSafe<FeedbackData> {
 
   FeedbackData();
 
-  // Called once we've update all the data from the feedback page.
+  // Called once we've updated all the data from the feedback page.
   void OnFeedbackPageDataComplete();
 
-  // Called once we have read our system logs.
-  void CompressSyslogs(scoped_ptr<SystemLogsMap> sys_info);
+  // Sets the system information for this instance and kicks off its
+  // compression.
+  void SetAndCompressSystemInfo(scoped_ptr<SystemLogsMap> sys_info);
 
-  // Called once we have read and compressed our system logs.
-  void OnCompressLogsComplete(scoped_ptr<SystemLogsMap> sys_info,
-                              scoped_ptr<std::string> compressed_logs);
+  // Called once we have compressed our system logs.
+  void OnCompressLogsComplete(scoped_ptr<std::string> compressed_logs);
 
   // Returns true if we've completed all the tasks needed before we can send
   // feedback - at this time this is includes getting the feedback page data
@@ -45,10 +45,6 @@ class FeedbackData : public base::RefCountedThreadSafe<FeedbackData> {
   // Sends the feedback report if we have all our data complete.
   void SendReport();
 
-  // Starts reading the file to attach to this report into the string
-  // file_data. ReadFileComplete is called once this is done.
-  void StartReadFile(const std::string filename, const std::string* file_data);
-
   // Getters
   Profile* profile() const { return profile_; }
   const std::string& category_tag() const { return category_tag_; }
@@ -84,7 +80,6 @@ class FeedbackData : public base::RefCountedThreadSafe<FeedbackData> {
   }
   void set_attached_file_url(const GURL& url) { attached_file_url_ = url; }
   void set_screenshot_url(const GURL& url) { screenshot_url_ = url; }
-  void set_sys_info(scoped_ptr<SystemLogsMap> sys_info);
 
  private:
   friend class base::RefCountedThreadSafe<FeedbackData>;

chrome/common/extensions/api/feedback_private.idl

@@ -8,7 +8,7 @@ namespace feedbackPrivate {
 
   dictionary AttachedFile {
     DOMString name;
-    [instanceOf=Blob] object data;
+    [instanceOf=Blob] object? data;
   };
 
   dictionary SystemInformation {