[CodeHealth] Migrate a little extensions code to Once/RepeatingCallback.

Bug: 1007635
Change-Id: I84efa1eb7b59fb8906b94ca6c05d24bbdf565fde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2600383
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Auto-Submit: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840443}

extensions/browser/api/feedback_private/feedback_private_api_chromeos_unittest.cc

@@ -127,11 +127,11 @@ class FeedbackPrivateApiUnittest : public FeedbackPrivateApiUnittestBase {
 
     scoped_refptr<FeedbackData> actual_feedback_data;
     EXPECT_CALL(*mock, SendFeedback(_, _))
-        .WillOnce(Invoke([&](scoped_refptr<FeedbackData> feedback_data,
-                             FeedbackService::SendFeedbackCallback callback) {
-          actual_feedback_data = feedback_data;
+        .WillOnce([&](scoped_refptr<FeedbackData> feedback_data,
+                      FeedbackService::SendFeedbackCallback callback) {
+          actual_feedback_data = std::move(feedback_data);
           std::move(callback).Run(true);
-        }));
+        });
 
     FeedbackPrivateAPI::GetFactoryInstance()
         ->Get(browser_context())

extensions/browser/api/feedback_private/feedback_service.cc

@@ -8,6 +8,7 @@
 #include <string>
 #include <utility>
 
+#include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
@@ -26,9 +27,6 @@
 #include "mojo/public/cpp/bindings/remote.h"
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
-using content::BrowserThread;
-using feedback::FeedbackData;
-
 namespace extensions {
 
 FeedbackService::FeedbackService(content::BrowserContext* browser_context)
@@ -36,52 +34,54 @@ FeedbackService::FeedbackService(content::BrowserContext* browser_context)
 
 FeedbackService::~FeedbackService() = default;
 
-void FeedbackService::SendFeedback(scoped_refptr<FeedbackData> feedback_data,
-                                   SendFeedbackCallback callback) {
-  send_feedback_callback_ = std::move(callback);
-  feedback_data->set_locale(
-      ExtensionsBrowserClient::Get()->GetApplicationLocale());
-  feedback_data->set_user_agent(ExtensionsBrowserClient::Get()->GetUserAgent());
-
-  if (!feedback_data->attached_file_uuid().empty()) {
+void FeedbackService::SendFeedback(
+    scoped_refptr<feedback::FeedbackData> feedback_data,
+    SendFeedbackCallback callback) {
+  auto* browser_client = ExtensionsBrowserClient::Get();
+  feedback_data->set_locale(browser_client->GetApplicationLocale());
+  feedback_data->set_user_agent(browser_client->GetUserAgent());
+
+  // CompleteSendFeedback must be called once the attached file and screenshot
+  // have been read, if applicable. The barrier closure will call this when its
+  // count of remaining tasks has been reduced to zero (immediately, if none are
+  // there in the first place).
+  const bool must_attach_file = !feedback_data->attached_file_uuid().empty();
+  const bool must_attach_screenshot = !feedback_data->screenshot_uuid().empty();
+  auto barrier_closure = base::BarrierClosure(
+      (must_attach_file ? 1 : 0) + (must_attach_screenshot ? 1 : 0),
+      base::BindOnce(&FeedbackService::CompleteSendFeedback, AsWeakPtr(),
+                     feedback_data, std::move(callback)));
+
+  if (must_attach_file) {
+    auto populate_attached_file = base::BindOnce(
+        [](scoped_refptr<feedback::FeedbackData> feedback_data,
+           std::unique_ptr<std::string> data, int64_t /* total_blob_length */) {
+          feedback_data->set_attached_file_uuid(std::string());
+          if (data)
+            feedback_data->AttachAndCompressFileData(std::move(*data));
+        },
+        feedback_data);
     BlobReader::Read(browser_context_, feedback_data->attached_file_uuid(),
-                     base::BindOnce(&FeedbackService::AttachedFileCallback,
-                                    AsWeakPtr(), feedback_data));
+                     std::move(populate_attached_file).Then(barrier_closure));
   }
 
-  if (!feedback_data->screenshot_uuid().empty()) {
+  if (must_attach_screenshot) {
+    auto populate_screenshot = base::BindOnce(
+        [](scoped_refptr<feedback::FeedbackData> feedback_data,
+           std::unique_ptr<std::string> data, int64_t /* total_blob_length */) {
+          feedback_data->set_screenshot_uuid(std::string());
+          if (data)
+            feedback_data->set_image(std::move(*data));
+        },
+        feedback_data);
     BlobReader::Read(browser_context_, feedback_data->screenshot_uuid(),
-                     base::BindOnce(&FeedbackService::ScreenshotCallback,
-                                    AsWeakPtr(), feedback_data));
+                     std::move(populate_screenshot).Then(barrier_closure));
   }
-
-  CompleteSendFeedback(feedback_data);
-}
-
-void FeedbackService::AttachedFileCallback(
-    scoped_refptr<feedback::FeedbackData> feedback_data,
-    std::unique_ptr<std::string> data,
-    int64_t /* total_blob_length */) {
-  feedback_data->set_attached_file_uuid(std::string());
-  if (data)
-    feedback_data->AttachAndCompressFileData(std::move(*data));
-
-  CompleteSendFeedback(feedback_data);
-}
-
-void FeedbackService::ScreenshotCallback(
-    scoped_refptr<feedback::FeedbackData> feedback_data,
-    std::unique_ptr<std::string> data,
-    int64_t /* total_blob_length */) {
-  feedback_data->set_screenshot_uuid(std::string());
-  if (data)
-    feedback_data->set_image(std::move(*data));
-
-  CompleteSendFeedback(feedback_data);
 }
 
 void FeedbackService::CompleteSendFeedback(
-    scoped_refptr<feedback::FeedbackData> feedback_data) {
+    scoped_refptr<feedback::FeedbackData> feedback_data,
+    SendFeedbackCallback callback) {
   // A particular data collection is considered completed if,
   // a.) The blob URL is invalid - this will either happen because we never had
   //     a URL and never needed to read this data, or that the data read failed
@@ -89,31 +89,28 @@ void FeedbackService::CompleteSendFeedback(
   // b.) The associated data object exists, meaning that the data has been read
   //     and the read callback has updated the associated data on the feedback
   //     object.
-  const bool attached_file_completed =
-      feedback_data->attached_file_uuid().empty();
-  const bool screenshot_completed = feedback_data->screenshot_uuid().empty();
+  DCHECK(feedback_data->attached_file_uuid().empty());
+  DCHECK(feedback_data->screenshot_uuid().empty());
 
-  if (screenshot_completed && attached_file_completed) {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-    // Send feedback to Assistant server if triggered from Google Assistant.
-    if (feedback_data->from_assistant()) {
-      ash::AssistantController::Get()->SendAssistantFeedback(
-          feedback_data->assistant_debug_info_allowed(),
-          feedback_data->description(), feedback_data->image());
-    }
+  // Send feedback to Assistant server if triggered from Google Assistant.
+  if (feedback_data->from_assistant()) {
+    ash::AssistantController::Get()->SendAssistantFeedback(
+        feedback_data->assistant_debug_info_allowed(),
+        feedback_data->description(), feedback_data->image());
+  }
 #endif
 
-    // Signal the feedback object that the data from the feedback page has been
-    // filled - the object will manage sending of the actual report.
-    feedback_data->OnFeedbackPageDataComplete();
+  // Signal the feedback object that the data from the feedback page has been
+  // filled - the object will manage sending of the actual report.
+  feedback_data->OnFeedbackPageDataComplete();
 
-    // Sending the feedback will be delayed if the user is offline.
-    const bool result = !net::NetworkChangeNotifier::IsOffline();
+  // Sending the feedback will be delayed if the user is offline.
+  const bool result = !net::NetworkChangeNotifier::IsOffline();
 
-    // TODO(rkc): Change this once we have FeedbackData/Util refactored to
-    // report the status of the report being sent.
-    std::move(send_feedback_callback_).Run(result);
-  }
+  // TODO(rkc): Change this once we have FeedbackData/Util refactored to
+  // report the status of the report being sent.
+  std::move(callback).Run(result);
 }
 
 }  // namespace extensions

extensions/browser/api/feedback_private/feedback_service.h

@@ -39,20 +39,10 @@ class FeedbackService : public base::SupportsWeakPtr<FeedbackService> {
                             SendFeedbackCallback callback);
 
  private:
-  // Callbacks to receive blob data.
-  void AttachedFileCallback(scoped_refptr<feedback::FeedbackData> feedback_data,
-                            std::unique_ptr<std::string> data,
-                            int64_t total_blob_length);
-  void ScreenshotCallback(scoped_refptr<feedback::FeedbackData> feedback_data,
-                          std::unique_ptr<std::string> data,
-                          int64_t total_blob_length);
-
   // Checks if we have read all the blobs we need to; signals the feedback
   // data object once all the requisite data has been populated.
-  void CompleteSendFeedback(
-      scoped_refptr<feedback::FeedbackData> feedback_data);
-
-  SendFeedbackCallback send_feedback_callback_;
+  void CompleteSendFeedback(scoped_refptr<feedback::FeedbackData> feedback_data,
+                            SendFeedbackCallback callback);
 
   content::BrowserContext* browser_context_;
 

extensions/renderer/gc_callback.cc

@@ -21,7 +21,7 @@ GCCallback::GCCallback(ScriptContext* context,
     : GCCallback(context,
                  object,
                  callback,
-                 base::Closure(),
+                 base::OnceClosure(),
                  std::move(fallback)) {}
 
 GCCallback::GCCallback(ScriptContext* context,