Cache feedback reports to disk in case of send failure.

chrome/browser/feedback/feedback_uploader_unittest.cc

Index: chrome/browser/feedback/feedback_uploader_unittest.cc
diff --git a/chrome/browser/feedback/feedback_uploader_unittest.cc b/chrome/browser/feedback/feedback_uploader_unittest.cc
index 407a286c0e3fc347f3398ad0d63f7011792e7bc3..61f4d3eec4d1bd7b42c99ed690aa88c557951fbd 100644
--- a/chrome/browser/feedback/feedback_uploader_unittest.cc
+++ b/chrome/browser/feedback/feedback_uploader_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/threading/platform_thread.h"
 #include "chrome/browser/feedback/feedback_uploader_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
@@ -54,15 +55,17 @@ class FeedbackUploaderTest : public testing::Test {
   }
 
   void QueueReport(const std::string& data) {
-    uploader_->QueueReport(make_scoped_ptr(new std::string(data)));
+    uploader_->QueueReport(data);
+    base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));

[Lei Zhang, 2014/02/01 01:13:45]

No, Please don't do this. See ToTT episode 303.

[Lei Zhang, 2014/02/01 01:37:22]

If you explain what you are waiting for, I might be able to suggest a better
solution.

[rkc, 2014/02/03 22:13:11]

This was to ensure that the timestamps of each report added were different, not
to actually 'wait' for anything. The reason for doing this was that even though
the case was handled correctly, it caused flakiness in the tests, since the
reports could now be processed out of order.

Fixed the test completion criteria to fix that flakiness (doesn't check by order
anymore), hence removed these sleeps.

Done.

[rkc, 2014/02/03 22:13:11]

This wasn't for 'timing' anything, it was to ensure we get different timestamps
per report. Explained in detail below.

[Lei Zhang, 2014/02/03 22:29:41]

Ok, if you really need to do it, please add a comment to explain why it's there.
Otherwise it raises a red flag.

[rkc, 2014/02/03 22:41:26]

Don't need to do it anymore since I changed the check. Removed it.

   }
 
   void ReportFailure(const std::string& data) {
-    uploader_->RetryReport(make_scoped_ptr(new std::string(data)));
+    uploader_->RetryReport(data);
+    base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));
   }
 
-  void MockDispatchReport(scoped_ptr<std::string> report_data) {
-    dispatched_reports_.push_back(*report_data.get());
+  void MockDispatchReport(const std::string& report_data) {
+    dispatched_reports_.push_back(report_data);
 
     // Dispatch will always update the timer, whether successful or not,
     // simulate the same behavior.
@@ -90,7 +93,13 @@ class FeedbackUploaderTest : public testing::Test {
   size_t expected_reports_;
 };
 
-TEST_F(FeedbackUploaderTest, QueueMultiple) {
+#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_MACOSX)
+#define MAYBE_QueueMultiple QueueMultiple
+#else
+// crbug.com/330547
+#define MAYBE_QueueMultiple DISABLED_QueueMultiple
+#endif
+TEST_F(FeedbackUploaderTest, MAYBE_QueueMultiple) {
   dispatched_reports_.clear();
   QueueReport(kReportOne);
   QueueReport(kReportTwo);
@@ -104,11 +113,11 @@ TEST_F(FeedbackUploaderTest, QueueMultiple) {
   EXPECT_EQ(dispatched_reports_[3], kReportFour);
 }
 
-#if defined(OS_WIN)
+#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_MACOSX)
+#define MAYBE_QueueMultipleWithFailures QueueMultipleWithFailures
+#else
 // crbug.com/330547
 #define MAYBE_QueueMultipleWithFailures DISABLED_QueueMultipleWithFailures
-#else
-#define MAYBE_QueueMultipleWithFailures QueueMultipleWithFailures
 #endif
 TEST_F(FeedbackUploaderTest, MAYBE_QueueMultipleWithFailures) {
   dispatched_reports_.clear();