Clear out navigation hosts when response comes back

Previously, we were over-counting when a race has finished but there
was no hint for some reason (either request failed or no hints for host
were returned)

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

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -529,8 +529,10 @@ void OptimizationGuideHintsManager::OnTopHostsHintsFetched(
 void OptimizationGuideHintsManager::OnPageNavigationHintsFetched(
     base::Optional<std::unique_ptr<optimization_guide::proto::GetHintsResponse>>
         get_hints_response) {
-  if (!get_hints_response.has_value() || !get_hints_response.value())
+  if (!get_hints_response.has_value() || !get_hints_response.value()) {
+    page_navigation_hosts_being_fetched_.clear();
     return;
+  }
 
   hint_cache_->UpdateFetchedHints(
       std::move(*get_hints_response), clock_->Now() + kUpdateFetchedHintsDelay,
@@ -551,8 +553,10 @@ void OptimizationGuideHintsManager::OnFetchedTopHostsHintsStored() {
 
 void OptimizationGuideHintsManager::OnFetchedPageNavigationHintsStored() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  for (const auto& host : navigation_hosts_last_fetched_real_time_)
+  for (const auto& host : page_navigation_hosts_being_fetched_)
     LoadHintForHost(host, base::DoNothing());
+
+  page_navigation_hosts_being_fetched_.clear();
 }
 
 base::Time OptimizationGuideHintsManager::GetLastHintsFetchAttemptTime() const {
@@ -649,9 +653,9 @@ void OptimizationGuideHintsManager::OnPredictionUpdated(
   if (target_hosts.empty())
     return;
 
-  navigation_hosts_last_fetched_real_time_.clear();
+  page_navigation_hosts_being_fetched_.clear();
   for (const auto& host : target_hosts)
-    navigation_hosts_last_fetched_real_time_.push_back(host);
+    page_navigation_hosts_being_fetched_.push_back(host);
 
   if (!hints_fetcher_) {
     hints_fetcher_ = std::make_unique<optimization_guide::HintsFetcher>(
@@ -934,8 +938,8 @@ void OptimizationGuideHintsManager::OnNavigationStartOrRedirect(
   if (IsAllowedToFetchNavigationHints(navigation_handle->GetURL()) &&
       !hint_cache_->HasHint(navigation_handle->GetURL().host())) {
     std::vector<std::string> hosts{navigation_handle->GetURL().host()};
-    navigation_hosts_last_fetched_real_time_.clear();
-    navigation_hosts_last_fetched_real_time_.push_back(
+    page_navigation_hosts_being_fetched_.clear();
+    page_navigation_hosts_being_fetched_.push_back(
         navigation_handle->GetURL().host());
 
     if (!hints_fetcher_) {
@@ -966,7 +970,7 @@ void OptimizationGuideHintsManager::ClearFetchedHints() {
 
 bool OptimizationGuideHintsManager::IsHintBeingFetched(
     const std::string& host) const {
-  return std::find(navigation_hosts_last_fetched_real_time_.begin(),
-                   navigation_hosts_last_fetched_real_time_.end(),
-                   host) != navigation_hosts_last_fetched_real_time_.end();
+  return std::find(page_navigation_hosts_being_fetched_.begin(),
+                   page_navigation_hosts_being_fetched_.end(),
+                   host) != page_navigation_hosts_being_fetched_.end();
 }

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -344,8 +344,10 @@ class OptimizationGuideHintsManager
   // Used in testing to subscribe to an update event in this class.
   base::OnceClosure next_update_closure_;
 
-  // Hosts for which hints were last fetched in the real-time.
-  std::vector<std::string> navigation_hosts_last_fetched_real_time_;
+  // Hosts for which hints are currently being fetched with the PAGE_NAVIGATION
+  // request context.
+  // This will be cleared at request completion.
+  std::vector<std::string> page_navigation_hosts_being_fetched_;
 
   // Used to get |weak_ptr_| to self on the UI thread.
   base::WeakPtrFactory<OptimizationGuideHintsManager> ui_weak_ptr_factory_{

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -2267,3 +2267,65 @@ TEST_F(OptimizationGuideHintsManagerFetchingTest,
             optimization_guide::OptimizationTypeDecision::
                 kHintFetchStartedButNotAvailableInTime);
 }
+
+TEST_F(OptimizationGuideHintsManagerFetchingTest,
+       CanApplyOptimizationCalledPostFetchButNoHintsCameBack) {
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::DEFER_ALL_SCRIPT});
+  InitializeWithDefaultConfig("1.0.0.0");
+
+  hints_manager()->SetHintsFetcherForTesting(
+      BuildTestHintsFetcher(HintsFetcherEndState::kFetchSuccessWithNoHints));
+
+  // Set ECT estimate so fetch is activated.
+  hints_manager()->OnEffectiveConnectionTypeChanged(
+      net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_without_hints());
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               base::DoNothing());
+  RunUntilIdle();
+
+  optimization_guide::OptimizationTargetDecision unused_target_decision;
+  optimization_guide::OptimizationTypeDecision optimization_type_decision;
+  hints_manager()->CanApplyOptimization(
+      navigation_handle.get(),
+      optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD,
+      optimization_guide::proto::DEFER_ALL_SCRIPT, &unused_target_decision,
+      &optimization_type_decision, /*optimization_metadata=*/nullptr);
+
+  EXPECT_EQ(optimization_type_decision,
+            optimization_guide::OptimizationTypeDecision::kNoHintAvailable);
+}
+
+TEST_F(OptimizationGuideHintsManagerFetchingTest,
+       CanApplyOptimizationCalledPostFetchButFetchFailed) {
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::DEFER_ALL_SCRIPT});
+  InitializeWithDefaultConfig("1.0.0.0");
+
+  hints_manager()->SetHintsFetcherForTesting(
+      BuildTestHintsFetcher(HintsFetcherEndState::kFetchFailed));
+
+  // Set ECT estimate so fetch is activated.
+  hints_manager()->OnEffectiveConnectionTypeChanged(
+      net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_without_hints());
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               base::DoNothing());
+  RunUntilIdle();
+
+  optimization_guide::OptimizationTargetDecision unused_target_decision;
+  optimization_guide::OptimizationTypeDecision optimization_type_decision;
+  hints_manager()->CanApplyOptimization(
+      navigation_handle.get(),
+      optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD,
+      optimization_guide::proto::DEFER_ALL_SCRIPT, &unused_target_decision,
+      &optimization_type_decision, /*optimization_metadata=*/nullptr);
+
+  EXPECT_EQ(optimization_type_decision,
+            optimization_guide::OptimizationTypeDecision::kNoHintAvailable);
+}