Optimize sending sys info to the feedback app

chrome/browser/extensions/api/feedback_private/feedback_private_api.cc

Index: chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
diff --git a/chrome/browser/extensions/api/feedback_private/feedback_private_api.cc b/chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
index 10fdd04d3fd17bc5e712cedad514f285422830e2..a50cf56748f16c44edc03ce050efabd2154ddd9d 100644
--- a/chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
+++ b/chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/simple_message_box.h"
+#include "chrome/common/extensions/api/feedback_private.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/feedback/tracing_manager.h"
@@ -73,6 +74,9 @@ using feedback_private::SystemInformation;
 using feedback_private::FeedbackInfo;
 using feedback_private::FeedbackFlow;
 
+using SystemInformationList =
+    std::vector<api::feedback_private::SystemInformation>;
+
 static base::LazyInstance<BrowserContextKeyedAPIFactory<FeedbackPrivateAPI> >
     g_factory = LAZY_INSTANCE_INITIALIZER;
 
@@ -220,21 +224,33 @@ ExtensionFunction::ResponseAction FeedbackPrivateGetUserEmailFunction::Run() {
                      : std::string())));
 }
 
-bool FeedbackPrivateGetSystemInformationFunction::RunAsync() {
-  FeedbackService* service =
-      FeedbackPrivateAPI::GetFactoryInstance()->Get(GetProfile())->GetService();
+ExtensionFunction::ResponseAction
+FeedbackPrivateGetSystemInformationFunction::Run() {
+  FeedbackService* service = FeedbackPrivateAPI::GetFactoryInstance()
+                                 ->Get(browser_context())
+                                 ->GetService();
   DCHECK(service);
   service->GetSystemInformation(
       base::Bind(
           &FeedbackPrivateGetSystemInformationFunction::OnCompleted, this));

[xiyuan, 2017/01/17 18:38:14]

Is this Bind safe? |service| and |this| are not bound with each other.

[afakhry, 2017/01/17 18:51:13]

The ExtensionFunctions are RefCountedThreadSafe, so it should be safe.

[xiyuan, 2017/01/17 19:15:11]

Acknowledged. Forgot that. :p

-  return true;
+  return RespondLater();
 }
 
 void FeedbackPrivateGetSystemInformationFunction::OnCompleted(
-    const SystemInformationList& sys_info) {
-  results_ = feedback_private::GetSystemInformation::Results::Create(
-      sys_info);
-  SendResponse(true);
+    std::unique_ptr<system_logs::SystemLogsResponse> sys_info) {
+  SystemInformationList sys_info_list;
+  if (sys_info) {
+    sys_info_list.reserve(sys_info->size());
+    for (auto& itr : *sys_info) {
+      SystemInformation sys_info;

[xiyuan, 2017/01/17 18:38:14]

Can you rename this or |sys_info| arg of the function? It is a bit confusing to
read when two vars have the same name.

[afakhry, 2017/01/17 18:51:13]

Oops, that was a copy-paste mistake! :)

Done.

+      sys_info.key = std::move(itr.first);
+      sys_info.value = std::move(itr.second);
+      sys_info_list.emplace_back(std::move(sys_info));
+    }
+  }
+
+  Respond(ArgumentList(
+      feedback_private::GetSystemInformation::Results::Create(sys_info_list)));
 }
 
 bool FeedbackPrivateSendFeedbackFunction::RunAsync() {