Clean up ExternalCacheDelegate

This CL:
* uses ExtensionId as type instead of std::string
* adds empty default implementations to ExternalCacheDelegate since
  most delegates don't care about all On*() methods.

Bug: none
Change-Id: I37cce73d60708305b8746e20803216da21eefa9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366744
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801323}

chrome/browser/chromeos/BUILD.gn

@@ -1103,6 +1103,7 @@ source_set("chromeos") {
     "extensions/extensions_permissions_tracker.h",
     "extensions/external_cache.cc",
     "extensions/external_cache.h",
+    "extensions/external_cache_delegate.cc",
     "extensions/external_cache_delegate.h",
     "extensions/external_cache_impl.cc",
     "extensions/external_cache_impl.h",

chrome/browser/chromeos/app_mode/kiosk_app_manager.cc

@@ -809,11 +809,8 @@ void KioskAppManager::UpdateExternalCachePrefs() {
   external_cache_->UpdateExtensionsList(std::move(prefs));
 }
 
-void KioskAppManager::OnExtensionListsUpdated(
-    const base::DictionaryValue* prefs) {
-}
-
-void KioskAppManager::OnExtensionLoadedInCache(const std::string& id) {
+void KioskAppManager::OnExtensionLoadedInCache(
+    const extensions::ExtensionId& id) {
   KioskAppData* app_data = GetAppDataMutable(id);
   if (!app_data)
     return;
@@ -827,7 +824,8 @@ void KioskAppManager::OnExtensionLoadedInCache(const std::string& id) {
     observer.OnKioskExtensionLoadedInCache(id);
 }
 
-void KioskAppManager::OnExtensionDownloadFailed(const std::string& id) {
+void KioskAppManager::OnExtensionDownloadFailed(
+    const extensions::ExtensionId& id) {
   KioskAppData* app_data = GetAppDataMutable(id);
   if (!app_data)
     return;
@@ -835,11 +833,6 @@ void KioskAppManager::OnExtensionDownloadFailed(const std::string& id) {
     observer.OnKioskExtensionDownloadFailed(id);
 }
 
-std::string KioskAppManager::GetInstalledExtensionVersion(
-    const std::string& id) {
-  return std::string();
-}
-
 KioskAppManager::AutoLoginState KioskAppManager::GetAutoLoginState() const {
   PrefService* prefs = g_browser_process->local_state();
   const base::DictionaryValue* dict =

chrome/browser/chromeos/app_mode/kiosk_app_manager.h

@@ -21,6 +21,7 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chromeos/tpm/install_attributes.h"
 #include "components/account_id/account_id.h"
+#include "extensions/common/extension_id.h"
 
 class GURL;
 class PrefRegistrySimple;
@@ -278,10 +279,8 @@ class KioskAppManager : public KioskAppManagerBase,
   void UpdateExternalCachePrefs();
 
   // ExternalCacheDelegate:
-  void OnExtensionListsUpdated(const base::DictionaryValue* prefs) override;
-  void OnExtensionLoadedInCache(const std::string& id) override;
-  void OnExtensionDownloadFailed(const std::string& id) override;
-  std::string GetInstalledExtensionVersion(const std::string& id) override;
+  void OnExtensionLoadedInCache(const extensions::ExtensionId& id) override;
+  void OnExtensionDownloadFailed(const extensions::ExtensionId& id) override;
 
   // Callback for InstallAttributes::LockDevice() during
   // EnableConsumerModeKiosk() call.

chrome/browser/chromeos/extensions/device_local_account_external_policy_loader.cc

@@ -91,18 +91,6 @@ void DeviceLocalAccountExternalPolicyLoader::OnExtensionListsUpdated(
     LoadFinished(std::move(prefs_));
 }
 
-void DeviceLocalAccountExternalPolicyLoader::OnExtensionLoadedInCache(
-    const std::string& id) {}
-
-void DeviceLocalAccountExternalPolicyLoader::OnExtensionDownloadFailed(
-    const std::string& id) {}
-
-std::string
-DeviceLocalAccountExternalPolicyLoader::GetInstalledExtensionVersion(
-    const std::string& id) {
-  return std::string();
-}
-
 ExternalCache*
 DeviceLocalAccountExternalPolicyLoader::GetExternalCacheForTesting() {
   return external_cache_.get();

chrome/browser/chromeos/extensions/device_local_account_external_policy_loader.h

@@ -60,9 +60,6 @@ class DeviceLocalAccountExternalPolicyLoader
 
   // ExternalCacheDelegate:
   void OnExtensionListsUpdated(const base::DictionaryValue* prefs) override;
-  void OnExtensionLoadedInCache(const std::string& id) override;
-  void OnExtensionDownloadFailed(const std::string& id) override;
-  std::string GetInstalledExtensionVersion(const std::string& id) override;
 
   ExternalCache* GetExternalCacheForTesting();
 

chrome/browser/chromeos/extensions/external_cache_delegate.cc

+// Copyright 2020 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/chromeos/extensions/external_cache_delegate.h"
+
+namespace chromeos {
+
+void ExternalCacheDelegate::OnExtensionListsUpdated(
+    const base::DictionaryValue* prefs) {}
+
+void ExternalCacheDelegate::OnExtensionLoadedInCache(
+    const extensions::ExtensionId& id) {}
+
+void ExternalCacheDelegate::OnExtensionDownloadFailed(
+    const extensions::ExtensionId& id) {}
+
+std::string ExternalCacheDelegate::GetInstalledExtensionVersion(
+    const extensions::ExtensionId& id) {
+  return std::string();
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/extensions/external_cache_delegate.h

@@ -7,6 +7,8 @@
 
 #include <string>
 
+#include "extensions/common/extension_id.h"
+
 namespace base {
 class DictionaryValue;
 }
@@ -18,18 +20,19 @@ class ExternalCacheDelegate {
   virtual ~ExternalCacheDelegate() = default;
 
   // Caller owns |prefs|.
-  virtual void OnExtensionListsUpdated(const base::DictionaryValue* prefs) = 0;
+  virtual void OnExtensionListsUpdated(const base::DictionaryValue* prefs);
 
   // Called after extension with |id| is loaded in cache.
-  virtual void OnExtensionLoadedInCache(const std::string& id) = 0;
+  virtual void OnExtensionLoadedInCache(const extensions::ExtensionId& id);
 
   // Called when extension with |id| fails to load due to a download error.
-  virtual void OnExtensionDownloadFailed(const std::string& id) = 0;
+  virtual void OnExtensionDownloadFailed(const extensions::ExtensionId& id);
 
   // Cache needs to provide already installed extensions otherwise they
   // will be removed. Cache calls this function to get version of installed
   // extension or empty string if not installed.
-  virtual std::string GetInstalledExtensionVersion(const std::string& id) = 0;
+  virtual std::string GetInstalledExtensionVersion(
+      const extensions::ExtensionId& id);
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/extensions/external_cache_impl_unittest.cc

@@ -63,9 +63,8 @@ class ExternalCacheImplTest : public testing::Test,
   void OnExtensionListsUpdated(const base::DictionaryValue* prefs) override {
     prefs_.reset(prefs->DeepCopy());
   }
-  void OnExtensionLoadedInCache(const std::string& id) override {}
-  void OnExtensionDownloadFailed(const std::string& id) override {}
-  std::string GetInstalledExtensionVersion(const std::string& id) override {
+  std::string GetInstalledExtensionVersion(
+      const extensions::ExtensionId& id) override {
     std::map<std::string, std::string>::iterator it =
         installed_extensions_.find(id);
     return it != installed_extensions_.end() ? it->second : std::string();

chrome/browser/chromeos/extensions/signin_screen_extensions_external_loader.cc

@@ -85,17 +85,6 @@ void SigninScreenExtensionsExternalLoader::OnExtensionListsUpdated(
   LoadFinished(prefs->CreateDeepCopy());
 }
 
-void SigninScreenExtensionsExternalLoader::OnExtensionLoadedInCache(
-    const std::string& id) {}
-
-void SigninScreenExtensionsExternalLoader::OnExtensionDownloadFailed(
-    const std::string& id) {}
-
-std::string SigninScreenExtensionsExternalLoader::GetInstalledExtensionVersion(
-    const std::string& id) {
-  return std::string();
-}
-
 SigninScreenExtensionsExternalLoader::~SigninScreenExtensionsExternalLoader() =
     default;
 

chrome/browser/chromeos/extensions/signin_screen_extensions_external_loader.h

@@ -39,9 +39,6 @@ class SigninScreenExtensionsExternalLoader : public extensions::ExternalLoader,
 
   // ExternalCacheDelegate:
   void OnExtensionListsUpdated(const base::DictionaryValue* prefs) override;
-  void OnExtensionLoadedInCache(const std::string& id) override;
-  void OnExtensionDownloadFailed(const std::string& id) override;
-  std::string GetInstalledExtensionVersion(const std::string& id) override;
 
  private:
   friend class base::RefCounted<SigninScreenExtensionsExternalLoader>;

chrome/browser/chromeos/login/demo_mode/demo_extensions_external_loader.cc

@@ -121,17 +121,6 @@ void DemoExtensionsExternalLoader::OnExtensionListsUpdated(
   LoadFinished(prefs->CreateDeepCopy());
 }
 
-void DemoExtensionsExternalLoader::OnExtensionLoadedInCache(
-    const std::string& id) {}
-
-void DemoExtensionsExternalLoader::OnExtensionDownloadFailed(
-    const std::string& id) {}
-
-std::string DemoExtensionsExternalLoader::GetInstalledExtensionVersion(
-    const std::string& id) {
-  return std::string();
-}
-
 void DemoExtensionsExternalLoader::StartLoadingFromOfflineDemoResources() {
   DemoSession* demo_session = DemoSession::Get();
   DCHECK(demo_session->resources()->loaded());

chrome/browser/chromeos/login/demo_mode/demo_extensions_external_loader.h

@@ -50,9 +50,6 @@ class DemoExtensionsExternalLoader : public extensions::ExternalLoader,
 
   // ExternalCacheDelegate:
   void OnExtensionListsUpdated(const base::DictionaryValue* prefs) override;
-  void OnExtensionLoadedInCache(const std::string& id) override;
-  void OnExtensionDownloadFailed(const std::string& id) override;
-  std::string GetInstalledExtensionVersion(const std::string& id) override;
 
  protected:
   ~DemoExtensionsExternalLoader() override;