Revert 64321 (broke mac compile; please use tryservers) - Changes to enable retry of failed sends on feedback reports.

If the initial send fails, the first retry will be in 15 minutes after the
attempt. The next attempt will be in 30 minutes and the time will double till
we hit a 4 hour delay - the send will be then retried every 4 hours till the
Chrome instance is up.

BUG=cros:6331
TEST=Tested with up to 25 reports at a time, with varying network connectivity
(via pulling out the network cable); report data verified on the feedback
server.

Review URL: http://codereview.chromium.org/4027005

TBR=rkc@chromium.org
Review URL: http://codereview.chromium.org/4096007

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

1 files changed, 103 insertions, 96 deletions

diff --git a/chrome/browser/bug_report_util.cc b/chrome/browser/bug_report_util.cc
index a726c74..db771cc 100644
--- a/chrome/browser/bug_report_util.cc
+++ b/chrome/browser/bug_report_util.cc
@@ -72,21 +72,42 @@ const size_t kMaxLineCount       = 10;
 const size_t kMaxSystemLogLength = 1024;
 #endif
 
-const int64 kInitialRetryDelay = 900000; // 15 minutes
-const int64 kRetryDelayIncreaseFactor = 2;
-const int64 kRetryDelayLimit = 14400000; // 4 hours
+}  // namespace
 
 
-}  // namespace
+#if defined(OS_CHROMEOS)
+class FeedbackNotification {
+ public:
+  // Note: notification will show only on one profile at a time.
+  void Show(Profile* profile, const string16& message, bool urgent) {
+    if (notification_.get()) {
+        notification_->Hide();
+    }
+    notification_.reset(
+        new chromeos::SystemNotification(profile, kNotificationId,
+                               IDR_STATUSBAR_FEEDBACK,
+                               l10n_util::GetStringUTF16(
+                                   IDS_BUGREPORT_NOTIFICATION_TITLE)));
+    notification_->Show(message, urgent, false);
+  }
 
+ private:
+  FeedbackNotification() {}
+  friend struct DefaultSingletonTraits<FeedbackNotification>;
+
+  scoped_ptr<chromeos::SystemNotification> notification_;
+  DISALLOW_COPY_AND_ASSIGN(FeedbackNotification);
+};
+#endif
 
 // Simple URLFetcher::Delegate to clean up URLFetcher on completion.
 class BugReportUtil::PostCleanup : public URLFetcher::Delegate {
  public:
-  PostCleanup(Profile* profile, std::string* post_body,
-              int64 previous_delay) : profile_(profile),
-                                      post_body_(post_body),
-                                      previous_delay_(previous_delay) { }
+#if defined(OS_CHROMEOS)
+  explicit PostCleanup(Profile* profile);
+#else
+  PostCleanup();
+#endif
   // Overridden from URLFetcher::Delegate.
   virtual void OnURLFetchComplete(const URLFetcher* source,
                                   const GURL& url,
@@ -100,15 +121,18 @@ class BugReportUtil::PostCleanup : public URLFetcher::Delegate {
 
  private:
   Profile* profile_;
-  std::string* post_body_;
-  int64 previous_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(PostCleanup);
 };
 
-// Don't use the data parameter, instead use the pointer we pass into every
-// post cleanup object - that pointer will be deleted and deleted only on a
-// successful post to the feedback server.
+#if defined(OS_CHROMEOS)
+  BugReportUtil::PostCleanup::PostCleanup(Profile* profile)
+      : profile_(profile) {
+#else
+  BugReportUtil::PostCleanup::PostCleanup() : profile_(NULL) {
+#endif
+}
+
 void BugReportUtil::PostCleanup::OnURLFetchComplete(
     const URLFetcher* source,
     const GURL& url,
@@ -119,35 +143,34 @@ void BugReportUtil::PostCleanup::OnURLFetchComplete(
 
   std::stringstream error_stream;
   if (response_code == kHttpPostSuccessNoContent) {
-    // We've sent our report, delete the report data
-    delete post_body_;
-
     error_stream << "Success";
-  } else {
-    // Uh oh, feedback failed, send it off to retry
-    if (previous_delay_) {
-      if (previous_delay_ < kRetryDelayLimit)
-        previous_delay_ *= kRetryDelayIncreaseFactor;
-    } else {
-      previous_delay_ = kInitialRetryDelay;
-    }
-    BugReportUtil::DispatchFeedback(profile_, post_body_, previous_delay_);
-
-    // Process the error for debug output
-    if (response_code == kHttpPostFailNoConnection) {
-        error_stream << "No connection to server.";
-    } else if ((response_code > kHttpPostFailClientError) &&
+  } else if (response_code == kHttpPostFailNoConnection) {
+    error_stream << "No connection to server.";
+  } else if ((response_code > kHttpPostFailClientError) &&
       (response_code < kHttpPostFailServerError)) {
-        error_stream << "Client error: HTTP response code " << response_code;
-    } else if (response_code > kHttpPostFailServerError) {
-        error_stream << "Server error: HTTP response code " << response_code;
-    } else {
-        error_stream << "Unknown error: HTTP response code " << response_code;
-    }
+    error_stream << "Client error: HTTP response code " << response_code;
+  } else if (response_code > kHttpPostFailServerError) {
+    error_stream << "Server error: HTTP response code " << response_code;
+  } else {
+    error_stream << "Unknown error: HTTP response code " << response_code;
   }
 
-  LOG(WARNING) << "FEEDBACK: Submission to feedback server (" << url <<
-      ") status: " << error_stream.str() << std::endl;
+  LOG(WARNING) << "Submission to feedback server (" << url
+               << ") status: " << error_stream.str();
+
+#if defined(OS_CHROMEOS)
+  // Show the notification to the user; this notification will stay active till
+  // either the user closes it, or we display another notification.
+  if (response_code == kHttpPostSuccessNoContent) {
+    Singleton<FeedbackNotification>()->Show(profile_, l10n_util::GetStringUTF16(
+        IDS_BUGREPORT_FEEDBACK_STATUS_SUCCESS), false);
+  } else {
+    Singleton<FeedbackNotification>()->Show(profile_,
+        l10n_util::GetStringFUTF16(IDS_BUGREPORT_FEEDBACK_STATUS_FAIL,
+                                   ASCIIToUTF16(error_stream.str())),
+        true);
+  }
+#endif
 
   // Delete the URLFetcher.
   delete source;
@@ -190,47 +213,13 @@ void BugReportUtil::SetFeedbackServer(const std::string& server) {
   feedback_server_ = server;
 }
 
-// static
-void BugReportUtil::DispatchFeedback(Profile* profile,
-                                     std::string* post_body,
-                                     int64 delay) {
-  DCHECK(post_body);
-
-  MessageLoop::current()->PostDelayedTask(FROM_HERE, NewRunnableFunction(
-      &BugReportUtil::SendFeedback, profile, post_body, delay), delay);
-}
-
-// static
-void BugReportUtil::SendFeedback(Profile* profile,
-                                 std::string* post_body,
-                                 int64 previous_delay) {
-  DCHECK(post_body);
-
-  GURL post_url;
-  if (CommandLine::ForCurrentProcess()->
-      HasSwitch(switches::kFeedbackServer))
-    post_url = GURL(CommandLine::ForCurrentProcess()->
-        GetSwitchValueASCII(switches::kFeedbackServer));
-  else
-    post_url = GURL(kBugReportPostUrl);
-
-  URLFetcher* fetcher = new URLFetcher(post_url, URLFetcher::POST,
-                            new BugReportUtil::PostCleanup(profile,
-                                                           post_body,
-                                                           previous_delay));
-  fetcher->set_request_context(profile->GetRequestContext());
-
-  fetcher->set_upload_data(std::string(kProtBufMimeType), *post_body);
-  fetcher->Start();
-}
-
 
 // static
 void BugReportUtil::AddFeedbackData(
     userfeedback::ExternalExtensionSubmit* feedback_data,
     const std::string& key, const std::string& value) {
-  // Don't bother with empty keys or values
-  if (key=="" || value == "") return;
+  // We have no reason to log any empty values - gives us no data
+  if (value == "") return;
   // Create log_value object and add it to the web_data object
   userfeedback::ProductSpecificData log_value;
   log_value.set_key(key);
@@ -274,6 +263,15 @@ void BugReportUtil::SendReport(Profile* profile,
 #else
     int png_height) {
 #endif
+  GURL post_url;
+
+  if (CommandLine::ForCurrentProcess()->
+      HasSwitch(switches::kFeedbackServer))
+    post_url = GURL(CommandLine::ForCurrentProcess()->
+        GetSwitchValueASCII(switches::kFeedbackServer));
+  else
+    post_url = GURL(kBugReportPostUrl);
+
   // Create google feedback protocol buffer objects
   userfeedback::ExternalExtensionSubmit feedback_data;
   // type id set to 0, unused field but needs to be initialized to 0
@@ -322,6 +320,17 @@ void BugReportUtil::SendReport(Profile* profile,
   SetOSVersion(&os_version);
   AddFeedbackData(&feedback_data, std::string(kOsVersionTag), os_version);
 
+#if defined(OS_CHROMEOS)
+  if (sys_info) {
+    for (chromeos::LogDictionaryType::const_iterator i = sys_info->begin();
+         i != sys_info->end(); ++i)
+      if (!CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kCompressSystemFeedback) || ValidFeedbackSize(i->second)) {
+        AddFeedbackData(&feedback_data, i->first, i->second);
+      }
+  }
+#endif
+
   // Include the page image if we have one.
   if (png_data) {
     userfeedback::PostedScreenshot screenshot;
@@ -338,24 +347,15 @@ void BugReportUtil::SendReport(Profile* profile,
   }
 
 #if defined(OS_CHROMEOS)
-  if (sys_info) {
-    // Add the product specific data
-    for (chromeos::LogDictionaryType::const_iterator i = sys_info->begin();
-         i != sys_info->end(); ++i)
-      if (!CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kCompressSystemFeedback) || ValidFeedbackSize(i->second)) {
-        AddFeedbackData(&feedback_data, i->first, i->second);
-      }
-
-    // If we have zipped logs, add them here
-    if (zipped_logs_data && CommandLine::ForCurrentProcess()->HasSwitch(
+  // Include the page image if we have one.
+  if (zipped_logs_data && CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kCompressSystemFeedback)) {
-      userfeedback::ProductSpecificBinaryData attachment;
-      attachment.set_mime_type(kBZip2MimeType);
-      attachment.set_name(kLogsAttachmentName);
-      attachment.set_data(std::string(zipped_logs_data, zipped_logs_length));
-      *(feedback_data.add_product_specific_binary_data()) = attachment;
-    }
+    userfeedback::ProductSpecificBinaryData attachment;
+    attachment.set_mime_type(kBZip2MimeType);
+    attachment.set_name(kLogsAttachmentName);
+    attachment.set_data(std::string(zipped_logs_data, zipped_logs_length));
+
+    *(feedback_data.add_product_specific_binary_data()) = attachment;
   }
 #endif
 
@@ -379,12 +379,19 @@ void BugReportUtil::SendReport(Profile* profile,
 
   *(feedback_data.mutable_chrome_data()) = chrome_data;
 
-  // Serialize our report to a string pointer we can pass around
-  std::string* post_body = new std::string;
-  feedback_data.SerializeToString(post_body);
+  // We have the body of our POST, so send it off to the server.
+  URLFetcher* fetcher = new URLFetcher(post_url, URLFetcher::POST,
+#if defined(OS_CHROMEOS)
+                                       new BugReportUtil::PostCleanup(profile));
+#else
+                                       new BugReportUtil::PostCleanup());
+#endif
+  fetcher->set_request_context(profile->GetRequestContext());
 
-  // We have the body of our POST, so send it off to the server with 0 delay
-  DispatchFeedback(profile, post_body, 0);
+  std::string post_body;
+  feedback_data.SerializeToString(&post_body);
+  fetcher->set_upload_data(std::string(kProtBufMimeType), post_body);
+  fetcher->Start();
 }
 
 // static