Refactor FeedbackReport, FeedbackUpload to remove chrome/net deps

Remove use of BrowserContext and BrowserThread from FeedbackReport and
FeedbackUpload. Moves net/ specific functionality into a new class
FeedbackUploadChrome, and moves using the browser context out of
the base classes.

CrOS should be able to directly use feedback_report.cc/h &
feedback_uploader.cc/h, providing a curl-based upload version.

BUG=341554

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

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

11 files changed, 172 insertions, 97 deletions

diff --git a/chrome/browser/feedback/feedback_profile_observer.cc b/chrome/browser/feedback/feedback_profile_observer.cc
index 827faa9..a08b653 100644
--- a/chrome/browser/feedback/feedback_profile_observer.cc
+++ b/chrome/browser/feedback/feedback_profile_observer.cc
@@ -47,15 +47,23 @@ void FeedbackProfileObserver::Observe(
     QueueUnsentReports(profile);
 }
 
+void FeedbackProfileObserver::QueueSingleReport(
+    feedback::FeedbackUploader* uploader,
+    const std::string& data) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE, base::Bind(&FeedbackUploader::QueueReport,
+                                               uploader->AsWeakPtr(), data));
+}
+
 void FeedbackProfileObserver::QueueUnsentReports(
     content::BrowserContext* context) {
   feedback::FeedbackUploader* uploader =
       feedback::FeedbackUploaderFactory::GetForBrowserContext(context);
   BrowserThread::PostBlockingPoolTask(FROM_HERE,
-      base::Bind(&FeedbackReport::LoadReportsAndQueue,
-                 context->GetPath(),
-                 base::Bind(&FeedbackUploader::QueueReport,
-                            uploader->AsWeakPtr())));
+      base::Bind(
+          &FeedbackReport::LoadReportsAndQueue,
+          uploader->GetFeedbackReportsPath(),
+          base::Bind(&FeedbackProfileObserver::QueueSingleReport, uploader)));
 }
 
 }  // namespace feedback
diff --git a/chrome/browser/feedback/feedback_profile_observer.h b/chrome/browser/feedback/feedback_profile_observer.h
index 4a48c99..bde27ff 100644
--- a/chrome/browser/feedback/feedback_profile_observer.h
+++ b/chrome/browser/feedback/feedback_profile_observer.h
@@ -16,6 +16,8 @@ class BrowserContext;
 
 namespace feedback {
 
+class FeedbackUploader;
+
 // FeedbackProfileObserver waits on profile creation notifications to check
 // if the profile has any pending feedback reports to upload. If it does, it
 // queues those reports for upload.
@@ -38,6 +40,9 @@ class FeedbackProfileObserver : public content::NotificationObserver {
   // the given browser context.
   void QueueUnsentReports(content::BrowserContext* context);
 
+  static void QueueSingleReport(feedback::FeedbackUploader* uploader,
+                                const std::string& data);
+
   // Used to track creation of profiles so we can load any unsent reports
   // for that profile.
   content::NotificationRegistrar prefs_registrar_;
diff --git a/chrome/browser/feedback/feedback_report.cc b/chrome/browser/feedback/feedback_report.cc
index 52f4f8c..8518d2b 100644
--- a/chrome/browser/feedback/feedback_report.cc
+++ b/chrome/browser/feedback/feedback_report.cc
@@ -10,27 +10,15 @@
 #include "base/guid.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/sequenced_worker_pool.h"
-#include "content/public/browser/browser_context.h"
-#include "content/public/browser/browser_thread.h"
 #include "net/base/directory_lister.h"
 
-using content::BrowserThread;
-
 namespace {
 
-const base::FilePath::CharType kFeedbackReportPath[] =
-    FILE_PATH_LITERAL("Feedback Reports");
 const base::FilePath::CharType kFeedbackReportFilenameWildcard[] =
     FILE_PATH_LITERAL("Feedback Report.*");
 
 const char kFeedbackReportFilenamePrefix[] = "Feedback Report.";
 
-base::FilePath GetFeedbackReportsPath(content::BrowserContext* context) {
-  if (!context)
-    return base::FilePath();
-  return context->GetPath().Append(kFeedbackReportPath);
-}
-
 void WriteReportOnBlockingPool(const base::FilePath reports_path,
                                const base::FilePath& file,
                                const std::string& data) {
@@ -47,24 +35,20 @@ void WriteReportOnBlockingPool(const base::FilePath reports_path,
 
 namespace feedback {
 
-FeedbackReport::FeedbackReport(content::BrowserContext* context,
-                               const base::Time& upload_at,
-                               const std::string& data)
-    : upload_at_(upload_at),
-      data_(data) {
-  reports_path_ = GetFeedbackReportsPath(context);
+FeedbackReport::FeedbackReport(
+    const base::FilePath& path,
+    const base::Time& upload_at,
+    const std::string& data,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    : reports_path_(path),
+      upload_at_(upload_at),
+      data_(data),
+      reports_task_runner_(task_runner) {
   if (reports_path_.empty())
     return;
   file_ = reports_path_.AppendASCII(
       kFeedbackReportFilenamePrefix + base::GenerateGUID());
 
-  // Uses a BLOCK_SHUTDOWN file task runner because we really don't want to
-  // lose reports.
-  base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
-  reports_task_runner_ = pool->GetSequencedTaskRunnerWithShutdownBehavior(
-      pool->GetSequenceToken(),
-      base::SequencedWorkerPool::BLOCK_SHUTDOWN);
-
   reports_task_runner_->PostTask(FROM_HERE, base::Bind(
       &WriteReportOnBlockingPool, reports_path_, file_, data_));
 }
@@ -82,7 +66,7 @@ void FeedbackReport::LoadReportsAndQueue(
   if (user_dir.empty())
     return;
 
-  base::FileEnumerator enumerator(user_dir.Append(kFeedbackReportPath),
+  base::FileEnumerator enumerator(user_dir,
                                   false,
                                   base::FileEnumerator::FILES,
                                   kFeedbackReportFilenameWildcard);
@@ -91,8 +75,7 @@ void FeedbackReport::LoadReportsAndQueue(
        name = enumerator.Next()) {
     std::string data;
     if (ReadFileToString(name, &data))
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE, base::Bind(callback, data));
+      callback.Run(data);
     base::DeleteFile(name, false);
   }
 }
diff --git a/chrome/browser/feedback/feedback_report.h b/chrome/browser/feedback/feedback_report.h
index 2584903..92914d1 100644
--- a/chrome/browser/feedback/feedback_report.h
+++ b/chrome/browser/feedback/feedback_report.h
@@ -17,10 +17,6 @@ namespace base {
 class SequencedTaskRunner;
 }
 
-namespace content {
-class BrowserContext;
-}
-
 namespace feedback {
 
 typedef base::Callback<void(const std::string&)> QueueCallback;
@@ -30,9 +26,10 @@ typedef base::Callback<void(const std::string&)> QueueCallback;
 // deleted by calling DeleteReportOnDisk.
 class FeedbackReport : public base::RefCounted<FeedbackReport> {
  public:
-  FeedbackReport(content::BrowserContext* context,
+  FeedbackReport(const base::FilePath& path,
                  const base::Time& upload_at,
-                 const std::string& data);
+                 const std::string& data,
+                 scoped_refptr<base::SequencedTaskRunner> task_runner);
 
   // Stops the disk write of the report and deletes the report file if already
   // written.
diff --git a/chrome/browser/feedback/feedback_uploader.cc b/chrome/browser/feedback/feedback_uploader.cc
index 75b3a6a..1798e3bf 100644
--- a/chrome/browser/feedback/feedback_uploader.cc
+++ b/chrome/browser/feedback/feedback_uploader.cc
@@ -7,27 +7,22 @@
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/sequenced_task_runner.h"
 #include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/feedback/feedback_report.h"
-#include "chrome/common/chrome_switches.h"
-#include "content/public/browser/browser_context.h"
-#include "content/public/browser/browser_thread.h"
-#include "net/base/load_flags.h"
-#include "net/url_request/url_fetcher.h"
-#include "url/gurl.h"
-
-using content::BrowserThread;
 
 namespace feedback {
 namespace {
 
 const char kFeedbackPostUrl[] =
     "https://www.google.com/tools/feedback/chrome/__submit";
-const char kProtBufMimeType[] = "application/x-protobuf";
 
 const int64 kRetryDelayMinutes = 60;
 
+const base::FilePath::CharType kFeedbackReportPath[] =
+    FILE_PATH_LITERAL("Feedback Reports");
+
 }  // namespace
 
 bool FeedbackUploader::ReportsUploadTimeComparator::operator()(
@@ -35,10 +30,12 @@ bool FeedbackUploader::ReportsUploadTimeComparator::operator()(
   return a->upload_at() > b->upload_at();
 }
 
-FeedbackUploader::FeedbackUploader(content::BrowserContext* context)
-    : context_(context),
-      retry_delay_(base::TimeDelta::FromMinutes(kRetryDelayMinutes)) {
-  CHECK(context_);
+FeedbackUploader::FeedbackUploader(const base::FilePath& path,
+                                   base::SequencedWorkerPool* pool)
+    : report_path_(path.Append(kFeedbackReportPath)),
+      retry_delay_(base::TimeDelta::FromMinutes(kRetryDelayMinutes)),
+      url_(kFeedbackPostUrl),
+      pool_(pool) {
   dispatch_callback_ = base::Bind(&FeedbackUploader::DispatchReport,
                                   AsWeakPtr());
 }
@@ -46,31 +43,7 @@ FeedbackUploader::FeedbackUploader(content::BrowserContext* context)
 FeedbackUploader::~FeedbackUploader() {}
 
 void FeedbackUploader::QueueReport(const std::string& data) {
-  reports_queue_.push(
-      new FeedbackReport(context_, base::Time::Now(), data));
-  UpdateUploadTimer();
-}
-
-void FeedbackUploader::DispatchReport(const std::string& data) {
-  GURL post_url;
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kFeedbackServer))
-    post_url = GURL(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-        switches::kFeedbackServer));
-  else
-    post_url = GURL(kFeedbackPostUrl);
-
-  net::URLFetcher* fetcher = net::URLFetcher::Create(
-      post_url, net::URLFetcher::POST,
-      new FeedbackUploaderDelegate(
-          data,
-          base::Bind(&FeedbackUploader::UpdateUploadTimer, AsWeakPtr()),
-          base::Bind(&FeedbackUploader::RetryReport, AsWeakPtr())));
-
-  fetcher->SetUploadData(std::string(kProtBufMimeType), data);
-  fetcher->SetRequestContext(context_->GetRequestContext());
-  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
-                        net::LOAD_DO_NOT_SEND_COOKIES);
-  fetcher->Start();
+  QueueReportWithDelay(data, base::TimeDelta());
 }
 
 void FeedbackUploader::UpdateUploadTimer() {
@@ -94,9 +67,22 @@ void FeedbackUploader::UpdateUploadTimer() {
 }
 
 void FeedbackUploader::RetryReport(const std::string& data) {
-  reports_queue_.push(new FeedbackReport(context_,
-                                         base::Time::Now() + retry_delay_,
-                                         data));
+  QueueReportWithDelay(data, retry_delay_);
+}
+
+void FeedbackUploader::QueueReportWithDelay(const std::string& data,
+                                            base::TimeDelta delay) {
+  // Uses a BLOCK_SHUTDOWN file task runner because we really don't want to
+  // lose reports.
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      pool_->GetSequencedTaskRunnerWithShutdownBehavior(
+          pool_->GetSequenceToken(),
+          base::SequencedWorkerPool::BLOCK_SHUTDOWN);
+
+  reports_queue_.push(new FeedbackReport(report_path_,
+                                         base::Time::Now() + delay,
+                                         data,
+                                         task_runner));
   UpdateUploadTimer();
 }
 
diff --git a/chrome/browser/feedback/feedback_uploader.h b/chrome/browser/feedback/feedback_uploader.h
index 415b16d..2bef774 100644
--- a/chrome/browser/feedback/feedback_uploader.h
+++ b/chrome/browser/feedback/feedback_uploader.h
@@ -9,41 +9,40 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
-#include "chrome/browser/feedback/feedback_uploader_delegate.h"
-#include "components/keyed_service/core/keyed_service.h"
-
-namespace content {
-class BrowserContext;
-}
 
 namespace feedback {
 
+typedef base::Callback<void(const std::string&)> ReportDataCallback;
+
 class FeedbackReport;
 
 // FeedbackUploader is used to add a feedback report to the queue of reports
 // being uploaded. In case uploading a report fails, it is written to disk and
 // tried again when it's turn comes up next in the queue.
-class FeedbackUploader : public KeyedService,
-                         public base::SupportsWeakPtr<FeedbackUploader> {
+class FeedbackUploader : public base::SupportsWeakPtr<FeedbackUploader> {
  public:
-  explicit FeedbackUploader(content::BrowserContext* context);
+  explicit FeedbackUploader(const base::FilePath& path,
+                            base::SequencedWorkerPool* pool);
   virtual ~FeedbackUploader();
 
   // Queues a report for uploading.
   void QueueReport(const std::string& data);
 
- private:
+  base::FilePath GetFeedbackReportsPath() { return report_path_; }
+
+ protected:
   friend class FeedbackUploaderTest;
   struct ReportsUploadTimeComparator {
     bool operator()(FeedbackReport* a, FeedbackReport* b) const;
   };
 
   // Dispatches the report to be uploaded.
-  void DispatchReport(const std::string& data);
+  virtual void DispatchReport(const std::string& data) = 0;
 
   // Update our timer for uploading the next report.
   void UpdateUploadTimer();
@@ -51,11 +50,12 @@ class FeedbackUploader : public KeyedService,
   // Requeue this report with a delay.
   void RetryReport(const std::string& data);
 
+  void QueueReportWithDelay(const std::string& data, base::TimeDelta delay);
+
   void setup_for_test(const ReportDataCallback& dispatch_callback,
                       const base::TimeDelta& retry_delay);
 
-  // Browser context this uploader was created for.
-  content::BrowserContext* context_;
+  base::FilePath report_path_;
   // Timer to upload the next report at.
   base::OneShotTimer<FeedbackUploader> upload_timer_;
   // Priority queue of reports prioritized by the time the report is supposed
@@ -66,6 +66,8 @@ class FeedbackUploader : public KeyedService,
 
   ReportDataCallback dispatch_callback_;
   base::TimeDelta retry_delay_;
+  std::string url_;
+  base::SequencedWorkerPool* pool_;
 
   DISALLOW_COPY_AND_ASSIGN(FeedbackUploader);
 };
diff --git a/chrome/browser/feedback/feedback_uploader_chrome.cc b/chrome/browser/feedback/feedback_uploader_chrome.cc
new file mode 100644
index 0000000..7a0be8f
--- /dev/null
+++ b/chrome/browser/feedback/feedback_uploader_chrome.cc
@@ -0,0 +1,58 @@
+// Copyright 2014 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/feedback/feedback_uploader_chrome.h"
+
+#include "base/callback.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/task_runner_util.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "chrome/browser/feedback/feedback_report.h"
+#include "chrome/browser/feedback/feedback_uploader_delegate.h"
+#include "chrome/common/chrome_switches.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/base/load_flags.h"
+#include "net/url_request/url_fetcher.h"
+#include "url/gurl.h"
+
+using content::BrowserThread;
+
+namespace feedback {
+namespace {
+
+const char kProtoBufMimeType[] = "application/x-protobuf";
+
+}  // namespace
+
+FeedbackUploaderChrome::FeedbackUploaderChrome(
+    content::BrowserContext* context)
+    : FeedbackUploader(context ? context->GetPath() : base::FilePath(),
+                       BrowserThread::GetBlockingPool()),
+      context_(context) {
+  CHECK(context_);
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kFeedbackServer))
+    url_ = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+        switches::kFeedbackServer);
+}
+
+void FeedbackUploaderChrome::DispatchReport(const std::string& data) {
+  GURL post_url(url_);
+
+  net::URLFetcher* fetcher = net::URLFetcher::Create(
+      post_url, net::URLFetcher::POST,
+      new FeedbackUploaderDelegate(
+          data,
+          base::Bind(&FeedbackUploaderChrome::UpdateUploadTimer, AsWeakPtr()),
+          base::Bind(&FeedbackUploaderChrome::RetryReport, AsWeakPtr())));
+
+  fetcher->SetUploadData(std::string(kProtoBufMimeType), data);
+  fetcher->SetRequestContext(context_->GetRequestContext());
+  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
+                        net::LOAD_DO_NOT_SEND_COOKIES);
+  fetcher->Start();
+}
+
+}  // namespace feedback
diff --git a/chrome/browser/feedback/feedback_uploader_chrome.h b/chrome/browser/feedback/feedback_uploader_chrome.h
new file mode 100644
index 0000000..dc4ac59
--- /dev/null
+++ b/chrome/browser/feedback/feedback_uploader_chrome.h
@@ -0,0 +1,34 @@
+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_FEEDBACK_FEEDBACK_UPLOADER_CHROME_H_
+#define CHROME_BROWSER_FEEDBACK_FEEDBACK_UPLOADER_CHROME_H_
+
+#include "chrome/browser/feedback/feedback_uploader.h"
+
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace content {
+class BrowserContext;
+}
+
+namespace feedback {
+
+class FeedbackUploaderChrome : public FeedbackUploader,
+                               public KeyedService {
+ public:
+  explicit FeedbackUploaderChrome(content::BrowserContext* context);
+
+  virtual void DispatchReport(const std::string& data) OVERRIDE;
+
+ private:
+  // Browser context this uploader was created for.
+  content::BrowserContext* context_;
+
+  DISALLOW_COPY_AND_ASSIGN(FeedbackUploaderChrome);
+};
+
+}  // namespace feedback
+
+#endif  // CHROME_BROWSER_FEEDBACK_FEEDBACK_UPLOADER_CHROME_H_
diff --git a/chrome/browser/feedback/feedback_uploader_delegate.h b/chrome/browser/feedback/feedback_uploader_delegate.h
index b500104..67d23a7 100644
--- a/chrome/browser/feedback/feedback_uploader_delegate.h
+++ b/chrome/browser/feedback/feedback_uploader_delegate.h
@@ -9,12 +9,11 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "chrome/browser/feedback/feedback_uploader.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
 namespace feedback {
 
-typedef base::Callback<void(const std::string&)> ReportDataCallback;
-
 // FeedbackUploaderDelegate is a simple http uploader for a feedback report. On
 // succes or failure, it deletes itself, but on failure it also notifies the
 // error callback specified when constructing the class instance.
diff --git a/chrome/browser/feedback/feedback_uploader_factory.cc b/chrome/browser/feedback/feedback_uploader_factory.cc
index 4a715ff..bc20ecc 100644
--- a/chrome/browser/feedback/feedback_uploader_factory.cc
+++ b/chrome/browser/feedback/feedback_uploader_factory.cc
@@ -6,6 +6,7 @@
 
 #include "base/memory/singleton.h"
 #include "chrome/browser/feedback/feedback_uploader.h"
+#include "chrome/browser/feedback/feedback_uploader_chrome.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 
@@ -19,7 +20,7 @@ FeedbackUploaderFactory* FeedbackUploaderFactory::GetInstance() {
 // static
 FeedbackUploader* FeedbackUploaderFactory::GetForBrowserContext(
     content::BrowserContext* context) {
-  return static_cast<FeedbackUploader*>(
+  return static_cast<FeedbackUploaderChrome*>(
       GetInstance()->GetServiceForBrowserContext(context, true));
 }
 
@@ -32,7 +33,7 @@ FeedbackUploaderFactory::~FeedbackUploaderFactory() {}
 
 KeyedService* FeedbackUploaderFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  return new FeedbackUploader(context);
+  return new FeedbackUploaderChrome(context);
 }
 
 content::BrowserContext* FeedbackUploaderFactory::GetBrowserContextToUse(
diff --git a/chrome/browser/feedback/feedback_uploader_unittest.cc b/chrome/browser/feedback/feedback_uploader_unittest.cc
index 2b07f91..c795c18 100644
--- a/chrome/browser/feedback/feedback_uploader_unittest.cc
+++ b/chrome/browser/feedback/feedback_uploader_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "chrome/browser/feedback/feedback_uploader_chrome.h"
 #include "chrome/browser/feedback/feedback_uploader_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
@@ -26,7 +27,8 @@ const base::TimeDelta kRetryDelayForTest =
     base::TimeDelta::FromMilliseconds(100);
 
 KeyedService* CreateFeedbackUploaderService(content::BrowserContext* context) {
-  return new feedback::FeedbackUploader(Profile::FromBrowserContext(context));
+  return new feedback::FeedbackUploaderChrome(
+      Profile::FromBrowserContext(context));
 }
 
 }  // namespace