[SHv2] Add extensions result to Chromenu

This CL adds logic for the Chromenu entry of extensions results. Only
extensions that have been unpublished for a long time will be shown.

Bug: 1443466, 1498051
Change-Id: I8c5b0e2e2872deed510509395b55e3af091100ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4987551
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Side YILMAZ <sideyilmaz@chromium.org>
Commit-Queue: Tom Van Goethem <tov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1217759}

chrome/browser/ui/safety_hub/extensions_result.cc

@@ -10,10 +10,12 @@
 #include "base/ranges/algorithm.h"
 #include "base/values.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/safety_hub/safety_hub_constants.h"
 #include "chrome/browser/ui/safety_hub/safety_hub_service.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_prefs_factory.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension_set.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -94,9 +96,12 @@ SafetyHubExtensionsResult::~SafetyHubExtensionsResult() = default;
 absl::optional<std::unique_ptr<SafetyHubService::Result>>
 SafetyHubExtensionsResult::GetResult(
     const extensions::CWSInfoService* extension_info_service,
-    const extensions::ExtensionPrefs* extension_prefs,
-    const extensions::ExtensionRegistry* extension_registry,
+    Profile* profile,
     bool only_unpublished_extensions = false) {
+  extensions::ExtensionPrefs* extension_prefs =
+      extensions::ExtensionPrefsFactory::GetForBrowserContext(profile);
+  extensions::ExtensionRegistry* extension_registry =
+      extensions::ExtensionRegistry::Get(profile);
   std::set<extensions::ExtensionId> triggering_extensions;
   if (base::FeatureList::IsEnabled(extensions::kCWSInfoService)) {
     const extensions::ExtensionSet all_installed_extensions =

chrome/browser/ui/safety_hub/extensions_result.h

@@ -39,8 +39,7 @@ class SafetyHubExtensionsResult : public SafetyHubService::Result {
   // that have been unpublished for a long time should be considered.
   static absl::optional<std::unique_ptr<SafetyHubService::Result>> GetResult(
       const extensions::CWSInfoService* extension_info_service,
-      const extensions::ExtensionPrefs* extension_prefs,
-      const extensions::ExtensionRegistry* extension_registry,
+      Profile* profile,
       bool only_unpublished_extensions);
 
   // Returns the number of extensions that need review according to the result.

chrome/browser/ui/safety_hub/extensions_result_unittest.cc

@@ -7,148 +7,20 @@
 #include <memory>
 
 #include "chrome/browser/ui/safety_hub/safety_hub_service.h"
+#include "chrome/browser/ui/safety_hub/safety_hub_test_util.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/crx_file/id_util.h"
-#include "components/prefs/testing_pref_service.h"
 #include "content/public/test/browser_task_environment.h"
-#include "extensions/browser/extension_prefs.h"
-#include "extensions/browser/extension_prefs_factory.h"
-#include "extensions/browser/extension_registry.h"
-#include "extensions/common/extension_builder.h"
-#include "extensions/common/mojom/manifest.mojom-shared.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace {
-
-using extensions::mojom::ManifestLocation;
-
-const char kAllHostsPermission[] = "*://*/*";
-
-// These `cws_info` variables are used to test the various states that an
-// extension could be in. Is a trigger due to the malware violation.
-static extensions::CWSInfoService::CWSInfo cws_info_malware{
-    true,
-    false,
-    base::Time::Now(),
-    extensions::CWSInfoService::CWSViolationType::kMalware,
-    false,
-    false};
-// Is a trigger due to the policy violation.
-static extensions::CWSInfoService::CWSInfo cws_info_policy{
-    true,
-    false,
-    base::Time::Now(),
-    extensions::CWSInfoService::CWSViolationType::kPolicy,
-    false,
-    false};
-// Is a trigger due to being unpublished.
-static extensions::CWSInfoService::CWSInfo cws_info_unpublished{
-    true,
-    false,
-    base::Time::Now(),
-    extensions::CWSInfoService::CWSViolationType::kNone,
-    true,
-    false};
-// Is a trigger due to multiple triggers (malware and unpublished).
-static extensions::CWSInfoService::CWSInfo cws_info_multi{
-    true,
-    false,
-    base::Time::Now(),
-    extensions::CWSInfoService::CWSViolationType::kMalware,
-    true,
-    false};
-// Is not a trigger.
-static extensions::CWSInfoService::CWSInfo cws_info_no_trigger{
-    true,
-    false,
-    base::Time::Now(),
-    extensions::CWSInfoService::CWSViolationType::kNone,
-    false,
-    false};
-// Is not a trigger: malware but not present.
-static extensions::CWSInfoService::CWSInfo cws_info_no_data{
-    false,
-    false,
-    base::Time::Now(),
-    extensions::CWSInfoService::CWSViolationType::kMalware,
-    false,
-    false};
-
-class MockCWSInfoService : public extensions::CWSInfoService {
- public:
-  MOCK_METHOD(absl::optional<CWSInfoServiceInterface::CWSInfo>,
-              GetCWSInfo,
-              (const extensions::Extension&),
-              (const, override));
-};
-
-}  // namespace
-
 class SafetyHubExtensionsResultTest : public testing::Test {
  protected:
-  void AddExtension(const std::string& name,
-                    extensions::mojom::ManifestLocation location) {
-    const std::string kId = crx_file::id_util::GenerateId(name);
-    scoped_refptr<const extensions::Extension> extension =
-        CreateExtension(name, location);
-    extensions::ExtensionRegistry::Get(profile())->AddEnabled(extension);
-  }
-
-  scoped_refptr<const extensions::Extension> CreateExtension(
-      const std::string& name,
-      extensions::mojom::ManifestLocation location) {
-    const std::string kId = crx_file::id_util::GenerateId(name);
-    scoped_refptr<const extensions::Extension> extension =
-        extensions::ExtensionBuilder(name)
-            .SetManifestKey("host_permissions",
-                            base::Value::List().Append(kAllHostsPermission))
-            .SetLocation(location)
-            .SetID(kId)
-            .Build();
-    extensions::ExtensionPrefs::Get(profile())->OnExtensionInstalled(
-        extension.get(), extensions::Extension::State::ENABLED,
-        syncer::StringOrdinal(), "");
-    return extension;
-  }
-
-  void AddMockExtensions() {
-    AddExtension("TestExtension1", ManifestLocation::kInternal);
-    AddExtension("TestExtension2", ManifestLocation::kInternal);
-    AddExtension("TestExtension3", ManifestLocation::kInternal);
-    AddExtension("TestExtension4", ManifestLocation::kInternal);
-    AddExtension("TestExtension5", ManifestLocation::kInternal);
-    AddExtension("TestExtension6", ManifestLocation::kInternal);
-    // Extensions installed by policies will be ignored by the safety
-    // check. So extension 7 will not trigger the handler.
-    AddExtension("TestExtension7", ManifestLocation::kExternalPolicyDownload);
-  }
-
-  void SetMockGetCWSInfoCalls() {
-    // Ensure that the mock CWSInfo service returns the needed information.
-    EXPECT_CALL(mock_cws_info_service_, GetCWSInfo)
-        .Times(6)
-        .WillOnce(testing::Return(cws_info_malware))
-        .WillOnce(testing::Return(cws_info_policy))
-        .WillOnce(testing::Return(cws_info_unpublished))
-        .WillOnce(testing::Return(cws_info_multi))
-        .WillOnce(testing::Return(cws_info_no_data))
-        .WillOnce(testing::Return(cws_info_no_trigger));
-  }
-
   TestingProfile* profile() { return &profile_; }
-  extensions::ExtensionPrefs* extension_prefs() {
-    return extensions::ExtensionPrefsFactory::GetForBrowserContext(profile());
-  }
-  extensions::ExtensionRegistry* extension_registry() {
-    return extensions::ExtensionRegistry::Get(profile());
-  }
-  MockCWSInfoService* cws_info_service() { return &mock_cws_info_service_; }
 
  private:
   content::BrowserTaskEnvironment task_environment_;
   TestingProfile profile_;
-  testing::NiceMock<MockCWSInfoService> mock_cws_info_service_;
 };
 
 TEST_F(SafetyHubExtensionsResultTest, ResultToFromDictAndClone) {
@@ -180,21 +52,22 @@ TEST_F(SafetyHubExtensionsResultTest, ResultToFromDictAndClone) {
 TEST_F(SafetyHubExtensionsResultTest, GetResult) {
   // Create mock extensions, of which four are a trigger for review (malware,
   // policy violation, unpublished, and a combination of malware + unpublished).
-  AddMockExtensions();
-  SetMockGetCWSInfoCalls();
+  safety_hub_test_util::CreateMockExtensions(profile());
+  std::unique_ptr<testing::NiceMock<safety_hub_test_util::MockCWSInfoService>>
+      cws_info_service = safety_hub_test_util::GetMockCWSInfoService(profile());
   absl::optional<std::unique_ptr<SafetyHubService::Result>> sh_result =
-      SafetyHubExtensionsResult::GetResult(
-          cws_info_service(), extension_prefs(), extension_registry(), false);
+      SafetyHubExtensionsResult::GetResult(cws_info_service.get(), profile(),
+                                           false);
   ASSERT_TRUE(sh_result.has_value());
   auto* result = static_cast<SafetyHubExtensionsResult*>(sh_result->get());
   EXPECT_EQ(4U, result->GetNumTriggeringExtensions());
 
   // Reset the same mock calls, of which two are unpublished extensions
   // (including one where this is combined with malware).
-  SetMockGetCWSInfoCalls();
+  cws_info_service = safety_hub_test_util::GetMockCWSInfoService(profile());
   absl::optional<std::unique_ptr<SafetyHubService::Result>> sh_menu_result =
-      SafetyHubExtensionsResult::GetResult(
-          cws_info_service(), extension_prefs(), extension_registry(), true);
+      SafetyHubExtensionsResult::GetResult(cws_info_service.get(), profile(),
+                                           true);
   ASSERT_TRUE(sh_menu_result.has_value());
   auto* menu_result =
       static_cast<SafetyHubExtensionsResult*>(sh_menu_result->get());

chrome/browser/ui/safety_hub/menu_notification.cc

@@ -9,6 +9,7 @@
 
 #include "base/json/values_util.h"
 #include "base/time/time.h"
+#include "chrome/browser/ui/safety_hub/extensions_result.h"
 #include "chrome/browser/ui/safety_hub/notification_permission_review_service.h"
 #include "chrome/browser/ui/safety_hub/safe_browsing_result.h"
 #include "chrome/browser/ui/safety_hub/safety_hub_constants.h"
@@ -212,6 +213,8 @@ SafetyHubMenuNotification::GetResultFromDict(
           dict);
     case safety_hub::SafetyHubModuleType::SAFE_BROWSING:
       return std::make_unique<SafetyHubSafeBrowsingResult>(dict);
+    case safety_hub::SafetyHubModuleType::EXTENSIONS:
+      return std::make_unique<SafetyHubExtensionsResult>(dict);
   }
 }
 

chrome/browser/ui/safety_hub/menu_notification_service.cc

@@ -11,6 +11,7 @@
 #include "base/functional/callback_forward.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "chrome/browser/ui/safety_hub/extensions_result.h"
 #include "chrome/browser/ui/safety_hub/menu_notification.h"
 #include "chrome/browser/ui/safety_hub/notification_permission_review_service.h"
 #include "chrome/browser/ui/safety_hub/safe_browsing_result.h"
@@ -41,7 +42,9 @@ SafetyHubModuleInfoElement::SafetyHubModuleInfoElement(
 SafetyHubMenuNotificationService::SafetyHubMenuNotificationService(
     PrefService* pref_service,
     UnusedSitePermissionsService* unused_site_permissions_service,
-    NotificationPermissionsReviewService* notification_permissions_service) {
+    NotificationPermissionsReviewService* notification_permissions_service,
+    extensions::CWSInfoService* extension_info_service,
+    Profile* profile) {
   pref_service_ = std::move(pref_service);
   const base::Value::Dict& stored_notifications =
       pref_service_->GetDict(safety_hub_prefs::kMenuNotificationsPrefsKey);
@@ -67,6 +70,12 @@ SafetyHubMenuNotificationService::SafetyHubMenuNotificationService(
                  base::BindRepeating(&SafetyHubSafeBrowsingResult::GetResult,
                                      base::Unretained(pref_service)),
                  stored_notifications);
+  SetInfoElement(safety_hub::SafetyHubModuleType::EXTENSIONS,
+                 MenuNotificationPriority::LOW, base::Days(10),
+                 base::BindRepeating(&SafetyHubExtensionsResult::GetResult,
+                                     base::Unretained(extension_info_service),
+                                     profile, true),
+                 stored_notifications);
   // Listen for changes to the Safe Browsing pref to accommodate the trigger
   // logic.
   registrar_.Init(pref_service);

chrome/browser/ui/safety_hub/menu_notification_service.h

@@ -10,6 +10,7 @@
 #include <memory>
 
 #include "base/time/time.h"
+#include "chrome/browser/extensions/cws_info_service.h"
 #include "chrome/browser/ui/safety_hub/menu_notification.h"
 #include "chrome/browser/ui/safety_hub/notification_permission_review_service.h"
 #include "chrome/browser/ui/safety_hub/safety_hub_constants.h"
@@ -65,7 +66,9 @@ class SafetyHubMenuNotificationService : public KeyedService {
   explicit SafetyHubMenuNotificationService(
       PrefService* pref_service,
       UnusedSitePermissionsService* unused_site_permissions_service,
-      NotificationPermissionsReviewService* notification_permissions_service);
+      NotificationPermissionsReviewService* notification_permissions_service,
+      extensions::CWSInfoService* extension_info_service,
+      Profile* profile);
   SafetyHubMenuNotificationService(const SafetyHubMenuNotificationService&) =
       delete;
   SafetyHubMenuNotificationService& operator=(
@@ -120,6 +123,7 @@ class SafetyHubMenuNotificationService : public KeyedService {
           {safety_hub::SafetyHubModuleType::NOTIFICATION_PERMISSIONS,
            "notification-permissions"},
           {safety_hub::SafetyHubModuleType::SAFE_BROWSING, "safe-browsing"},
+          {safety_hub::SafetyHubModuleType::EXTENSIONS, "extensions"},
       };
 
   // Preference service that persists the notifications.

chrome/browser/ui/safety_hub/menu_notification_service_factory.cc

@@ -13,6 +13,9 @@
 #include "chrome/browser/ui/safety_hub/safety_hub_service.h"
 #include "chrome/browser/ui/safety_hub/unused_site_permissions_service.h"
 #include "chrome/browser/ui/safety_hub/unused_site_permissions_service_factory.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_prefs_factory.h"
+#include "extensions/browser/extension_registry.h"
 
 // static
 SafetyHubMenuNotificationServiceFactory*
@@ -36,6 +39,7 @@ SafetyHubMenuNotificationServiceFactory::
               .WithRegular(ProfileSelection::kOriginalOnly)
               .Build()) {
   DependsOn(UnusedSitePermissionsServiceFactory::GetInstance());
+  DependsOn(extensions::ExtensionPrefsFactory::GetInstance());
 }
 
 SafetyHubMenuNotificationServiceFactory::
@@ -49,7 +53,9 @@ SafetyHubMenuNotificationServiceFactory::BuildServiceInstanceForBrowserContext(
       UnusedSitePermissionsServiceFactory::GetForProfile(profile);
   NotificationPermissionsReviewService* notification_permission_review_service =
       NotificationPermissionsReviewServiceFactory::GetForProfile(profile);
+  extensions::CWSInfoService* extension_info_service =
+      extensions::CWSInfoService::Get(profile);
   return std::make_unique<SafetyHubMenuNotificationService>(
       profile->GetPrefs(), unused_site_permissions_service,
-      notification_permission_review_service);
+      notification_permission_review_service, extension_info_service, profile);
 }

chrome/browser/ui/safety_hub/menu_notification_service_unittest.cc

@@ -102,6 +102,9 @@ class SafetyHubMenuNotificationServiceTest
   SafetyHubMenuNotificationService* menu_notification_service() {
     return SafetyHubMenuNotificationServiceFactory::GetForProfile(profile());
   }
+  extensions::CWSInfoService* extension_info_service() {
+    return extensions::CWSInfoService::Get(profile());
+  }
   sync_preferences::TestingPrefServiceSyncable* prefs() {
     return profile()->GetTestingPrefService();
   }
@@ -166,7 +169,8 @@ TEST_F(SafetyHubMenuNotificationServiceTest, PersistInPrefs) {
   std::unique_ptr<SafetyHubMenuNotificationService> new_service =
       std::make_unique<SafetyHubMenuNotificationService>(
           prefs(), unused_site_permissions_service(),
-          notification_permissions_service());
+          notification_permissions_service(), extension_info_service(),
+          profile());
   // Getting the in-memory notification to prevent the service from generating a
   // new one.
   SafetyHubMenuNotification* new_notification =
@@ -333,3 +337,24 @@ TEST_F(SafetyHubMenuNotificationServiceTest, SafeBrowsingTriggerLogic) {
   notification = menu_notification_service()->GetNotificationToShow();
   EXPECT_TRUE(notification.has_value());
 }
+
+TEST_F(SafetyHubMenuNotificationServiceTest, ExtensionsMenuNotification) {
+  // Create mock extensions that should result in two violations that are shown
+  // in the menu notification.
+  safety_hub_test_util::CreateMockExtensions(profile());
+  // The mock CWS info service ensures that the correct extension properties are
+  // provided.
+  std::unique_ptr<testing::NiceMock<safety_hub_test_util::MockCWSInfoService>>
+      cws_info_service = safety_hub_test_util::GetMockCWSInfoService(profile());
+  // Create a menu notification service with the mocked CWS info service.
+  std::unique_ptr<SafetyHubMenuNotificationService> mocked_service =
+      std::make_unique<SafetyHubMenuNotificationService>(
+          prefs(), unused_site_permissions_service(),
+          notification_permissions_service(), cws_info_service.get(),
+          profile());
+  absl::optional<MenuNotificationEntry> notification =
+      mocked_service->GetNotificationToShow();
+  EXPECT_TRUE(notification.has_value());
+  ExpectPluralString(IDS_SETTINGS_SAFETY_HUB_EXTENSIONS_MENU_NOTIFICATION, 2,
+                     notification->label);
+}

chrome/browser/ui/safety_hub/safety_hub_constants.h

@@ -53,6 +53,7 @@ enum class SafetyHubModuleType {
   UNUSED_SITE_PERMISSIONS,
   NOTIFICATION_PERMISSIONS,
   SAFE_BROWSING,
+  EXTENSIONS,
 };
 
 }  // namespace safety_hub