Add full Profile report into enterprise report.

Full profile reports are append into the report for all activated profiles.
Multiple requests are created if total report size exceeds the limitation.

Bug: 956237
Change-Id: Iee36c34857346ca2a6aa2dbbfe7c09301fd3d129
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702770
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678372}

chrome/browser/enterprise_reporting/profile_report_generator.cc

@@ -36,12 +36,13 @@ void ProfileReportGenerator::set_policies_enabled(bool enabled) {
 void ProfileReportGenerator::MaybeGenerate(const base::FilePath& path,
                                            const std::string& name,
                                            ReportCallback callback) {
+  DCHECK(!callback_);
   callback_ = std::move(callback);
 
   profile_ = g_browser_process->profile_manager()->GetProfileByPath(path);
 
   if (!profile_) {
-    CheckReportStatusAsync();
+    CheckReportStatus();
     return;
   }
 
@@ -66,7 +67,7 @@ void ProfileReportGenerator::MaybeGenerate(const base::FilePath& path,
     GetExtensionPolicyInfo();
   }
 
-  CheckReportStatusAsync();
+  CheckReportStatus();
   return;
 }
 
@@ -133,10 +134,4 @@ void ProfileReportGenerator::CheckReportStatus() {
   std::move(callback_).Run(std::move(report_));
 }
 
-void ProfileReportGenerator::CheckReportStatusAsync() {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(&ProfileReportGenerator::CheckReportStatus,
-                                weak_ptr_factory_.GetWeakPtr()));
-}
-
 }  // namespace enterprise_reporting

chrome/browser/enterprise_reporting/profile_report_generator.h

@@ -60,7 +60,6 @@ class ProfileReportGenerator {
   void OnPluginsLoaded(const std::vector<content::WebPluginInfo>& plugins);
 
   void CheckReportStatus();
-  void CheckReportStatusAsync();
 
   Profile* profile_;
   base::Value policies_;

chrome/browser/enterprise_reporting/report_generator.cc

@@ -29,6 +29,9 @@ namespace em = enterprise_management;
 namespace enterprise_reporting {
 namespace {
 
+const size_t kMaximumReportSize =
+    5000000;  // The report size limitation is 5mb.
+
 std::string GetChromePath() {
   base::FilePath path;
   base::PathService::Get(base::DIR_EXE, &path);
@@ -37,15 +40,31 @@ std::string GetChromePath() {
 
 }  // namespace
 
-ReportGenerator::ReportGenerator() = default;
+ReportGenerator::ReportGenerator()
+    : maximum_report_size_(kMaximumReportSize), weak_ptr_factory_(this) {}
+
 ReportGenerator::~ReportGenerator() = default;
 
-std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>>
-ReportGenerator::Generate() {
+void ReportGenerator::Generate(ReportCallback callback) {
+  DCHECK(!callback_);
+  callback_ = std::move(callback);
   CreateBasicRequest();
+
+  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().profile_info_list_size());
+    return;
+  }
+
   requests_.push_back(
       std::make_unique<em::ChromeDesktopReportRequest>(basic_request_));
-  return std::move(requests_);
+  GetNextProfileReport(0);
+}
+
+void ReportGenerator::SetMaximumReportSizeForTesting(size_t size) {
+  maximum_report_size_ = size;
 }
 
 void ReportGenerator::CreateBasicRequest() {
@@ -58,6 +77,7 @@ void ReportGenerator::CreateBasicRequest() {
     basic_request_.mutable_browser_report()->add_profile_info_list()->Swap(
         profile.get());
   }
+  basic_request_size_ = basic_request_.ByteSizeLong();
 }
 
 std::unique_ptr<em::OSReport> ReportGenerator::GetOSReport() {
@@ -107,4 +127,57 @@ ReportGenerator::GetProfiles() {
   return profiles;
 }
 
+void ReportGenerator::GetNextProfileReport(int profile_index) {
+  if (profile_index >=
+      basic_request_.browser_report().profile_info_list_size()) {
+    std::move(callback_).Run(std::move(requests_));
+    return;
+  }
+
+  auto basic_profile_report =
+      basic_request_.browser_report().profile_info_list(profile_index);
+  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));
+}
+
+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.
+  if (!profile_report) {
+    GetNextProfileReport(profile_index + 1);
+    return;
+  }
+
+  size_t profile_report_size = profile_report->ByteSizeLong();
+  size_t current_request_size = requests_.back()->ByteSizeLong();
+
+  if (current_request_size + profile_report_size <= maximum_report_size_) {
+    // The new full Profile report can be appended into the current request.
+    requests_.back()
+        ->mutable_browser_report()
+        ->mutable_profile_info_list(profile_index)
+        ->Swap(profile_report.get());
+  } else if (basic_request_size_ + profile_report_size <=
+             maximum_report_size_) {
+    // The new full Profile report is too big to be appended into the current
+    // request, move it to the next request if possible.
+    requests_.push_back(
+        std::make_unique<em::ChromeDesktopReportRequest>(basic_request_));
+    requests_.back()
+        ->mutable_browser_report()
+        ->mutable_profile_info_list(profile_index)
+        ->Swap(profile_report.get());
+  }
+  // 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

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "chrome/browser/enterprise_reporting/profile_report_generator.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 
 namespace em = enterprise_management;
@@ -18,10 +19,15 @@ namespace enterprise_reporting {
 
 class ReportGenerator {
  public:
+  using ReportCallback = base::OnceCallback<void(
+      std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>>)>;
+
   ReportGenerator();
   ~ReportGenerator();
 
-  std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>> Generate();
+  void Generate(ReportCallback callback);
+
+  void SetMaximumReportSizeForTesting(size_t size);
 
  protected:
   // Creates a basic request that will be used by all Profiles.
@@ -50,10 +56,24 @@ 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);
+
+  ProfileReportGenerator profile_report_generator_;
+
+  ReportCallback callback_;
+
   std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>> requests_;
 
   // Basic information that is shared among requests.
   em::ChromeDesktopReportRequest basic_request_;
+  size_t basic_request_size_;
+
+  size_t maximum_report_size_;
+
+  base::WeakPtrFactory<ReportGenerator> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ReportGenerator);
 };

chrome/browser/enterprise_reporting/report_generator_unittest.cc

@@ -6,23 +6,30 @@
 
 #include <set>
 
-#include "base/logging.h"
+#include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "build/build_config.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/test/test_browser_thread_bundle.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension_builder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace em = enterprise_management;
 
 namespace enterprise_reporting {
 namespace {
+
 #if !defined(OS_CHROMEOS)
+constexpr char kProfile[] = "Profile";
+
 // We only upload serial number on Windows.
 void VerifySerialNumber(const std::string& serial_number) {
 #if defined(OS_WIN)
@@ -32,7 +39,14 @@ void VerifySerialNumber(const std::string& serial_number) {
 #endif
 }
 
-const char kProfile1[] = "Profile";
+// Verify the name is in the set. Remove the name from the set afterwards.
+void FindAndRemoveProfileName(std::set<std::string>* names,
+                              const std::string& name) {
+  auto it = names->find(name);
+  EXPECT_NE(names->end(), it);
+  names->erase(it);
+}
+
 #endif
 }  // namespace
 
@@ -43,7 +57,14 @@ class ReportGeneratorTest : public ::testing::Test {
       : profile_manager_(TestingBrowserProcess::GetGlobal()) {}
   ~ReportGeneratorTest() override = default;
 
-  void SetUp() override { ASSERT_TRUE(profile_manager_.SetUp()); }
+  void SetUp() override {
+    ASSERT_TRUE(profile_manager_.SetUp());
+
+    profile_manager_.CreateGuestProfile();
+    profile_manager_.CreateSystemProfile();
+
+    content::PluginService::GetInstance()->Init();
+  }
 
   // Creates |number| of Profiles. Returns the set of their names. The profile
   // names begin with Profile|start_index|. Profile instances are created if
@@ -55,7 +76,7 @@ class ReportGeneratorTest : public ::testing::Test {
     std::set<std::string> profile_names;
     for (int i = start_index; i < number; i++) {
       std::string profile_name =
-          std::string(kProfile1) + base::NumberToString(i);
+          std::string(kProfile) + base::NumberToString(i);
       if (is_active) {
         profile_manager_.CreateTestingProfile(profile_name);
       } else {
@@ -66,14 +87,63 @@ class ReportGeneratorTest : public ::testing::Test {
       }
       profile_names.insert(profile_name);
     }
-    profile_manager_.CreateGuestProfile();
-    profile_manager_.CreateSystemProfile();
     return profile_names;
   }
 
+  std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>>
+  GenerateRequests() {
+    base::RunLoop run_loop;
+    std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>> rets;
+    generator_.Generate(base::BindLambdaForTesting(
+        [&run_loop,
+         &rets](std::vector<std::unique_ptr<em::ChromeDesktopReportRequest>>
+                    requests) {
+          rets = std::move(requests);
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+    return rets;
+  }
+
+  // Verify the profile report matches actual Profile setup.
+  void VerifyProfileReport(const std::set<std::string>& active_profiles_names,
+                           const std::set<std::string>& inactive_profiles_names,
+                           const em::BrowserReport& actual_browser_report) {
+    int expected_profile_number =
+        active_profiles_names.size() + inactive_profiles_names.size();
+    EXPECT_EQ(expected_profile_number,
+              actual_browser_report.profile_info_list_size());
+
+    auto mutable_active_profiles_names(active_profiles_names);
+    auto mutable_inactive_profiles_names(inactive_profiles_names);
+    for (int i = 0; i < expected_profile_number; i++) {
+      auto actual_profile_info = actual_browser_report.profile_info_list(i);
+      std::string actual_profile_name = actual_profile_info.name();
+
+      // Verify that the profile id is set as profile path.
+      EXPECT_EQ(profile_manager_.profiles_dir()
+                    .AppendASCII(actual_profile_name)
+                    .AsUTF8Unsafe(),
+                actual_profile_info.id());
+
+      EXPECT_TRUE(actual_profile_info.has_is_full_report());
+
+      // Activate profiles have full report while the inactive ones don't.
+      if (actual_profile_info.is_full_report())
+        FindAndRemoveProfileName(&mutable_active_profiles_names,
+                                 actual_profile_name);
+      else
+        FindAndRemoveProfileName(&mutable_inactive_profiles_names,
+                                 actual_profile_name);
+    }
+  }
+
   TestingProfileManager* profile_manager() { return &profile_manager_; }
+  ReportGenerator* generator() { return &generator_; }
 
  private:
+  ReportGenerator generator_;
+
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfileManager profile_manager_;
 
@@ -81,11 +151,9 @@ class ReportGeneratorTest : public ::testing::Test {
 };
 
 TEST_F(ReportGeneratorTest, GenerateBasicReport) {
-  int profile_number = 2;
-  auto profile_names = CreateProfiles(profile_number, /*is_active=*/false);
+  auto profile_names = CreateProfiles(/*number*/ 2, /*is_active=*/false);
 
-  ReportGenerator generator;
-  auto requests = generator.Generate();
+  auto requests = GenerateRequests();
   EXPECT_EQ(1u, requests.size());
 
   auto* basic_request = requests[0].get();
@@ -105,27 +173,93 @@ TEST_F(ReportGeneratorTest, GenerateBasicReport) {
   EXPECT_NE(std::string(), browser_report.executable_path());
   EXPECT_TRUE(browser_report.has_channel());
 
-  EXPECT_EQ(profile_number, browser_report.profile_info_list_size());
-  for (int i = 0; i < profile_number; i++) {
-    auto profile_info = browser_report.profile_info_list(i);
-    std::string profile_name = profile_info.name();
-
-    // Verify that the profile id is set as profile path.
-    EXPECT_EQ(profile_manager()
-                  ->profiles_dir()
-                  .AppendASCII(profile_name)
-                  .AsUTF8Unsafe(),
-              profile_info.id());
-
-    // Verify that the basic report does not contain any full Profile report.
-    EXPECT_FALSE(profile_info.is_full_report());
-
-    // Verify the profile name is one of the profiles that were created above.
-    auto it = profile_names.find(profile_name);
-    EXPECT_NE(profile_names.end(), it);
-    profile_names.erase(it);
-  }
+  VerifyProfileReport(/*active_profile_names*/ std::set<std::string>(),
+                      profile_names, browser_report);
 }
+
+TEST_F(ReportGeneratorTest, GenerateActiveProfiles) {
+  auto inactive_profiles_names =
+      CreateProfiles(/*number*/ 2, /*is_active=*/false);
+  auto active_profiles_names =
+      CreateProfiles(/*number*/ 2, /*is_active=*/true, /*start_index*/ 2);
+
+  auto requests = GenerateRequests();
+  EXPECT_EQ(1u, requests.size());
+
+  VerifyProfileReport(active_profiles_names, inactive_profiles_names,
+                      requests[0]->browser_report());
+}
+
+TEST_F(ReportGeneratorTest, BasicReportIsTooBig) {
+  CreateProfiles(/*number*/ 2, /*is_active=*/false);
+
+  // Set a super small limitation.
+  generator()->SetMaximumReportSizeForTesting(5);
+
+  // Because the limitation is so small, no request can be created.
+  auto requests = GenerateRequests();
+  EXPECT_EQ(0u, requests.size());
+}
+
+TEST_F(ReportGeneratorTest, ReportSeparation) {
+  auto profile_names = CreateProfiles(/*number*/ 2, /*is_active=*/true);
+
+  // Set the limitation just below the size of the report so that it needs to be
+  // separated into two requests later.
+  auto requests = GenerateRequests();
+  EXPECT_EQ(1u, requests.size());
+  generator()->SetMaximumReportSizeForTesting(requests[0]->ByteSizeLong() - 30);
+
+  std::set<std::string> first_request_profiles, second_request_profiles;
+  first_request_profiles.insert(
+      requests[0]->browser_report().profile_info_list(0).name());
+  second_request_profiles.insert(
+      requests[0]->browser_report().profile_info_list(1).name());
+
+  requests = GenerateRequests();
+
+  // The first profile is activated in the first request only while the second
+  // profile is activated in the second request.
+  EXPECT_EQ(2u, requests.size());
+  VerifyProfileReport(first_request_profiles, second_request_profiles,
+                      requests[0]->browser_report());
+  VerifyProfileReport(second_request_profiles, first_request_profiles,
+                      requests[1]->browser_report());
+}
+
+TEST_F(ReportGeneratorTest, ProfileReportIsTooBig) {
+  TestingProfile* first_profile =
+      profile_manager()->CreateTestingProfile(kProfile);
+  std::set<std::string> first_profile_name = {kProfile};
+
+  // Add more things into the Profile to make the report bigger.
+  extensions::ExtensionRegistry* extension_registry =
+      extensions::ExtensionRegistry::Get(first_profile);
+
+  std::string extension_name =
+      "a super super super super super super super super super super super "
+      "super super super super super super long extension name";
+  extension_registry->AddEnabled(extensions::ExtensionBuilder(extension_name)
+                                     .SetID("abcdefghijklmnoabcdefghijklmnoab")
+                                     .Build());
+
+  // Set the limitation just below the size of the report.
+  auto requests = GenerateRequests();
+  EXPECT_EQ(1u, requests.size());
+  generator()->SetMaximumReportSizeForTesting(requests[0]->ByteSizeLong() - 30);
+
+  // Add a smaller Profile.
+  auto second_profile_name = CreateProfiles(/*number*/ 1, /*is_active=*/true);
+
+  requests = GenerateRequests();
+
+  EXPECT_EQ(1u, requests.size());
+  // Only the second Profile is activated while the first one is too big to be
+  // reported.
+  VerifyProfileReport(second_profile_name, first_profile_name,
+                      requests[0]->browser_report());
+}
+
 #endif
 
 }  // namespace enterprise_reporting