Moving plugins in enterprise report from profile level to browser level.

Plugin belongs to browser instance. We don't need browser context to get
plugin lists. Hence move plugin from ChromeUserProfileInfo to
BrwoserReport.

1) Remove Plugin related code from ProfileReportGenerator
2) Because of 1), ProfileReportGenerator does not need to be async.
3) ReportGenerator remains async as Plugin information will be get here.

Getting plugin info in ReportGenerator will be added in the following CL.

Bug: 956237
Change-Id: I04c5ac519bc5c03e5436e10e98eebcf9a27b11fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764278
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: Marc-André Decoste <mad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690884}

chrome/browser/enterprise_reporting/profile_report_generator.cc

@@ -16,8 +16,6 @@
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
-#include "content/public/browser/plugin_service.h"
-#include "content/public/common/webplugininfo.h"
 
 namespace enterprise_reporting {
 
@@ -25,25 +23,21 @@ ProfileReportGenerator::ProfileReportGenerator() {}
 
 ProfileReportGenerator::~ProfileReportGenerator() = default;
 
-void ProfileReportGenerator::set_extensions_and_plugins_enabled(bool enabled) {
-  extensions_and_plugins_enabled_ = enabled;
+void ProfileReportGenerator::set_extensions_enabled(bool enabled) {
+  extensions_enabled_ = enabled;
 }
 
 void ProfileReportGenerator::set_policies_enabled(bool enabled) {
   policies_enabled_ = enabled;
 }
 
-void ProfileReportGenerator::MaybeGenerate(const base::FilePath& path,
-                                           const std::string& name,
-                                           ReportCallback callback) {
-  DCHECK(!callback_);
-  callback_ = std::move(callback);
-
+std::unique_ptr<em::ChromeUserProfileInfo>
+ProfileReportGenerator::MaybeGenerate(const base::FilePath& path,
+                                      const std::string& name) {
   profile_ = g_browser_process->profile_manager()->GetProfileByPath(path);
 
   if (!profile_) {
-    CheckReportStatus();
-    return;
+    return nullptr;
   }
 
   report_ = std::make_unique<em::ChromeUserProfileInfo>();
@@ -51,11 +45,8 @@ void ProfileReportGenerator::MaybeGenerate(const base::FilePath& path,
   report_->set_name(name);
   report_->set_is_full_report(true);
 
-  is_plugin_info_ready_ = false;
-
   GetSigninUserInfo();
   GetExtensionInfo();
-  GetPluginInfo();
 
   if (policies_enabled_) {
     // TODO(crbug.com/983151): Upload policy error as their IDs.
@@ -69,8 +60,7 @@ void ProfileReportGenerator::MaybeGenerate(const base::FilePath& path,
     GetPolicyFetchTimestampInfo();
   }
 
-  CheckReportStatus();
-  return;
+  return std::move(report_);
 }
 
 void ProfileReportGenerator::GetSigninUserInfo() {
@@ -84,22 +74,11 @@ void ProfileReportGenerator::GetSigninUserInfo() {
 }
 
 void ProfileReportGenerator::GetExtensionInfo() {
-  if (!extensions_and_plugins_enabled_)
+  if (!extensions_enabled_)
     return;
   AppendExtensionInfoIntoProfileReport(profile_, report_.get());
 }
 
-void ProfileReportGenerator::GetPluginInfo() {
-  if (!extensions_and_plugins_enabled_) {
-    is_plugin_info_ready_ = true;
-    return;
-  }
-
-  content::PluginService::GetInstance()->GetPlugins(
-      base::BindOnce(&ProfileReportGenerator::OnPluginsLoaded,
-                     weak_ptr_factory_.GetWeakPtr()));
-}
-
 void ProfileReportGenerator::GetChromePolicyInfo() {
   AppendChromePolicyInfoIntoProfileReport(policies_, report_.get());
 }
@@ -112,32 +91,5 @@ void ProfileReportGenerator::GetPolicyFetchTimestampInfo() {
   AppendMachineLevelUserCloudPolicyFetchTimestamp(report_.get());
 }
 
-void ProfileReportGenerator::OnPluginsLoaded(
-    const std::vector<content::WebPluginInfo>& plugins) {
-  for (auto plugin : plugins) {
-    auto* 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));
-  }
-
-  is_plugin_info_ready_ = true;
-  CheckReportStatus();
-}
-
-void ProfileReportGenerator::CheckReportStatus() {
-  // Report is not generated, return nullptr.
-  if (!report_) {
-    std::move(callback_).Run(nullptr);
-    return;
-  }
-
-  // Report is not ready, quit and wait.
-  if (!is_plugin_info_ready_)
-    return;
-
-  std::move(callback_).Run(std::move(report_));
-}
 
 }  // namespace enterprise_reporting

chrome/browser/enterprise_reporting/profile_report_generator.h

@@ -17,10 +17,6 @@ namespace base {
 class FilePath;
 }
 
-namespace content {
-struct WebPluginInfo;
-}
-
 class Profile;
 
 namespace enterprise_reporting {
@@ -31,43 +27,34 @@ namespace enterprise_reporting {
  */
 class ProfileReportGenerator {
  public:
-  using ReportCallback =
-      base::OnceCallback<void(std::unique_ptr<em::ChromeUserProfileInfo>)>;
 
   ProfileReportGenerator();
   ~ProfileReportGenerator();
 
-  void set_extensions_and_plugins_enabled(bool enabled);
+  void set_extensions_enabled(bool enabled);
   void set_policies_enabled(bool enabled);
 
   // Generates report for Profile if it's activated. Returns the report with
   // |callback| once it's ready. The report is null if it can't be generated.
-  void MaybeGenerate(const base::FilePath& path,
-                     const std::string& name,
-                     ReportCallback callback);
+  std::unique_ptr<em::ChromeUserProfileInfo> MaybeGenerate(
+      const base::FilePath& path,
+      const std::string& name);
 
  protected:
   // Get Signin information includes email and gaia id.
   virtual void GetSigninUserInfo();
 
   void GetExtensionInfo();
-  void GetPluginInfo();
   void GetChromePolicyInfo();
   void GetExtensionPolicyInfo();
   void GetPolicyFetchTimestampInfo();
 
  private:
-  void OnPluginsLoaded(const std::vector<content::WebPluginInfo>& plugins);
-
-  void CheckReportStatus();
-
   Profile* profile_;
   base::Value policies_;
 
-  bool extensions_and_plugins_enabled_ = true;
+  bool extensions_enabled_ = true;
   bool policies_enabled_ = true;
-  bool is_plugin_info_ready_ = false;
-  ReportCallback callback_;
 
   std::unique_ptr<em::ChromeUserProfileInfo> report_ = nullptr;
 

chrome/browser/enterprise_reporting/profile_report_generator_unittest.cc

@@ -4,19 +4,14 @@
 
 #include "chrome/browser/enterprise_reporting/profile_report_generator.h"
 
-#include "base/logging.h"
-#include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/bind_test_util.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/signin/identity_test_environment_profile_adaptor.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 "components/signin/public/identity_manager/identity_test_environment.h"
-#include "content/public/browser/plugin_service.h"
-#include "content/public/common/webplugininfo.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -26,11 +21,6 @@ namespace {
 const char kProfile[] = "Profile";
 const char kIdleProfile[] = "IdleProfile";
 
-const char kPluginName[] = "plugin";
-const char kPluginVersion[] = "1.0";
-const char kPluginDescription[] = "This is a plugin.";
-const char kPluginFileName[] = "file_name";
-
 }  // namespace
 
 class ProfileReportGeneratorTest : public ::testing::Test {
@@ -42,25 +32,13 @@ class ProfileReportGeneratorTest : public ::testing::Test {
   void SetUp() override {
     ASSERT_TRUE(profile_manager_.SetUp());
     profile_ = profile_manager_.CreateTestingProfile(kProfile);
-    content::PluginService::GetInstance()->Init();
   }
 
   std::unique_ptr<em::ChromeUserProfileInfo> GenerateReport(
       const base::FilePath& path,
       const std::string& name) {
-    base::RunLoop run_loop;
     std::unique_ptr<em::ChromeUserProfileInfo> report =
-        std::make_unique<em::ChromeUserProfileInfo>();
-    generator_.MaybeGenerate(
-        path, name,
-        base::BindLambdaForTesting(
-            [&run_loop, report = report.get()](
-                std::unique_ptr<em::ChromeUserProfileInfo> response) {
-              DCHECK(response);
-              report->Swap(response.get());
-              run_loop.Quit();
-            }));
-    run_loop.Run();
+        generator_.MaybeGenerate(path, name);
     return report;
   }
 
@@ -75,19 +53,6 @@ class ProfileReportGeneratorTest : public ::testing::Test {
     return report;
   }
 
-  void CreatePlugin() {
-    content::WebPluginInfo info;
-    info.name = base::ASCIIToUTF16(kPluginName);
-    info.version = base::ASCIIToUTF16(kPluginVersion);
-    info.desc = base::ASCIIToUTF16(kPluginDescription);
-    info.path =
-        base::FilePath().AppendASCII("path").AppendASCII(kPluginFileName);
-    content::PluginService* plugin_service =
-        content::PluginService::GetInstance();
-    plugin_service->RegisterInternalPlugin(info, true);
-    plugin_service->RefreshPlugins();
-  }
-
   TestingProfile* profile() { return profile_; }
   TestingProfileManager* profile_manager() { return &profile_manager_; }
 
@@ -107,15 +72,9 @@ TEST_F(ProfileReportGeneratorTest, ProfileNotActivated) {
   profile_manager()->profile_attributes_storage()->AddProfile(
       profile_path, base::ASCIIToUTF16(kIdleProfile), std::string(),
       base::string16(), 0, std::string(), EmptyAccountId());
-  base::RunLoop run_loop;
-  generator_.MaybeGenerate(
-      profile_path, kIdleProfile,
-      base::BindLambdaForTesting(
-          [&run_loop](std::unique_ptr<em::ChromeUserProfileInfo> response) {
-            EXPECT_FALSE(response);
-            run_loop.Quit();
-          }));
-  run_loop.Run();
+  std::unique_ptr<em::ChromeUserProfileInfo> response =
+      generator_.MaybeGenerate(profile_path, kIdleProfile);
+  ASSERT_FALSE(response.get());
 }
 
 TEST_F(ProfileReportGeneratorTest, UnsignedInProfile) {
@@ -135,23 +94,4 @@ TEST_F(ProfileReportGeneratorTest, SignedInProfile) {
             report->chrome_signed_in_user().obfudscated_gaia_id());
 }
 
-TEST_F(ProfileReportGeneratorTest, PluginIsDisabled) {
-  CreatePlugin();
-  generator_.set_extensions_and_plugins_enabled(false);
-  auto report = GenerateReport();
-  EXPECT_EQ(0, report->plugins_size());
-}
-
-TEST_F(ProfileReportGeneratorTest, PluginIsEnabled) {
-  CreatePlugin();
-  auto report = GenerateReport();
-  // There might be other plugins like PDF plugin, however, our fake plugin
-  // should be the first one in the report.
-  EXPECT_LE(1, report->plugins_size());
-  EXPECT_EQ(kPluginName, report->plugins(0).name());
-  EXPECT_EQ(kPluginVersion, report->plugins(0).version());
-  EXPECT_EQ(kPluginDescription, report->plugins(0).description());
-  EXPECT_EQ(kPluginFileName, report->plugins(0).filename());
-}
-
 }  // namespace enterprise_reporting

chrome/browser/enterprise_reporting/report_generator.cc

@@ -52,14 +52,18 @@ void ReportGenerator::Generate(ReportCallback callback) {
   if (basic_request_size_ > maximum_report_size_) {
     // Basic request is already too large so we can't upload any valid report.
     // Skip all Profiles and response an empty request list.
-    GetNextProfileReport(
-        basic_request_.browser_report().chrome_user_profile_infos_size());
+    std::move(callback_).Run(std::move(requests_));
     return;
   }
 
   requests_.push(
       std::make_unique<em::ChromeDesktopReportRequest>(basic_request_));
-  GetNextProfileReport(0);
+  for (int index = 0;
+       index < basic_request_.browser_report().chrome_user_profile_infos_size();
+       index++) {
+    GenerateProfileReportWithIndex(index);
+  }
+  std::move(callback_).Run(std::move(requests_));
 }
 
 void ReportGenerator::SetMaximumReportSizeForTesting(size_t size) {
@@ -127,29 +131,18 @@ ReportGenerator::GetProfiles() {
   return profiles;
 }
 
-void ReportGenerator::GetNextProfileReport(int profile_index) {
-  if (profile_index >=
-      basic_request_.browser_report().chrome_user_profile_infos_size()) {
-    std::move(callback_).Run(std::move(requests_));
-    return;
-  }
+void ReportGenerator::GenerateProfileReportWithIndex(int profile_index) {
+  DCHECK_LT(profile_index,
+            basic_request_.browser_report().chrome_user_profile_infos_size());
 
   auto basic_profile_report =
       basic_request_.browser_report().chrome_user_profile_infos(profile_index);
-  profile_report_generator_.MaybeGenerate(
+  auto profile_report = profile_report_generator_.MaybeGenerate(
       base::FilePath::FromUTF8Unsafe(basic_profile_report.id()),
-      basic_profile_report.name(),
-      base::BindOnce(&ReportGenerator::OnProfileReportReady,
-                     weak_ptr_factory_.GetWeakPtr(), profile_index));
-}
+      basic_profile_report.name());
 
-void ReportGenerator::OnProfileReportReady(
-    int profile_index,
-    std::unique_ptr<em::ChromeUserProfileInfo> profile_report) {
-  // Move to the next Profile if Profile is not loaded and there is no full
-  // report.
+  // Return if Profile is not loaded and there is no full report.
   if (!profile_report) {
-    GetNextProfileReport(profile_index + 1);
     return;
   }
 
@@ -176,8 +169,6 @@ void ReportGenerator::OnProfileReportReady(
   // Else: The new full Profile report is too big to be uploaded, skip this
   // Profile report.
   // TODO(crbug.com/956237): Record this event with UMA metrics.
-
-  GetNextProfileReport(profile_index + 1);
 }
 
 }  // namespace enterprise_reporting

chrome/browser/enterprise_reporting/report_generator.h

@@ -57,10 +57,7 @@ class ReportGenerator {
   std::vector<std::unique_ptr<em::ChromeUserProfileInfo>> GetProfiles();
 
  private:
-  void GetNextProfileReport(int profile_index);
-  void OnProfileReportReady(
-      int profile_index,
-      std::unique_ptr<em::ChromeUserProfileInfo> profile_report);
+  void GenerateProfileReportWithIndex(int profile_index);
 
   ProfileReportGenerator profile_report_generator_;
 

components/policy/proto/device_management_backend.proto

@@ -1287,6 +1287,8 @@ message PolicyFetchTimestamp {
 // Chrome user profile level status, used by activated Profiles. Profile name is
 // not listed here as they are in the ChromeUserProfileBasicInfo.
 message ChromeUserProfileInfo {
+  reserved 6;
+
   // A string to uniquely identify this profile within the browser.
   optional string id = 1;
 
@@ -1308,9 +1310,6 @@ message ChromeUserProfileInfo {
   // A list of extensions requested for installation.
   repeated ExtensionRequest extension_requests = 10;
 
-  // A list of plugins installed in the browser.
-  repeated Plugin plugins = 6;
-
   // A list of Chrome browser policies set for this user profile.
   repeated Policy chrome_policies = 7;
 
@@ -1347,6 +1346,9 @@ message BrowserReport {
   // upload full details due to the limitation of the report size.
   // These details will be uploaded in the following reports.
   repeated ChromeUserProfileInfo chrome_user_profile_infos = 6;
+
+  // A list of plugins installed in the browser.
+  repeated Plugin plugins = 7;
 }
 
 // Report Operating system related information.