Revert "Remove pre-initialization OptimizationGuideKeyedService logic"

This reverts commit e2eec62f.

Reason for revert: Depends on crrev.com/c/2586246 which needs to be reverted, see crbug.com/1158739

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}

TBR=sophiechang@chromium.org,mcrouse@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I2710ea9316e355c8e435d185f49828a11974a0c3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593108Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837033}

chrome/browser/optimization_guide/android/optimization_guide_bridge_unittest.cc

@@ -41,7 +41,8 @@ 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,
@@ -107,7 +108,6 @@ 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,6 +255,8 @@ 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,
@@ -289,6 +291,8 @@ 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);
 
@@ -306,12 +310,9 @@ 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);
 
@@ -321,6 +322,13 @@ void OptimizationGuideHintsManager::Shutdown() {
     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,6 +68,8 @@ 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,12 +136,14 @@ 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>(
-      profile_path, proto_db_provider, top_host_provider_.get(),
-      url_loader_factory, profile->GetPrefs(), profile);
+      pre_initialized_optimization_targets_, profile_path, proto_db_provider,
+      top_host_provider_.get(), url_loader_factory, profile->GetPrefs(),
+      profile);
 }
 
 OptimizationGuideHintsManager*
@@ -156,17 +158,22 @@ void OptimizationGuideKeyedService::OnNavigationStartOrRedirect(
   OptimizationGuideNavigationData* navigation_data =
       OptimizationGuideNavigationData::GetFromNavigationHandle(
           navigation_handle);
-  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 (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());
+    }
   }
 
-  if (navigation_data) {
-    navigation_data->set_registered_optimization_types(
-        hints_manager_->registered_optimization_types());
+  if (prediction_manager_ && navigation_data) {
     navigation_data->set_registered_optimization_targets(
         prediction_manager_->registered_optimization_targets());
   }
@@ -176,13 +183,23 @@ void OptimizationGuideKeyedService::OnNavigationFinish(
     const std::vector<GURL>& navigation_redirect_chain) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  hints_manager_->OnNavigationFinish(navigation_redirect_chain);
+  if (hints_manager_)
+    hints_manager_->OnNavigationFinish(navigation_redirect_chain);
 }
 
 void OptimizationGuideKeyedService::RegisterOptimizationTargets(
     const std::vector<optimization_guide::proto::OptimizationTarget>&
         optimization_targets) {
-  prediction_manager_->RegisterOptimizationTargets(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());
+  }
 }
 
 void OptimizationGuideKeyedService::ShouldTargetNavigationAsync(
@@ -194,6 +211,13 @@ 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);
@@ -204,21 +228,48 @@ 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
@@ -228,6 +279,11 @@ 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,
@@ -271,28 +327,38 @@ void OptimizationGuideKeyedService::AddHintForTesting(
 }
 
 void OptimizationGuideKeyedService::ClearData() {
-  hints_manager_->ClearFetchedHints();
-  prediction_manager_->ClearHostModelFeatures();
+  if (hints_manager_)
+    hints_manager_->ClearFetchedHints();
+  if (prediction_manager_)
+    prediction_manager_->ClearHostModelFeatures();
 }
 
 void OptimizationGuideKeyedService::Shutdown() {
-  hints_manager_->Shutdown();
+  if (hints_manager_) {
+    hints_manager_->Shutdown();
+    hints_manager_ = nullptr;
+  }
 }
 
 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,6 +147,14 @@ 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,6 +222,8 @@ 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,
@@ -229,6 +231,7 @@ PredictionManager::PredictionManager(
     PrefService* pref_service,
     Profile* profile)
     : PredictionManager(
+          optimization_targets_at_initialization,
           std::make_unique<OptimizationGuideStore>(
               database_provider,
               profile_path.AddExtensionASCII(
@@ -242,6 +245,8 @@ 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,
@@ -264,7 +269,7 @@ PredictionManager::PredictionManager(
       clock_(base::DefaultClock::GetInstance()) {
   if (prediction_model_download_manager_)
     prediction_model_download_manager_->AddObserver(this);
-  Initialize();
+  Initialize(optimization_targets_at_initialization);
 }
 
 PredictionManager::~PredictionManager() {
@@ -274,7 +279,9 @@ PredictionManager::~PredictionManager() {
       ->RemoveEffectiveConnectionTypeObserver(this);
 }
 
-void PredictionManager::Initialize() {
+void PredictionManager::Initialize(const std::vector<proto::OptimizationTarget>&
+                                       optimization_targets_at_initialization) {
+  RegisterOptimizationTargets(optimization_targets_at_initialization);
   g_browser_process->network_quality_tracker()
       ->AddEffectiveConnectionTypeObserver(this);
   model_and_features_store_->Initialize(

chrome/browser/optimization_guide/prediction/prediction_manager.h

@@ -72,6 +72,8 @@ 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,
@@ -80,6 +82,8 @@ 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,
@@ -225,9 +229,11 @@ class PredictionManager
           prediction_models);
 
  private:
-  // 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();
+  // 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);
 
   // Construct and return a map containing the current feature values for the
   // requested set of model features. The host model features cache is updated