Return earlier if we have all information available to make decision

Previously, if the decision was false and we had fetched already or
couldn't fetch, we would wait until DidFinishNavigation to invoke any
registered callbacks.

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

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -916,9 +916,9 @@ void OptimizationGuideHintsManager::CanApplyOptimizationAsync(
       GetOptimizationGuideDecisionFromOptimizationTypeDecision(type_decision);
   // It's possible that a hint that applies to |navigation_url| will come in
   // later, so only run the callback if we are sure we can apply the decision.
-  // Also return early if the decision came from an optimization filter.
   if (decision == optimization_guide::OptimizationGuideDecision::kTrue ||
-      HasLoadedOptimizationFilter(optimization_type)) {
+      HasAllInformationForDecisionAvailable(navigation_url,
+                                            optimization_type)) {
     base::UmaHistogramEnumeration(
         "OptimizationGuide.ApplyDecisionAsync." +
             optimization_guide::GetStringNameForOptimizationType(
@@ -1220,6 +1220,45 @@ void OptimizationGuideHintsManager::OnNavigationFinish(
   }
 }
 
+bool OptimizationGuideHintsManager::HasAllInformationForDecisionAvailable(
+    const GURL& navigation_url,
+    optimization_guide::proto::OptimizationType optimization_type) {
+  if (HasLoadedOptimizationFilter(optimization_type)) {
+    // If we have an optimization filter for the optimization type, it is
+    // consulted instead of any hints that may be available.
+    return true;
+  }
+
+  bool has_host_keyed_hint = hint_cache_->HasHint(navigation_url.host());
+  const auto* host_keyed_hint =
+      hint_cache_->GetHostKeyedHintIfLoaded(navigation_url.host());
+  if (has_host_keyed_hint && host_keyed_hint == nullptr) {
+    // If we have a host-keyed hint in the cache and it is not loaded, we do not
+    // have all information available, regardless of whether we can fetch hints
+    // or not.
+    return false;
+  }
+
+  if (!IsAllowedToFetchNavigationHints(navigation_url)) {
+    // If we are not allowed to fetch hints for the navigation, we have all
+    // information available if the host-keyed hint we have has been loaded
+    // already or we don't have a hint available.
+    return host_keyed_hint != nullptr || !has_host_keyed_hint;
+  }
+
+  if (IsHintBeingFetchedForNavigation(navigation_url)) {
+    // If a hint is being fetched for the navigation, then we do not have all
+    // information available yet.
+    return false;
+  }
+
+  // If we are allowed to fetch hints for the navigation, we only have all
+  // information available for certain if we have attempted to get the URL-keyed
+  // hint and if the host-keyed hint is loaded.
+  return hint_cache_->HasURLKeyedEntryForURL(navigation_url) &&
+         host_keyed_hint != nullptr;
+}
+
 void OptimizationGuideHintsManager::ClearFetchedHints() {
   hint_cache_->ClearFetchedHints();
   optimization_guide::HintsFetcher::ClearHostsSuccessfullyFetched(

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -329,6 +329,12 @@ class OptimizationGuideHintsManager
   // Invokes the registered callbacks for |navigation_url|, if applicable.
   void OnReadyToInvokeRegisteredCallbacks(const GURL& navigation_url);
 
+  // Whether all information was available to make a decision for
+  // |navigation_url| and |optimization type}.
+  bool HasAllInformationForDecisionAvailable(
+      const GURL& navigation_url,
+      optimization_guide::proto::OptimizationType optimization_type);
+
   // The OptimizationGuideService that this guide is listening to. Not owned.
   optimization_guide::OptimizationGuideService* const
       optimization_guide_service_;

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -1873,6 +1873,65 @@ TEST_F(OptimizationGuideHintsManagerFetchingDisabledTest,
   EXPECT_FALSE(batch_update_hints_fetcher());
 }
 
+TEST_F(OptimizationGuideHintsManagerTest,
+       CanApplyOptimizationAsyncReturnsRightAwayIfNotAllowedToFetch) {
+  base::HistogramTester histogram_tester;
+
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::COMPRESS_PUBLIC_IMAGES});
+  InitializeWithDefaultConfig("1.0.0.0");
+
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_without_hints());
+  hints_manager()->CanApplyOptimizationAsync(
+      url_without_hints(), optimization_guide::proto::COMPRESS_PUBLIC_IMAGES,
+      base::BindOnce(
+          [](optimization_guide::OptimizationGuideDecision decision,
+             const optimization_guide::OptimizationMetadata& metadata) {
+            EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
+                      decision);
+          }));
+  RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.ApplyDecisionAsync.CompressPublicImages",
+      optimization_guide::OptimizationTypeDecision::kNoHintAvailable, 1);
+}
+
+TEST_F(
+    OptimizationGuideHintsManagerTest,
+    CanApplyOptimizationAsyncReturnsRightAwayIfNotAllowedToFetchAndNotWhitelistedByAvailableHint) {
+  base::HistogramTester histogram_tester;
+
+  InitializeWithDefaultConfig("1.0.0.0");
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::COMPRESS_PUBLIC_IMAGES});
+
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_with_hints());
+  // Wait for hint to be loaded.
+  base::RunLoop run_loop;
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               run_loop.QuitClosure());
+  run_loop.Run();
+
+  hints_manager()->CanApplyOptimizationAsync(
+      url_with_hints(), optimization_guide::proto::COMPRESS_PUBLIC_IMAGES,
+      base::BindOnce(
+          [](optimization_guide::OptimizationGuideDecision decision,
+             const optimization_guide::OptimizationMetadata& metadata) {
+            EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
+                      decision);
+          }));
+  RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.ApplyDecisionAsync.CompressPublicImages",
+      optimization_guide::OptimizationTypeDecision::kNotAllowedByHint, 1);
+}
+
 class OptimizationGuideHintsManagerFetchingTest
     : public OptimizationGuideHintsManagerTest {
  public:
@@ -3087,6 +3146,46 @@ TEST_F(OptimizationGuideHintsManagerFetchingTest,
       optimization_guide::OptimizationTypeDecision::kAllowedByHint, 1);
 }
 
+TEST_F(OptimizationGuideHintsManagerFetchingTest,
+       CanApplyOptimizationAsyncInfoAlreadyInPriorToCallAndNotWhitelisted) {
+  base::HistogramTester histogram_tester;
+
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      optimization_guide::switches::kDisableCheckingUserPermissionsForTesting);
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::PERFORMANCE_HINTS});
+
+  InitializeWithDefaultConfig("1.0.0.0");
+
+  // Set ECT estimate so fetch is activated.
+  hints_manager()->OnEffectiveConnectionTypeChanged(
+      net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
+
+  hints_manager()->SetHintsFetcherFactoryForTesting(
+      BuildTestHintsFetcherFactory(
+          {HintsFetcherEndState::kFetchSuccessWithURLHints}));
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_with_url_keyed_hint());
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               base::DoNothing());
+  RunUntilIdle();
+
+  hints_manager()->CanApplyOptimizationAsync(
+      url_with_url_keyed_hint(), optimization_guide::proto::PERFORMANCE_HINTS,
+      base::BindOnce(
+          [](optimization_guide::OptimizationGuideDecision decision,
+             const optimization_guide::OptimizationMetadata& metadata) {
+            EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
+                      decision);
+          }));
+  RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.ApplyDecisionAsync.PerformanceHints",
+      optimization_guide::OptimizationTypeDecision::kNotAllowedByHint, 1);
+}
+
 TEST_F(OptimizationGuideHintsManagerFetchingTest,
        CanApplyOptimizationAsyncDoesNotStrandCallbacksAtBeginningOfChain) {
   base::HistogramTester histogram_tester;