Refactor class ReportGenerator for ChromeOsUserReportGenerator

Extract the logic of generating BrowserReport from ReportGenerator class. It
can be shared by following classes.
* ReportGenerator: which is existing to generate ChromeDesktopReportRequest.
* ChromeOsUserReportGenerator: which will be newly added to generate
  ChromeOsUserReportRequest (for Chrome OS only).

Bug: 1010213
Change-Id: I4bd64080319732bd200ae719dd1b9af00d41cdda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1876033
Commit-Queue: Anqing Zhao <anqing@google.com>
Reviewed-by: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710551}

chrome/browser/BUILD.gn

@@ -3030,6 +3030,8 @@ jumbo_static_library("browser") {
       "download/download_shelf_context_menu.h",
       "download/download_shelf_controller.cc",
       "download/download_shelf_controller.h",
+      "enterprise_reporting/browser_report_generator.cc",
+      "enterprise_reporting/browser_report_generator.h",
       "enterprise_reporting/extension_info.cc",
       "enterprise_reporting/extension_info.h",
       "enterprise_reporting/policy_info.cc",

chrome/browser/enterprise_reporting/browser_report_generator.cc

+// Copyright 2019 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/enterprise_reporting/browser_report_generator.h"
+
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/channel_info.h"
+#include "components/policy/core/common/cloud/cloud_policy_util.h"
+#include "components/version_info/channel.h"
+#include "components/version_info/version_info.h"
+#include "content/public/browser/plugin_service.h"
+#include "content/public/common/webplugininfo.h"
+
+namespace {
+
+std::string GetExecutablePath() {
+  base::FilePath path;
+  base::PathService::Get(base::DIR_EXE, &path);
+  return path.AsUTF8Unsafe();
+}
+
+}  // namespace
+
+namespace enterprise_reporting {
+
+BrowserReportGenerator::BrowserReportGenerator() = default;
+
+BrowserReportGenerator::~BrowserReportGenerator() = default;
+
+void BrowserReportGenerator::Generate(ReportCallback callback) {
+  DCHECK(!callback_);
+  callback_ = std::move(callback);
+
+  auto report = std::make_unique<em::BrowserReport>();
+  report->set_browser_version(version_info::GetVersionNumber());
+  report->set_channel(policy::ConvertToProtoChannel(chrome::GetChannel()));
+  report->set_executable_path(GetExecutablePath());
+
+  for (auto* entry : g_browser_process->profile_manager()
+                         ->GetProfileAttributesStorage()
+                         .GetAllProfilesAttributes()) {
+    em::ChromeUserProfileInfo* profile =
+        report->add_chrome_user_profile_infos();
+    profile->set_id(entry->GetPath().AsUTF8Unsafe());
+    profile->set_name(base::UTF16ToUTF8(entry->GetName()));
+    profile->set_is_full_report(false);
+  }
+
+  content::PluginService::GetInstance()->GetPlugins(
+      base::BindOnce(&BrowserReportGenerator::OnPluginsReady,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(report)));
+}
+
+void BrowserReportGenerator::OnPluginsReady(
+    std::unique_ptr<em::BrowserReport> report,
+    const std::vector<content::WebPluginInfo>& plugins) {
+  for (content::WebPluginInfo plugin : plugins) {
+    em::Plugin* plugin_info = report->add_plugins();
+    plugin_info->set_name(base::UTF16ToUTF8(plugin.name));
+    plugin_info->set_version(base::UTF16ToUTF8(plugin.version));
+    plugin_info->set_filename(plugin.path.BaseName().AsUTF8Unsafe());
+    plugin_info->set_description(base::UTF16ToUTF8(plugin.desc));
+  }
+
+  std::move(callback_).Run(std::move(report));
+}
+
+}  // namespace enterprise_reporting

chrome/browser/enterprise_reporting/browser_report_generator.h

+// Copyright 2019 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_ENTERPRISE_REPORTING_BROWSER_REPORT_GENERATOR_H_
+#define CHROME_BROWSER_ENTERPRISE_REPORTING_BROWSER_REPORT_GENERATOR_H_
+
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "components/policy/proto/device_management_backend.pb.h"
+
+namespace em = enterprise_management;
+
+namespace content {
+struct WebPluginInfo;
+}
+
+namespace enterprise_reporting {
+
+/**
+ * A report generator that collects Browser related information.
+ */
+class BrowserReportGenerator {
+ public:
+  using ReportCallback =
+      base::OnceCallback<void(std::unique_ptr<em::BrowserReport>)>;
+
+  BrowserReportGenerator();
+  ~BrowserReportGenerator();
+
+  // Generate a BrowserReport with following fields.
+  // - browser_version, channel, executable_path
+  // - user profiles: id, name, is_full_report (always be false).
+  // - plugins: name, version, filename, description.
+  void Generate(ReportCallback callback);
+
+ private:
+  void OnPluginsReady(std::unique_ptr<em::BrowserReport> report,
+                      const std::vector<content::WebPluginInfo>& plugins);
+
+  ReportCallback callback_;
+
+  base::WeakPtrFactory<BrowserReportGenerator> weak_ptr_factory_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserReportGenerator);
+};
+
+}  // namespace enterprise_reporting
+
+#endif  // CHROME_BROWSER_ENTERPRISE_REPORTING_BROWSER_REPORT_GENERATOR_H_

chrome/browser/enterprise_reporting/browser_report_generator_unittest.cc

+// Copyright 2019 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/enterprise_reporting/browser_report_generator.h"
+
+#include <memory>
+
+#include "base/files/file_path.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/enterprise_reporting/profile_report_generator.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "components/account_id/account_id.h"
+#include "content/public/browser/plugin_service.h"
+#include "content/public/common/webplugininfo.h"
+#include "content/public/test/browser_task_environment.h"
+#include "device_management_backend.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace em = enterprise_management;
+
+namespace enterprise_reporting {
+namespace {
+
+const char kProfileId[] = "profile_id";
+const char kProfileName[] = "profile_name";
+
+const char kPluginName[] = "plugin_name";
+const char kPluginVersion[] = "plugin_version";
+const char kPluginDescription[] = "plugin_description";
+const char kPluginFolderPath[] = "plugin_folder_path";
+const char kPluginFileName[] = "plugin_file_name";
+
+}  // namespace
+
+class BrowserReportGeneratorTest : public ::testing::Test {
+ public:
+  BrowserReportGeneratorTest()
+      : profile_manager_(TestingBrowserProcess::GetGlobal()) {}
+  ~BrowserReportGeneratorTest() override = default;
+
+  void SetUp() override {
+    ASSERT_TRUE(profile_manager_.SetUp());
+    profile_manager_.CreateGuestProfile();
+    profile_manager_.CreateSystemProfile();
+    content::PluginService::GetInstance()->Init();
+  }
+
+  void InitializeProfile() {
+    profile_manager_.profile_attributes_storage()->AddProfile(
+        profile_manager()->profiles_dir().AppendASCII(kProfileId),
+        base::ASCIIToUTF16(kProfileName), std::string(), base::string16(),
+        false, 0, std::string(), EmptyAccountId());
+  }
+
+  void InitializePlugin() {
+    content::WebPluginInfo info;
+    info.name = base::ASCIIToUTF16(kPluginName);
+    info.version = base::ASCIIToUTF16(kPluginVersion);
+    info.desc = base::ASCIIToUTF16(kPluginDescription);
+    info.path = base::FilePath()
+                    .AppendASCII(kPluginFolderPath)
+                    .AppendASCII(kPluginFileName);
+
+    content::PluginService* plugin_service =
+        content::PluginService::GetInstance();
+    plugin_service->RegisterInternalPlugin(info, true);
+    plugin_service->RefreshPlugins();
+  }
+
+  void GenerateAndVerify() {
+    base::RunLoop run_loop;
+    generator_.Generate(base::BindLambdaForTesting(
+        [&run_loop](std::unique_ptr<em::BrowserReport> report) {
+          EXPECT_TRUE(report.get());
+
+          EXPECT_NE(std::string(), report->browser_version());
+          EXPECT_NE(std::string(), report->executable_path());
+          EXPECT_TRUE(report->has_channel());
+
+          EXPECT_EQ(1, report->chrome_user_profile_infos_size());
+          em::ChromeUserProfileInfo profile =
+              report->chrome_user_profile_infos(0);
+          EXPECT_NE(std::string(), profile.id());
+          EXPECT_EQ(kProfileName, profile.name());
+          EXPECT_FALSE(profile.is_full_report());
+
+          EXPECT_LE(1, report->plugins_size());
+          em::Plugin plugin = report->plugins(0);
+          EXPECT_EQ(kPluginName, plugin.name());
+          EXPECT_EQ(kPluginVersion, plugin.version());
+          EXPECT_EQ(kPluginFileName, plugin.filename());
+          EXPECT_EQ(kPluginDescription, plugin.description());
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+  }
+
+  TestingProfileManager* profile_manager() { return &profile_manager_; }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfileManager profile_manager_;
+  BrowserReportGenerator generator_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserReportGeneratorTest);
+};
+
+TEST_F(BrowserReportGeneratorTest, GenerateBasicReport) {
+  InitializeProfile();
+  InitializePlugin();
+  GenerateAndVerify();
+}
+
+}  // namespace enterprise_reporting

chrome/browser/enterprise_reporting/report_generator.cc

@@ -6,22 +6,13 @@
 
 #include <utility>
 
-#include "base/base_paths.h"
+#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/profile_attributes_entry.h"
-#include "chrome/browser/profiles/profile_attributes_storage.h"
-#include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/common/channel_info.h"
 #include "components/policy/core/common/cloud/cloud_policy_util.h"
-#include "components/version_info/channel.h"
-#include "components/version_info/version_info.h"
-#include "content/public/browser/plugin_service.h"
-#include "content/public/common/webplugininfo.h"
 
 #if defined(OS_WIN)
 #include "base/win/wmi.h"
@@ -48,12 +39,6 @@ constexpr char kBasicRequestSizeMetricsName[] =
 // are needed if there are many reports exceed this limitation.
 const int kRequestCountMetricMaxValue = 21;
 
-std::string GetChromePath() {
-  base::FilePath path;
-  base::PathService::Get(base::DIR_EXE, &path);
-  return path.AsUTF8Unsafe();
-}
-
 }  // namespace
 
 ReportGenerator::ReportGenerator() : maximum_report_size_(kMaximumReportSize) {}
@@ -75,14 +60,8 @@ void ReportGenerator::CreateBasicRequest() {
   basic_request_.set_os_user_name(GetOSUserName());
   basic_request_.set_serial_number(GetSerialNumber());
   basic_request_.set_allocated_os_report(GetOSReport().release());
-  basic_request_.set_allocated_browser_report(GetBrowserReport().release());
-  for (auto& profile : GetProfiles()) {
-    basic_request_.mutable_browser_report()
-        ->add_chrome_user_profile_infos()
-        ->Swap(profile.get());
-  }
-  content::PluginService::GetInstance()->GetPlugins(base::BindOnce(
-      &ReportGenerator::OnPluginsReady, weak_ptr_factory_.GetWeakPtr()));
+  browser_report_generator_.Generate(base::BindOnce(
+      &ReportGenerator::OnBrowserReportReady, weak_ptr_factory_.GetWeakPtr()));
 }
 
 std::unique_ptr<em::OSReport> ReportGenerator::GetOSReport() {
@@ -110,28 +89,6 @@ std::string ReportGenerator::GetSerialNumber() {
 #endif
 }
 
-std::unique_ptr<em::BrowserReport> ReportGenerator::GetBrowserReport() {
-  auto report = std::make_unique<em::BrowserReport>();
-  report->set_browser_version(version_info::GetVersionNumber());
-  report->set_channel(policy::ConvertToProtoChannel(chrome::GetChannel()));
-  report->set_executable_path(GetChromePath());
-  return report;
-}
-
-std::vector<std::unique_ptr<em::ChromeUserProfileInfo>>
-ReportGenerator::GetProfiles() {
-  std::vector<std::unique_ptr<em::ChromeUserProfileInfo>> profiles;
-  for (auto* entry : g_browser_process->profile_manager()
-                         ->GetProfileAttributesStorage()
-                         .GetAllProfilesAttributes()) {
-    profiles.push_back(std::make_unique<em::ChromeUserProfileInfo>());
-    profiles.back()->set_id(entry->GetPath().AsUTF8Unsafe());
-    profiles.back()->set_name(base::UTF16ToUTF8(entry->GetName()));
-    profiles.back()->set_is_full_report(false);
-  }
-  return profiles;
-}
-
 void ReportGenerator::GenerateProfileReportWithIndex(int profile_index) {
   DCHECK_LT(profile_index,
             basic_request_.browser_report().chrome_user_profile_infos_size());
@@ -186,21 +143,9 @@ void ReportGenerator::GenerateProfileReportWithIndex(int profile_index) {
   }
 }
 
-void ReportGenerator::OnPluginsReady(
-    const std::vector<content::WebPluginInfo>& plugins) {
-  auto* browser_report = basic_request_.mutable_browser_report();
-  for (auto plugin : plugins) {
-    auto* plugin_info = browser_report->add_plugins();
-    plugin_info->set_name(base::UTF16ToUTF8(plugin.name));
-    plugin_info->set_version(base::UTF16ToUTF8(plugin.version));
-    plugin_info->set_filename(plugin.path.BaseName().AsUTF8Unsafe());
-    plugin_info->set_description(base::UTF16ToUTF8(plugin.desc));
-  }
-
-  OnBasicRequestReady();
-}
-
-void ReportGenerator::OnBasicRequestReady() {
+void ReportGenerator::OnBrowserReportReady(
+    std::unique_ptr<em::BrowserReport> browser_report) {
+  basic_request_.set_allocated_browser_report(browser_report.release());
   basic_request_size_ = basic_request_.ByteSizeLong();
   base::UmaHistogramMemoryKB(kBasicRequestSizeMetricsName,
                              basic_request_size_ / 1024);

chrome/browser/enterprise_reporting/report_generator.h

@@ -11,15 +11,12 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "chrome/browser/enterprise_reporting/browser_report_generator.h"
 #include "chrome/browser/enterprise_reporting/profile_report_generator.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 
 namespace em = enterprise_management;
 
-namespace content {
-struct WebPluginInfo;
-}
-
 namespace enterprise_reporting {
 
 class ReportGenerator {
@@ -52,23 +49,15 @@ class ReportGenerator {
   // on other platforms.
   virtual std::string GetSerialNumber();
 
-  // Returns a browser report contains browser related information includes
-  // browser version, channel and executable path.
-  virtual std::unique_ptr<em::BrowserReport> GetBrowserReport();
-
-  // Returns the list of Profiles that is owned by current Browser instance. It
-  // only contains Profile's path and name.
-  std::vector<std::unique_ptr<em::ChromeUserProfileInfo>> GetProfiles();
-
  private:
   void GenerateProfileReportWithIndex(int profile_index);
 
-  void OnPluginsReady(const std::vector<content::WebPluginInfo>& plugins);
-  void OnBasicRequestReady();
+  void OnBrowserReportReady(std::unique_ptr<em::BrowserReport> browser_report);
 
   void Response();
 
   ProfileReportGenerator profile_report_generator_;
+  BrowserReportGenerator browser_report_generator_;
 
   ReportCallback callback_;
 

chrome/test/BUILD.gn

@@ -3689,6 +3689,7 @@ test("unit_tests") {
       "../browser/diagnostics/diagnostics_model_unittest.cc",
       "../browser/download/download_commands_unittest.cc",
       "../browser/download/download_shelf_unittest.cc",
+      "../browser/enterprise_reporting/browser_report_generator_unittest.cc",
       "../browser/enterprise_reporting/extension_info_unittest.cc",
       "../browser/enterprise_reporting/policy_info_unittest.cc",
       "../browser/enterprise_reporting/profile_report_generator_unittest.cc",