Make OptimizationGuideKeyedService initialization resilient to browser initialization ordering

Bug: 969558
Change-Id: I6320efadce91869b6fe13bee8022e581102f0b29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896038Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712394}

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -157,6 +157,8 @@ const optimization_guide::proto::PageHint* GetPageHintForNavigation(
 }  // 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,
@@ -180,6 +182,8 @@ OptimizationGuideHintsManager::OptimizationGuideHintsManager(
       clock_(base::DefaultClock::GetInstance()) {
   DCHECK(optimization_guide_service_);
 
+  RegisterOptimizationTypes(optimization_types_at_initialization);
+
   g_browser_process->network_quality_tracker()
       ->AddEffectiveConnectionTypeObserver(this);
 

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -65,6 +65,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

@@ -80,13 +80,14 @@ GetOptimizationGuideDecisionFromOptimizationTypeDecision(
         kHadOptimizationFilterButNotLoadedInTime:
     case optimization_guide::OptimizationTypeDecision::
         kHadHintButNotLoadedInTime:
+    case optimization_guide::OptimizationTypeDecision::kDeciderNotInitialized:
       return optimization_guide::OptimizationGuideDecision::kUnknown;
     default:
       return optimization_guide::OptimizationGuideDecision::kFalse;
   }
   static_assert(
       optimization_guide::OptimizationTypeDecision::kMaxValue ==
-          optimization_guide::OptimizationTypeDecision::kNoHintAvailable,
+          optimization_guide::OptimizationTypeDecision::kDeciderNotInitialized,
       "This function should be updated when a new OptimizationTypeDecision is "
       "added");
 }
@@ -107,13 +108,14 @@ optimization_guide::OptimizationGuideDecision ResolveOptimizationGuideDecision(
     case optimization_guide::OptimizationTargetDecision::kPageLoadMatches:
       return GetOptimizationGuideDecisionFromOptimizationTypeDecision(
           optimization_type_decision);
+    case optimization_guide::OptimizationTargetDecision::kDeciderNotInitialized:
     default:
       return optimization_guide::OptimizationGuideDecision::kUnknown;
   }
   static_assert(
       optimization_guide::OptimizationTargetDecision::kMaxValue ==
           optimization_guide::OptimizationTargetDecision::
-              kModelPredictionHoldback,
+              kDeciderNotInitialized,
       "This function should be updated when a new OptimizationTargetDecision "
       "is added");
 }
@@ -144,13 +146,15 @@ void OptimizationGuideKeyedService::Initialize(
       content::BrowserContext::GetDefaultStoragePartition(profile)
           ->GetURLLoaderFactoryForBrowserProcess();
   hints_manager_ = std::make_unique<OptimizationGuideHintsManager>(
-      optimization_guide_service, profile, profile_path, profile->GetPrefs(),
-      database_provider, top_host_provider_.get(), url_loader_factory);
+      pre_initialized_optimization_types_, optimization_guide_service, profile,
+      profile_path, profile->GetPrefs(), database_provider,
+      top_host_provider_.get(), url_loader_factory);
   if (optimization_guide::features::IsOptimizationTargetPredictionEnabled() &&
       optimization_guide::features::IsHintsFetchingEnabled()) {
     prediction_manager_ =
         std::make_unique<optimization_guide::PredictionManager>(
-            top_host_provider_.get(), url_loader_factory);
+            pre_initialized_optimization_targets_, top_host_provider_.get(),
+            url_loader_factory);
   }
 }
 
@@ -169,7 +173,20 @@ void OptimizationGuideKeyedService::RegisterOptimizationTypesAndTargets(
         optimization_types,
     const std::vector<optimization_guide::proto::OptimizationTarget>&
         optimization_targets) {
-  DCHECK(hints_manager_);
+  // If the service has not been initialized yet, keep track of the optimization
+  // types and targets that are registered, so that we can pass them to the
+  // appropriate managers at initialization.
+  if (!hints_manager_) {
+    pre_initialized_optimization_types_.insert(
+        pre_initialized_optimization_types_.begin(), optimization_types.begin(),
+        optimization_types.end());
+
+    pre_initialized_optimization_targets_.insert(
+        pre_initialized_optimization_targets_.begin(),
+        optimization_targets.begin(), optimization_targets.end());
+    return;
+  }
+
   hints_manager_->RegisterOptimizationTypes(optimization_types);
 
   if (prediction_manager_)
@@ -182,7 +199,15 @@ OptimizationGuideKeyedService::CanApplyOptimization(
     optimization_guide::proto::OptimizationTarget optimization_target,
     optimization_guide::proto::OptimizationType optimization_type,
     optimization_guide::OptimizationMetadata* optimization_metadata) {
-  DCHECK(hints_manager_);
+  if (!hints_manager_) {
+    // We are not initialized yet, so return unknown.
+    LogDecisions(
+        navigation_handle, optimization_target,
+        optimization_guide::OptimizationTargetDecision::kDeciderNotInitialized,
+        optimization_type,
+        optimization_guide::OptimizationTypeDecision::kDeciderNotInitialized);
+    return optimization_guide::OptimizationGuideDecision::kUnknown;
+  }
 
   optimization_guide::OptimizationTargetDecision optimization_target_decision;
   optimization_guide::OptimizationTypeDecision optimization_type_decision;

chrome/browser/optimization_guide/optimization_guide_keyed_service.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -92,6 +93,14 @@ class OptimizationGuideKeyedService
  private:
   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

@@ -26,11 +26,14 @@
 namespace optimization_guide {
 
 PredictionManager::PredictionManager(
+    const std::vector<optimization_guide::proto::OptimizationTarget>&
+        optimization_targets_at_initialization,
     TopHostProvider* top_host_provider,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
     : session_fcp_(),
       top_host_provider_(top_host_provider),
       url_loader_factory_(url_loader_factory) {
+  RegisterOptimizationTargets(optimization_targets_at_initialization);
   g_browser_process->network_quality_tracker()
       ->AddEffectiveConnectionTypeObserver(this);
 }

chrome/browser/optimization_guide/prediction/prediction_manager.h

@@ -41,6 +41,8 @@ class PredictionManager
     : public network::NetworkQualityTracker::EffectiveConnectionTypeObserver {
  public:
   PredictionManager(
+      const std::vector<optimization_guide::proto::OptimizationTarget>&
+          optimization_targets_at_initialization,
       TopHostProvider* top_host_provider,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
   ~PredictionManager() override;

chrome/browser/optimization_guide/prediction/prediction_manager_unittest.cc

@@ -106,6 +106,8 @@ class FakeTopHostProvider : public TopHostProvider {
     return top_hosts_;
   }
 
+  int num_top_hosts_called() const { return num_top_hosts_called_; }
+
  private:
   std::vector<std::string> top_hosts_;
   int num_top_hosts_called_ = 0;
@@ -186,9 +188,13 @@ class TestPredictionModelFetcher : public PredictionModelFetcher {
 class TestPredictionManager : public PredictionManager {
  public:
   TestPredictionManager(
+      const std::vector<optimization_guide::proto::OptimizationTarget>&
+          optimization_targets_at_initialization,
       TopHostProvider* top_host_provider,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
-      : PredictionManager(top_host_provider, url_loader_factory) {}
+      : PredictionManager(optimization_targets_at_initialization,
+                          top_host_provider,
+                          url_loader_factory) {}
   ~TestPredictionManager() override = default;
 
   std::unique_ptr<PredictionModel> CreatePredictionModel(
@@ -232,7 +238,19 @@ class PredictionManagerTest
         base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
             &test_url_loader_factory_);
     prediction_manager_ = std::make_unique<TestPredictionManager>(
-        static_cast<TopHostProvider*>(top_host_provider_.get()),
+        std::vector<optimization_guide::proto::OptimizationTarget>({}),
+        top_host_provider_.get(), url_loader_factory_);
+  }
+
+  void CreatePredictionManager(
+      const std::vector<optimization_guide::proto::OptimizationTarget>&
+          optimization_targets_at_initialization) {
+    if (prediction_manager_) {
+      prediction_manager_.reset();
+    }
+
+    prediction_manager_ = std::make_unique<TestPredictionManager>(
+        optimization_targets_at_initialization, top_host_provider_.get(),
         url_loader_factory_);
   }
 
@@ -246,6 +264,10 @@ class PredictionManagerTest
 
   void TearDown() override { ChromeRenderViewHostTestHarness::TearDown(); }
 
+  FakeTopHostProvider* top_host_provider() const {
+    return top_host_provider_.get();
+  }
+
   std::unique_ptr<TestPredictionModelFetcher> BuildTestPredictionModelFetcher(
       PredictionModelFetcherEndState end_state) {
     std::unique_ptr<TestPredictionModelFetcher> prediction_model_fetcher =
@@ -268,6 +290,19 @@ class PredictionManagerTest
   DISALLOW_COPY_AND_ASSIGN(PredictionManagerTest);
 };
 
+TEST_F(PredictionManagerTest,
+       OptimizationTargetProvidedAtInitializationHasModelFetched) {
+  std::vector<optimization_guide::proto::OptimizationTarget>
+      optimization_targets_at_initialization = {
+          optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD};
+  CreatePredictionManager(optimization_targets_at_initialization);
+
+  // Given that we cannot inject a fetcher at initialization unless we create a
+  // constructor just for testing, we will just simulate this with a call to
+  // the top host provider.
+  EXPECT_EQ(1, top_host_provider()->num_top_hosts_called());
+}
+
 TEST_F(PredictionManagerTest, OptimizationTargetNotRegisteredForNavigation) {
   content::MockNavigationHandle navigation_handle;
   navigation_handle.set_url(GURL("https://foo.com"));

components/optimization_guide/optimization_guide_enums.h

@@ -38,9 +38,11 @@ enum class OptimizationTypeDecision {
   kHadHintButNotLoadedInTime,
   // No hints were available in the cache that matched the page load.
   kNoHintAvailable,
+  // The OptimizationGuideDecider was not initialized yet.
+  kDeciderNotInitialized,
 
   // Add new values above this line.
-  kMaxValue = kNoHintAvailable,
+  kMaxValue = kDeciderNotInitialized,
 };
 
 // The types of decisions that can be made for an optimization target.
@@ -59,8 +61,11 @@ enum class OptimizationTargetDecision {
   // will return |OptimizationGuideDecision::kFalse| in an attempt to not taint
   // the data for understanding the production recall of the model.
   kModelPredictionHoldback,
+  // The OptimizationGuideDecider was not initialized yet.
+  kDeciderNotInitialized,
+
   // Add new values above this line.
-  kMaxValue = kModelPredictionHoldback,
+  kMaxValue = kDeciderNotInitialized,
 };
 
 }  // namespace optimization_guide

tools/metrics/histograms/enums.xml

@@ -46455,6 +46455,9 @@ Called by update_net_trust_anchors.py.-->
     will return false in an attempt to not taint the data in order to understand
     the production recall of the model.
   </int>
+  <int value="5" label="Optimization Guide Decider not initialized">
+    The OptimizationGuideDecider was not initialized yet.
+  </int>
 </enum>
 
 <enum name="OptimizationGuideOptimizationTypeDecision">
@@ -46496,6 +46499,9 @@ Called by update_net_trust_anchors.py.-->
   <int value="8" label="No hint available for page load">
     There was no hint on the device that matched the page load.
   </int>
+  <int value="9" label="Optimization Guide Decider not initialized">
+    The OptimizationGuideDecider was not initialized yet.
+  </int>
 </enum>
 
 <enum name="OptimizationGuideProcessHintsResult">