Fix race condition in uploading feedback reports.

It is possible that if the browser starts and shuts down really quickly, we may end up with a race where the context passed to LoadReports has been nullified. We can completely avoid this race by simply passing in the user directory to LoadReportsAndQueue since the method doesn't really need the context for anything else.

R=zork@chromium.org
BUG=345805

Review URL: https://codereview.chromium.org/185003005

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

4 files changed, 9 insertions, 13 deletions

diff --git a/chrome/browser/feedback/feedback_profile_observer.cc b/chrome/browser/feedback/feedback_profile_observer.cc
index 0ec0b73..827faa9 100644
--- a/chrome/browser/feedback/feedback_profile_observer.cc
+++ b/chrome/browser/feedback/feedback_profile_observer.cc
@@ -43,7 +43,7 @@ void FeedbackProfileObserver::Observe(
   DCHECK_EQ(chrome::NOTIFICATION_PROFILE_CREATED, type);
 
   Profile* profile = content::Source<Profile>(source).ptr();
-  if (!profile->IsOffTheRecord())
+  if (profile && !profile->IsOffTheRecord())
     QueueUnsentReports(profile);
 }
 
@@ -52,9 +52,10 @@ void FeedbackProfileObserver::QueueUnsentReports(
   feedback::FeedbackUploader* uploader =
       feedback::FeedbackUploaderFactory::GetForBrowserContext(context);
   BrowserThread::PostBlockingPoolTask(FROM_HERE,
-      base::Bind(
-          &FeedbackReport::LoadReportsAndQueue, context, base::Bind(
-              &FeedbackUploader::QueueReport, uploader->AsWeakPtr())));
+      base::Bind(&FeedbackReport::LoadReportsAndQueue,
+                 context->GetPath(),
+                 base::Bind(&FeedbackUploader::QueueReport,
+                            uploader->AsWeakPtr())));
 }
 
 }  // namespace feedback
diff --git a/chrome/browser/feedback/feedback_report.cc b/chrome/browser/feedback/feedback_report.cc
index 89a17fe..52f4f8c 100644
--- a/chrome/browser/feedback/feedback_report.cc
+++ b/chrome/browser/feedback/feedback_report.cc
@@ -78,12 +78,11 @@ void FeedbackReport::DeleteReportOnDisk() {
 
 // static
 void FeedbackReport::LoadReportsAndQueue(
-    content::BrowserContext* context, QueueCallback callback) {
-  base::FilePath reports_path = GetFeedbackReportsPath(context);
-  if (reports_path.empty())
+    const base::FilePath& user_dir, QueueCallback callback) {
+  if (user_dir.empty())
     return;
 
-  base::FileEnumerator enumerator(reports_path,
+  base::FileEnumerator enumerator(user_dir.Append(kFeedbackReportPath),
                                   false,
                                   base::FileEnumerator::FILES,
                                   kFeedbackReportFilenameWildcard);
diff --git a/chrome/browser/feedback/feedback_report.h b/chrome/browser/feedback/feedback_report.h
index 2926451..2584903 100644
--- a/chrome/browser/feedback/feedback_report.h
+++ b/chrome/browser/feedback/feedback_report.h
@@ -43,7 +43,7 @@ class FeedbackReport : public base::RefCounted<FeedbackReport> {
 
   // Loads the reports still on disk and queues then using the given callback.
   // This call blocks on the file reads.
-  static void LoadReportsAndQueue(content::BrowserContext* context,
+  static void LoadReportsAndQueue(const base::FilePath& user_dir,
                                   QueueCallback callback);
 
  private:
diff --git a/chrome/browser/feedback/feedback_uploader.h b/chrome/browser/feedback/feedback_uploader.h
index 9ddf3a0..aa91613 100644
--- a/chrome/browser/feedback/feedback_uploader.h
+++ b/chrome/browser/feedback/feedback_uploader.h
@@ -45,10 +45,6 @@ class FeedbackUploader : public BrowserContextKeyedService,
   // Dispatches the report to be uploaded.
   void DispatchReport(const std::string& data);
 
-  // Loads any unsent reports from disk and queues them to be uploaded in
-  // the given browser context.
-  void QueueUnsentReports(content::BrowserContext* context);
-
   // Update our timer for uploading the next report.
   void UpdateUploadTimer();