Add extension type to event-based reporting

Event-based reporting is a way to gather more feedback from managed
profiles by sending data about status of force-installed extensions.

This CL adds extension type (browser extension, chrome app, theme etc)
to the reports.

Bug: b:161368099
Change-Id: Ia463a5df1287174b62212783872cde5adb915665
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2329633
Commit-Queue: Oleg Davydov <burunduk@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795441}

chrome/browser/chromeos/policy/extension_install_event_log_collector.cc

@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/enterprise/reporting/extension_info.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/network/network_handler.h"
@@ -198,7 +199,7 @@ ExtensionInstallEventLogCollector::ExtensionInstallEventLogCollector(
   chromeos::PowerManagerClient::Get()->AddObserver(this);
   content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
   registry_observer_.Add(registry_);
-  collector_observer_.Add(extensions::InstallStageTracker::Get(profile_));
+  stage_tracker_observer_.Add(extensions::InstallStageTracker::Get(profile_));
 }
 
 ExtensionInstallEventLogCollector::~ExtensionInstallEventLogCollector() {
@@ -265,6 +266,12 @@ void ExtensionInstallEventLogCollector::OnExtensionInstallationFailed(
   event->set_event_type(
       em::ExtensionInstallReportLogEvent::INSTALLATION_FAILED);
   event->set_failure_reason(ConvertFailureReasonToProto(reason));
+  extensions::InstallStageTracker::InstallationData data =
+      extensions::InstallStageTracker::Get(profile_)->Get(extension_id);
+  if (data.extension_type) {
+    event->set_extension_type(enterprise_reporting::ConvertExtensionTypeToProto(
+        data.extension_type.value()));
+  }
   delegate_->Add(extension_id, true /* gather_disk_space_info */,
                  std::move(event));
   delegate_->OnExtensionInstallationFinished(extension_id);
@@ -295,23 +302,28 @@ void ExtensionInstallEventLogCollector::OnExtensionLoaded(
     const extensions::Extension* extension) {
   if (!delegate_->IsExtensionPending(extension->id()))
     return;
-  AddSuccessEvent(extension->id());
+  AddSuccessEvent(extension);
 }
 
 void ExtensionInstallEventLogCollector::OnExtensionsRequested(
     const extensions::ExtensionIdSet& extension_ids) {
   for (const auto& extension_id : extension_ids) {
-    if (registry_->enabled_extensions().Contains(extension_id))
-      AddSuccessEvent(extension_id);
+    const extensions::Extension* extension = registry_->GetExtensionById(
+        extension_id, extensions::ExtensionRegistry::ENABLED);
+    if (extension)
+      AddSuccessEvent(extension);
   }
 }
 
 void ExtensionInstallEventLogCollector::AddSuccessEvent(
-    const extensions::ExtensionId& id) {
+    const extensions::Extension* extension) {
   auto event = std::make_unique<em::ExtensionInstallReportLogEvent>();
   event->set_event_type(em::ExtensionInstallReportLogEvent::SUCCESS);
-  delegate_->Add(id, true /* gather_disk_space_info */, std::move(event));
-  delegate_->OnExtensionInstallationFinished(id);
+  event->set_extension_type(
+      enterprise_reporting::ConvertExtensionTypeToProto(extension->GetType()));
+  delegate_->Add(extension->id(), true /* gather_disk_space_info */,
+                 std::move(event));
+  delegate_->OnExtensionInstallationFinished(extension->id());
 }
 
 }  // namespace policy

chrome/browser/chromeos/policy/extension_install_event_log_collector.h

@@ -110,7 +110,7 @@ class ExtensionInstallEventLogCollector
 
   // Adds success events and notifies delegate that extension is loaded
   // successfully.
-  void AddSuccessEvent(const extensions::ExtensionId& id);
+  void AddSuccessEvent(const extensions::Extension* extension);
 
  private:
   extensions::ExtensionRegistry* registry_;
@@ -125,7 +125,7 @@ class ExtensionInstallEventLogCollector
       registry_observer_{this};
   ScopedObserver<extensions::InstallStageTracker,
                  extensions::InstallStageTracker::Observer>
-      collector_observer_{this};
+      stage_tracker_observer_{this};
 };
 
 }  // namespace policy

chrome/browser/chromeos/policy/extension_install_event_log_collector_unittest.cc

@@ -370,15 +370,39 @@ TEST_F(ExtensionInstallEventLogCollectorTest, ExtensionInstallFailed) {
   collector->OnExtensionInstallationFailed(
       kExtensionId1,
       extensions::InstallStageTracker::FailureReason::CRX_FETCH_URL_EMPTY);
-  ASSERT_EQ(1, delegate()->add_count());
-  ASSERT_EQ(0, delegate()->add_for_all_count());
-  EXPECT_EQ(kExtensionId1, delegate()->last_request().extension_id);
+  ASSERT_TRUE(VerifyEventAddedSuccessfully(1 /*expected_add_count*/,
+                                           0 /*expected_add_all_count*/));
   EXPECT_EQ(em::ExtensionInstallReportLogEvent::INSTALLATION_FAILED,
             delegate()->last_request().event.event_type());
   EXPECT_EQ(em::ExtensionInstallReportLogEvent::CRX_FETCH_URL_EMPTY,
             delegate()->last_request().event.failure_reason());
 }
 
+// Simulate failure after unpacking, so extension type should be reported.
+TEST_F(ExtensionInstallEventLogCollectorTest, ExtensionInstallFailedWithType) {
+  std::unique_ptr<ExtensionInstallEventLogCollector> collector =
+      std::make_unique<ExtensionInstallEventLogCollector>(
+          registry(), delegate(), profile());
+
+  extensions::InstallStageTracker* tracker =
+      extensions::InstallStageTracker::Get(profile());
+
+  // One extension failed.
+  tracker->ReportExtensionType(kExtensionId1,
+                               extensions::Manifest::TYPE_LEGACY_PACKAGED_APP);
+  tracker->ReportFailure(kExtensionId1,
+                         extensions::InstallStageTracker::FailureReason::
+                             CRX_INSTALL_ERROR_DECLINED);
+  ASSERT_TRUE(VerifyEventAddedSuccessfully(1 /*expected_add_count*/,
+                                           0 /*expected_add_all_count*/));
+  EXPECT_EQ(em::ExtensionInstallReportLogEvent::INSTALLATION_FAILED,
+            delegate()->last_request().event.event_type());
+  EXPECT_EQ(em::ExtensionInstallReportLogEvent::CRX_INSTALL_ERROR_DECLINED,
+            delegate()->last_request().event.failure_reason());
+  EXPECT_EQ(em::Extension::TYPE_LEGACY_PACKAGED_APP,
+            delegate()->last_request().event.extension_type());
+}
+
 TEST_F(ExtensionInstallEventLogCollectorTest, InstallExtension) {
   std::unique_ptr<ExtensionInstallEventLogCollector> collector =
       std::make_unique<ExtensionInstallEventLogCollector>(
@@ -393,6 +417,8 @@ TEST_F(ExtensionInstallEventLogCollectorTest, InstallExtension) {
                                            0 /*expected_add_all_count*/));
   EXPECT_EQ(em::ExtensionInstallReportLogEvent::SUCCESS,
             delegate()->last_request().event.event_type());
+  EXPECT_EQ(em::Extension::TYPE_EXTENSION,
+            delegate()->last_request().event.extension_type());
 }
 
 // Verifies that a new event is created when the installation stage is changed

chrome/browser/chromeos/policy/install_event_log_util.cc

@@ -42,6 +42,9 @@ constexpr char kAndroidAppInstallEvent[] = "androidAppInstallEvent";
 constexpr char kExtensionId[] = "extensionId";
 constexpr char kExtensionInstallEvent[] = "extensionAppInstallEvent";
 constexpr char kDownloadStage[] = "downloadStage";
+constexpr char kFailureReason[] = "failureReason";
+constexpr char kInstallationStage[] = "installationStage";
+constexpr char kExtensionType[] = "extensionType";
 
 // Calculates hash for the given |event| and |context|, and stores the hash in
 // |hash|. Returns true if |event| and |context| are json serializable and
@@ -167,6 +170,21 @@ base::Value ConvertExtensionEventToValue(
 
   event.SetStringKey(kSerialNumber, GetSerialNumber());
 
+  if (extension_install_report_log_event.has_failure_reason()) {
+    event.SetIntKey(kFailureReason,
+                    extension_install_report_log_event.failure_reason());
+  }
+
+  if (extension_install_report_log_event.has_installation_stage()) {
+    event.SetIntKey(kInstallationStage,
+                    extension_install_report_log_event.installation_stage());
+  }
+
+  if (extension_install_report_log_event.has_extension_type()) {
+    event.SetIntKey(kExtensionType,
+                    extension_install_report_log_event.extension_type());
+  }
+
   base::Value wrapper(base::Value::Type::DICTIONARY);
   wrapper.SetKey(kExtensionInstallEvent, std::move(event));
 

chrome/browser/enterprise/reporting/extension_info.cc

@@ -19,32 +19,6 @@ namespace enterprise_reporting {
 
 namespace {
 
-em::Extension_ExtensionType GetExtensionType(
-    extensions::Manifest::Type extension_type) {
-  switch (extension_type) {
-    case extensions::Manifest::TYPE_UNKNOWN:
-    case extensions::Manifest::TYPE_SHARED_MODULE:
-      return em::Extension_ExtensionType_TYPE_UNKNOWN;
-    case extensions::Manifest::TYPE_EXTENSION:
-      return em::Extension_ExtensionType_TYPE_EXTENSION;
-    case extensions::Manifest::TYPE_THEME:
-      return em::Extension_ExtensionType_TYPE_THEME;
-    case extensions::Manifest::TYPE_USER_SCRIPT:
-      return em::Extension_ExtensionType_TYPE_USER_SCRIPT;
-    case extensions::Manifest::TYPE_HOSTED_APP:
-      return em::Extension_ExtensionType_TYPE_HOSTED_APP;
-    case extensions::Manifest::TYPE_LEGACY_PACKAGED_APP:
-      return em::Extension_ExtensionType_TYPE_LEGACY_PACKAGED_APP;
-    case extensions::Manifest::TYPE_PLATFORM_APP:
-      return em::Extension_ExtensionType_TYPE_PLATFORM_APP;
-    case extensions::Manifest::TYPE_LOGIN_SCREEN_EXTENSION:
-      return em::Extension_ExtensionType_TYPE_LOGIN_SCREEN_EXTENSION;
-    case extensions::Manifest::NUM_LOAD_TYPES:
-      NOTREACHED();
-      return em::Extension_ExtensionType_TYPE_UNKNOWN;
-  }
-}
-
 em::Extension_InstallType GetExtensionInstallType(
     extensions::Manifest::Location extension_location) {
   switch (extension_location) {
@@ -99,7 +73,8 @@ void AddExtensions(const extensions::ExtensionSet& extensions,
     extension_info->set_version(extension->VersionString());
     extension_info->set_name(extension->name());
     extension_info->set_description(extension->description());
-    extension_info->set_app_type(GetExtensionType(extension->GetType()));
+    extension_info->set_app_type(
+        ConvertExtensionTypeToProto(extension->GetType()));
     extension_info->set_homepage_url(
         extensions::ManifestURL::GetHomepageURL(extension.get()).spec());
     extension_info->set_install_type(
@@ -113,6 +88,32 @@ void AddExtensions(const extensions::ExtensionSet& extensions,
 
 }  // namespace
 
+em::Extension_ExtensionType ConvertExtensionTypeToProto(
+    extensions::Manifest::Type extension_type) {
+  switch (extension_type) {
+    case extensions::Manifest::TYPE_UNKNOWN:
+    case extensions::Manifest::TYPE_SHARED_MODULE:
+      return em::Extension_ExtensionType_TYPE_UNKNOWN;
+    case extensions::Manifest::TYPE_EXTENSION:
+      return em::Extension_ExtensionType_TYPE_EXTENSION;
+    case extensions::Manifest::TYPE_THEME:
+      return em::Extension_ExtensionType_TYPE_THEME;
+    case extensions::Manifest::TYPE_USER_SCRIPT:
+      return em::Extension_ExtensionType_TYPE_USER_SCRIPT;
+    case extensions::Manifest::TYPE_HOSTED_APP:
+      return em::Extension_ExtensionType_TYPE_HOSTED_APP;
+    case extensions::Manifest::TYPE_LEGACY_PACKAGED_APP:
+      return em::Extension_ExtensionType_TYPE_LEGACY_PACKAGED_APP;
+    case extensions::Manifest::TYPE_PLATFORM_APP:
+      return em::Extension_ExtensionType_TYPE_PLATFORM_APP;
+    case extensions::Manifest::TYPE_LOGIN_SCREEN_EXTENSION:
+      return em::Extension_ExtensionType_TYPE_LOGIN_SCREEN_EXTENSION;
+    case extensions::Manifest::NUM_LOAD_TYPES:
+      NOTREACHED();
+      return em::Extension_ExtensionType_TYPE_UNKNOWN;
+  }
+}
+
 void AppendExtensionInfoIntoProfileReport(
     Profile* profile,
     em::ChromeUserProfileInfo* profile_info) {

chrome/browser/enterprise/reporting/extension_info.h

@@ -6,11 +6,15 @@
 #define CHROME_BROWSER_ENTERPRISE_REPORTING_EXTENSION_INFO_H_
 
 #include "components/policy/proto/device_management_backend.pb.h"
+#include "extensions/common/extension.h"
 
 class Profile;
 
 namespace enterprise_reporting {
 
+enterprise_management::Extension_ExtensionType ConvertExtensionTypeToProto(
+    extensions::Manifest::Type extension_type);
+
 void AppendExtensionInfoIntoProfileReport(
     Profile* profile,
     enterprise_management::ChromeUserProfileInfo* profile_info);

chrome/browser/extensions/crx_installer.cc

@@ -1055,14 +1055,14 @@ void CrxInstaller::NotifyCrxInstallComplete(
       extension_id, InstallStageTracker::Stage::COMPLETE);
   const bool success = !error.has_value();
 
+  if (extension()) {
+    install_stage_tracker->ReportExtensionType(extension_id,
+                                               extension()->GetType());
+  }
+
   if (!success && (!expected_id_.empty() || extension())) {
     switch (error->type()) {
       case CrxInstallErrorType::DECLINED:
-        if (error->detail() == CrxInstallErrorDetail::DISALLOWED_BY_POLICY) {
-          install_stage_tracker
-              ->ReportExtensionTypeForPolicyDisallowedExtension(
-                  extension_id, extension()->GetType());
-        }
         install_stage_tracker->ReportCrxInstallError(
             extension_id,
             InstallStageTracker::FailureReason::CRX_INSTALL_ERROR_DECLINED,

chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc

@@ -894,8 +894,8 @@ TEST_F(ForceInstalledMetricsTest,
       ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
   tracker_->OnExtensionLoaded(profile_, extension.get());
   // Hosted app is not a valid extension type, so this should report an error.
-  install_stage_tracker_->ReportExtensionTypeForPolicyDisallowedExtension(
-      kExtensionId2, Manifest::Type::TYPE_HOSTED_APP);
+  install_stage_tracker_->ReportExtensionType(kExtensionId2,
+                                              Manifest::Type::TYPE_HOSTED_APP);
   install_stage_tracker_->ReportCrxInstallError(
       kExtensionId2,
       InstallStageTracker::FailureReason::CRX_INSTALL_ERROR_DECLINED,
@@ -924,8 +924,8 @@ TEST_F(ForceInstalledMetricsTest,
   auto extension =
       ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
   tracker_->OnExtensionLoaded(profile_, extension.get());
-  install_stage_tracker_->ReportExtensionTypeForPolicyDisallowedExtension(
-      kExtensionId2, Manifest::Type::TYPE_EXTENSION);
+  install_stage_tracker_->ReportExtensionType(kExtensionId2,
+                                              Manifest::Type::TYPE_EXTENSION);
   install_stage_tracker_->ReportCrxInstallError(
       kExtensionId2,
       InstallStageTracker::FailureReason::CRX_INSTALL_ERROR_DECLINED,

chrome/browser/extensions/forced_extensions/install_stage_tracker.cc

@@ -247,9 +247,8 @@ void InstallStageTracker::ReportFailure(const ExtensionId& id,
   NotifyObserversOfFailure(id, reason, data);
 }
 
-void InstallStageTracker::ReportExtensionTypeForPolicyDisallowedExtension(
-    const ExtensionId& id,
-    Manifest::Type extension_type) {
+void InstallStageTracker::ReportExtensionType(const ExtensionId& id,
+                                              Manifest::Type extension_type) {
   InstallationData& data = installation_data_map_[id];
   data.extension_type = extension_type;
 }

chrome/browser/extensions/forced_extensions/install_stage_tracker.h

@@ -281,8 +281,7 @@ class InstallStageTracker : public KeyedService {
     // Unpack failure reason in case of
     // CRX_INSTALL_ERROR_SANDBOXED_UNPACKER_FAILURE.
     base::Optional<SandboxedUnpackerFailureReason> unpacker_failure_reason;
-    // Type of extension, assigned when CRX installation error detail is
-    // DISALLOWED_BY_POLICY.
+    // Type of extension, assigned during CRX installation process.
     base::Optional<Manifest::Type> extension_type;
     // Type of update check status received from server when manifest was
     // fetched.
@@ -381,11 +380,11 @@ class InstallStageTracker : public KeyedService {
       ExtensionDownloaderDelegate::CacheStatus cache_status);
   void ReportManifestUpdateCheckStatus(const ExtensionId& id,
                                        const std::string& status);
-  // Assigns the extension type. See InstallationData::extension_type for
-  // more details.
-  void ReportExtensionTypeForPolicyDisallowedExtension(
-      const ExtensionId& id,
-      Manifest::Type extension_type);
+  // Assigns the extension type. Reported from SandboxedInstalled when (and in
+  // case when) the extension type is discovered.
+  // See InstallationData::extension_type for more details.
+  void ReportExtensionType(const ExtensionId& id,
+                           Manifest::Type extension_type);
   void ReportCrxInstallError(const ExtensionId& id,
                              FailureReason reason,
                              CrxInstallErrorDetail crx_install_error);

components/policy/proto/device_management_backend.proto

@@ -2964,6 +2964,10 @@ message ExtensionInstallReportLogEvent {
 
   // Stage of downloading process.
   optional DownloadingStage downloading_stage = 9;
+
+  // Type of the extension. Set for event type SUCCESS and sometimes (when
+  // possible) for INSTALLATION_FAILED.
+  optional Extension.ExtensionType extension_type = 10;
 }
 
 // A single entry in the push-install log for an app.