Reland "Remove pre-initialization OptimizationGuideKeyedService logic"

This is a reland of e2eec62f

Original change's description:
> Remove pre-initialization OptimizationGuideKeyedService logic
>
> In practice, this doesn't happen at all and cannot happen at all after
> the incognito CL goes in
>
> Change-Id: I0daf1c4cbb189840913ee4a368dda437e07efaca
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587396
> Commit-Queue: Sophie Chang <sophiechang@chromium.org>
> Reviewed-by: Michael Crouse <mcrouse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#836699}

Change-Id: I7f9cb3fe9d8f16dd5da3dbadf7b65c687d63ddb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594117Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837376}

chrome/browser/optimization_guide/android/optimization_guide_bridge_unittest.cc

@@ -41,8 +41,7 @@ class MockOptimizationGuideHintsManager : public OptimizationGuideHintsManager {
       base::FilePath file_path,
       leveldb_proto::ProtoDatabaseProvider* db_provider,
       PrefService* pref_service)
-      : OptimizationGuideHintsManager({},
-                                      optimization_guide_service,
+      : OptimizationGuideHintsManager(optimization_guide_service,
                                       profile,
                                       file_path,
                                       pref_service,
@@ -108,6 +107,7 @@ class OptimizationGuideBridgeTest : public testing::Test {
   }
 
   void TearDown() override {
+    optimization_guide_hints_manager_->Shutdown();
     optimization_guide_hints_manager_.reset();
     db_provider_.reset();
     optimization_guide_service_.reset();

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -255,8 +255,6 @@ bool ShouldIgnoreNewlyRegisteredOptimizationType(
 }  // namespace
 
 OptimizationGuideHintsManager::OptimizationGuideHintsManager(
-    const std::vector<optimization_guide::proto::OptimizationType>&
-        optimization_types_at_initialization,
     optimization_guide::OptimizationGuideService* optimization_guide_service,
     Profile* profile,
     const base::FilePath& profile_path,
@@ -291,8 +289,6 @@ OptimizationGuideHintsManager::OptimizationGuideHintsManager(
               ExternalAppPackageNamesApprovedForFetch()),
       top_host_provider_(top_host_provider),
       clock_(base::DefaultClock::GetInstance()) {
-  RegisterOptimizationTypes(optimization_types_at_initialization);
-
   g_browser_process->network_quality_tracker()
       ->AddEffectiveConnectionTypeObserver(this);
 
@@ -310,9 +306,12 @@ OptimizationGuideHintsManager::OptimizationGuideHintsManager(
 
 OptimizationGuideHintsManager::~OptimizationGuideHintsManager() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+}
 
+void OptimizationGuideHintsManager::Shutdown() {
   if (optimization_guide_service_)
     optimization_guide_service_->RemoveObserver(this);
+
   g_browser_process->network_quality_tracker()
       ->RemoveEffectiveConnectionTypeObserver(this);
 
@@ -322,13 +321,6 @@ OptimizationGuideHintsManager::~OptimizationGuideHintsManager() {
     navigation_predictor_service->RemoveObserver(this);
 }
 
-void OptimizationGuideHintsManager::Shutdown() {
-  if (optimization_guide_service_)
-    optimization_guide_service_->RemoveObserver(this);
-  g_browser_process->network_quality_tracker()
-      ->RemoveEffectiveConnectionTypeObserver(this);
-}
-
 void OptimizationGuideHintsManager::OnHintsComponentAvailable(
     const optimization_guide::HintsComponentInfo& info) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -68,8 +68,6 @@ class OptimizationGuideHintsManager
       public NavigationPredictorKeyedService::Observer {
  public:
   OptimizationGuideHintsManager(
-      const std::vector<optimization_guide::proto::OptimizationType>&
-          optimization_types_at_initialization,
       optimization_guide::OptimizationGuideService* optimization_guide_service,
       Profile* profile,
       const base::FilePath& profile_path,

chrome/browser/optimization_guide/optimization_guide_keyed_service.cc

@@ -136,14 +136,12 @@ void OptimizationGuideKeyedService::Initialize() {
   }
 
   hints_manager_ = std::make_unique<OptimizationGuideHintsManager>(
-      pre_initialized_optimization_types_,
       g_browser_process->optimization_guide_service(), profile, profile_path,
       profile->GetPrefs(), proto_db_provider, top_host_provider_.get(),
       url_loader_factory);
   prediction_manager_ = std::make_unique<optimization_guide::PredictionManager>(
-      pre_initialized_optimization_targets_, profile_path, proto_db_provider,
-      top_host_provider_.get(), url_loader_factory, profile->GetPrefs(),
-      profile);
+      profile_path, proto_db_provider, top_host_provider_.get(),
+      url_loader_factory, profile->GetPrefs(), profile);
 }
 
 OptimizationGuideHintsManager*
@@ -158,22 +156,17 @@ void OptimizationGuideKeyedService::OnNavigationStartOrRedirect(
   OptimizationGuideNavigationData* navigation_data =
       OptimizationGuideNavigationData::GetFromNavigationHandle(
           navigation_handle);
-  if (hints_manager_) {
-    base::flat_set<optimization_guide::proto::OptimizationType>
-        registered_optimization_types =
-            hints_manager_->registered_optimization_types();
-
-    if (!registered_optimization_types.empty()) {
-      hints_manager_->OnNavigationStartOrRedirect(navigation_handle,
-                                                  base::DoNothing());
-    }
-    if (navigation_data) {
-      navigation_data->set_registered_optimization_types(
-          hints_manager_->registered_optimization_types());
-    }
+  base::flat_set<optimization_guide::proto::OptimizationType>
+      registered_optimization_types =
+          hints_manager_->registered_optimization_types();
+  if (!registered_optimization_types.empty()) {
+    hints_manager_->OnNavigationStartOrRedirect(navigation_handle,
+                                                base::DoNothing());
   }
 
-  if (prediction_manager_ && navigation_data) {
+  if (navigation_data) {
+    navigation_data->set_registered_optimization_types(
+        hints_manager_->registered_optimization_types());
     navigation_data->set_registered_optimization_targets(
         prediction_manager_->registered_optimization_targets());
   }
@@ -183,23 +176,13 @@ void OptimizationGuideKeyedService::OnNavigationFinish(
     const std::vector<GURL>& navigation_redirect_chain) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (hints_manager_)
-    hints_manager_->OnNavigationFinish(navigation_redirect_chain);
+  hints_manager_->OnNavigationFinish(navigation_redirect_chain);
 }
 
 void OptimizationGuideKeyedService::RegisterOptimizationTargets(
     const std::vector<optimization_guide::proto::OptimizationTarget>&
         optimization_targets) {
-  if (prediction_manager_) {
-    prediction_manager_->RegisterOptimizationTargets(optimization_targets);
-  } else {
-    // If the service has not been initialized yet, keep track of the
-    // optimization targets that are registered, so that we can pass them to the
-    // prediction manager at initialization.
-    pre_initialized_optimization_targets_.insert(
-        pre_initialized_optimization_targets_.begin(),
-        optimization_targets.begin(), optimization_targets.end());
-  }
+  prediction_manager_->RegisterOptimizationTargets(optimization_targets);
 }
 
 void OptimizationGuideKeyedService::ShouldTargetNavigationAsync(
@@ -211,13 +194,6 @@ void OptimizationGuideKeyedService::ShouldTargetNavigationAsync(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(navigation_handle->IsInMainFrame());
 
-  if (!prediction_manager_) {
-    // We are not initialized yet, so just return unknown.
-    std::move(callback).Run(
-        optimization_guide::OptimizationGuideDecision::kUnknown);
-    return;
-  }
-
   optimization_guide::OptimizationTargetDecision target_decision =
       prediction_manager_->ShouldTargetNavigation(
           navigation_handle, optimization_target, client_model_feature_values);
@@ -228,48 +204,21 @@ void OptimizationGuideKeyedService::ShouldTargetNavigationAsync(
 void OptimizationGuideKeyedService::AddObserverForOptimizationTargetModel(
     optimization_guide::proto::OptimizationTarget optimization_target,
     optimization_guide::OptimizationTargetModelObserver* observer) {
-  // Add this as a DCHECK since in reality, this keyed service is registered at
-  // browser initialization before anything can call it. If this is not the
-  // right assumption, then we need to add in facilities to store the registered
-  // observers and pass it in to the prediction manager constructor at this
-  // service's initialization.
-  DCHECK(prediction_manager_);
-
-  if (prediction_manager_) {
     prediction_manager_->AddObserverForOptimizationTargetModel(
         optimization_target, observer);
-  }
 }
 
 void OptimizationGuideKeyedService::RemoveObserverForOptimizationTargetModel(
     optimization_guide::proto::OptimizationTarget optimization_target,
     optimization_guide::OptimizationTargetModelObserver* observer) {
-  // Add this as a DCHECK since in reality, this keyed service is registered at
-  // browser initialization before anything can call it. If this is not the
-  // right assumption, then we need to add in facilities to store the registered
-  // observers and pass it in to the prediction manager constructor at this
-  // service's initialization.
-  DCHECK(prediction_manager_);
-
-  if (prediction_manager_) {
     prediction_manager_->RemoveObserverForOptimizationTargetModel(
         optimization_target, observer);
-  }
 }
 
 void OptimizationGuideKeyedService::RegisterOptimizationTypes(
     const std::vector<optimization_guide::proto::OptimizationType>&
         optimization_types) {
-  if (hints_manager_) {
     hints_manager_->RegisterOptimizationTypes(optimization_types);
-  } else {
-    // If the service has not been initialized yet, keep track of the
-    // optimization types that are registered, so that we can pass them to the
-    // hints manager at initialization.
-    pre_initialized_optimization_types_.insert(
-        pre_initialized_optimization_types_.begin(), optimization_types.begin(),
-        optimization_types.end());
-  }
 }
 
 optimization_guide::OptimizationGuideDecision
@@ -279,11 +228,6 @@ OptimizationGuideKeyedService::CanApplyOptimization(
     optimization_guide::OptimizationMetadata* optimization_metadata) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (!hints_manager_) {
-    // We are not initialized yet, just return unknown.
-    return optimization_guide::OptimizationGuideDecision::kUnknown;
-  }
-
   optimization_guide::OptimizationTypeDecision optimization_type_decision =
       hints_manager_->CanApplyOptimization(url, /*navigation_id=*/base::nullopt,
                                            optimization_type,
@@ -327,38 +271,28 @@ void OptimizationGuideKeyedService::AddHintForTesting(
 }
 
 void OptimizationGuideKeyedService::ClearData() {
-  if (hints_manager_)
-    hints_manager_->ClearFetchedHints();
-  if (prediction_manager_)
-    prediction_manager_->ClearHostModelFeatures();
+  hints_manager_->ClearFetchedHints();
+  prediction_manager_->ClearHostModelFeatures();
 }
 
 void OptimizationGuideKeyedService::Shutdown() {
-  if (hints_manager_) {
-    hints_manager_->Shutdown();
-    hints_manager_ = nullptr;
-  }
+  hints_manager_->Shutdown();
 }
 
 void OptimizationGuideKeyedService::UpdateSessionFCP(base::TimeDelta fcp) {
-  if (prediction_manager_)
     prediction_manager_->UpdateFCPSessionStatistics(fcp);
 }
 
 void OptimizationGuideKeyedService::OverrideTargetDecisionForTesting(
     optimization_guide::proto::OptimizationTarget optimization_target,
     optimization_guide::OptimizationGuideDecision optimization_guide_decision) {
-  if (prediction_manager_) {
     prediction_manager_->OverrideTargetDecisionForTesting(
         optimization_target, optimization_guide_decision);
-  }
 }
 
 void OptimizationGuideKeyedService::OverrideTargetModelFileForTesting(
     optimization_guide::proto::OptimizationTarget optimization_target,
     const base::FilePath& file_path) {
-  if (prediction_manager_) {
     prediction_manager_->OverrideTargetModelFileForTesting(optimization_target,
                                                            file_path);
-  }
 }

chrome/browser/optimization_guide/optimization_guide_keyed_service.h

@@ -147,14 +147,6 @@ class OptimizationGuideKeyedService
 
   content::BrowserContext* browser_context_;
 
-  // The optimization types registered prior to initialization.
-  std::vector<optimization_guide::proto::OptimizationType>
-      pre_initialized_optimization_types_;
-
-  // The optimization targets registered prior to initialization.
-  std::vector<optimization_guide::proto::OptimizationTarget>
-      pre_initialized_optimization_targets_;
-
   // Manages the storing, loading, and fetching of hints.
   std::unique_ptr<OptimizationGuideHintsManager> hints_manager_;
 

chrome/browser/optimization_guide/prediction/prediction_manager.cc

@@ -222,8 +222,6 @@ struct PredictionDecisionParams {
 };
 
 PredictionManager::PredictionManager(
-    const std::vector<optimization_guide::proto::OptimizationTarget>&
-        optimization_targets_at_initialization,
     const base::FilePath& profile_path,
     leveldb_proto::ProtoDatabaseProvider* database_provider,
     TopHostProvider* top_host_provider,
@@ -231,7 +229,6 @@ PredictionManager::PredictionManager(
     PrefService* pref_service,
     Profile* profile)
     : PredictionManager(
-          optimization_targets_at_initialization,
           std::make_unique<OptimizationGuideStore>(
               database_provider,
               profile_path.AddExtensionASCII(
@@ -245,8 +242,6 @@ PredictionManager::PredictionManager(
           profile) {}
 
 PredictionManager::PredictionManager(
-    const std::vector<optimization_guide::proto::OptimizationTarget>&
-        optimization_targets_at_initialization,
     std::unique_ptr<OptimizationGuideStore> model_and_features_store,
     TopHostProvider* top_host_provider,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
@@ -269,7 +264,7 @@ PredictionManager::PredictionManager(
       clock_(base::DefaultClock::GetInstance()) {
   if (prediction_model_download_manager_)
     prediction_model_download_manager_->AddObserver(this);
-  Initialize(optimization_targets_at_initialization);
+  Initialize();
 }
 
 PredictionManager::~PredictionManager() {
@@ -279,9 +274,7 @@ PredictionManager::~PredictionManager() {
       ->RemoveEffectiveConnectionTypeObserver(this);
 }
 
-void PredictionManager::Initialize(const std::vector<proto::OptimizationTarget>&
-                                       optimization_targets_at_initialization) {
-  RegisterOptimizationTargets(optimization_targets_at_initialization);
+void PredictionManager::Initialize() {
   g_browser_process->network_quality_tracker()
       ->AddEffectiveConnectionTypeObserver(this);
   model_and_features_store_->Initialize(

chrome/browser/optimization_guide/prediction/prediction_manager.h

@@ -72,8 +72,6 @@ class PredictionManager
       public PredictionModelDownloadObserver {
  public:
   PredictionManager(
-      const std::vector<optimization_guide::proto::OptimizationTarget>&
-          optimization_targets_at_initialization,
       const base::FilePath& profile_path,
       leveldb_proto::ProtoDatabaseProvider* database_provider,
       TopHostProvider* top_host_provider,
@@ -82,8 +80,6 @@ class PredictionManager
       Profile* profile);
 
   PredictionManager(
-      const std::vector<optimization_guide::proto::OptimizationTarget>&
-          optimization_targets_at_initialization,
       std::unique_ptr<OptimizationGuideStore> model_and_features_store,
       TopHostProvider* top_host_provider,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
@@ -229,11 +225,9 @@ class PredictionManager
           prediction_models);
 
  private:
-  // Called on construction to register optimization targets, initialize the
-  // prediction model and host model features store, and register as an observer
-  // to the network quality tracker.
-  void Initialize(const std::vector<proto::OptimizationTarget>&
-                      optimization_targets_at_intialization);
+  // Called on construction to initialize the prediction model and host model
+  // features store, and register as an observer to the network quality tracker.
+  void Initialize();
 
   // Construct and return a map containing the current feature values for the
   // requested set of model features. The host model features cache is updated