Only consult optimization guide from loading predictor if page load is from gws

Doing it this way to get more correct metrics around coverage and to
verify against server metrics for when this reactive prefetch can trigger

TBR=jwd@chromium.org

Change-Id: Ie8dc14673ea905c5a51e6fc130857e5255840da7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599445
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839678}

chrome/browser/metrics/variations/variations_http_headers_browsertest.cc

@@ -848,6 +848,8 @@ class VariationsHttpHeadersBrowserTestWithOptimizationGuide
     VariationsHttpHeadersBrowserTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(
         switches::kLoadingPredictorAllowLocalRequestForTesting);
+    command_line->AppendSwitch(
+        switches::kLoadingPredictorOptimizationGuideAllowNonGwsForTesting);
   }
 
   std::unique_ptr<content::TestNavigationManager> NavigateToURLAsync(

chrome/browser/predictors/loading_predictor_browsertest.cc

@@ -1661,6 +1661,12 @@ class LoadingPredictorBrowserTestWithOptimizationGuide
     }
   }
 
+  void SetUpCommandLine(base::CommandLine* cmd) override {
+    LoadingPredictorBrowserTest::SetUpCommandLine(cmd);
+    cmd->AppendSwitch(
+        switches::kLoadingPredictorOptimizationGuideAllowNonGwsForTesting);
+  }
+
   bool IsLocalPredictionEnabled() const { return std::get<0>(GetParam()); }
 
   bool ShouldUseOptimizationGuidePredictions() const {
@@ -2023,6 +2029,8 @@ class LoadingPredictorPrefetchBrowserTest
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
+    LoadingPredictorBrowserTestWithOptimizationGuide::SetUpCommandLine(
+        command_line);
     command_line->AppendSwitch(
         switches::kLoadingPredictorAllowLocalRequestForTesting);
   }

chrome/browser/predictors/loading_predictor_tab_helper.cc

@@ -7,6 +7,7 @@
 #include <set>
 #include <string>
 
+#include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
 #include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
@@ -14,8 +15,10 @@
 #include "chrome/browser/predictors/loading_predictor_factory.h"
 #include "chrome/browser/predictors/predictors_enums.h"
 #include "chrome/browser/predictors/predictors_features.h"
+#include "chrome/browser/predictors/predictors_switches.h"
 #include "chrome/browser/prefetch/no_state_prefetch/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/google/core/common/google_util.h"
 #include "components/no_state_prefetch/browser/prerender_manager.h"
 #include "components/optimization_guide/optimization_guide_decider.h"
 #include "components/optimization_guide/proto/hints.pb.h"
@@ -129,6 +132,11 @@ class ScopedOptimizationHintsReceiveStatusRecorder {
   OptimizationHintsReceiveStatus status_;
 };
 
+bool IsFromGwsPageLoad(content::WebContents* web_contents) {
+  GURL previous_main_frame_url = web_contents->GetLastCommittedURL();
+  return google_util::IsGoogleSearchUrl(previous_main_frame_url);
+}
+
 }  // namespace
 
 LoadingPredictorTabHelper::LoadingPredictorTabHelper(
@@ -181,6 +189,13 @@ void LoadingPredictorTabHelper::DidStartNavigation(
   if (!optimization_guide_decider_)
     return;
 
+  // Only consult Optimization Guide if it is a FromGWS page load.
+  if (!IsFromGwsPageLoad(web_contents()) &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kLoadingPredictorOptimizationGuideAllowNonGwsForTesting)) {
+    return;
+  }
+
   last_optimization_guide_prediction_ = OptimizationGuidePrediction();
   last_optimization_guide_prediction_->decision =
       optimization_guide::OptimizationGuideDecision::kUnknown;

chrome/browser/predictors/loading_predictor_tab_helper_unittest.cc

@@ -59,7 +59,6 @@ MockLoadingDataCollector::MockLoadingDataCollector(
     const LoadingPredictorConfig& config)
     : LoadingDataCollector(nullptr, nullptr, config) {}
 
-// TODO(crbug/1035698): Migrate to TestOptimizationGuideDecider when provided.
 class MockOptimizationGuideKeyedService : public OptimizationGuideKeyedService {
  public:
   explicit MockOptimizationGuideKeyedService(
@@ -339,14 +338,54 @@ class LoadingPredictorTabHelperOptimizationGuideDeciderTest
         {});
   }
 
+  void NavigateToGwsInMainFrame() {
+    NavigateAndCommitInMainFrameAndVerifyMetrics(
+        "https://www.google.com/search?q=test");
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
+// Tests that document on load completed is recorded with correct navigation
+// id and that optimization guide is not consulted when not from GWS.
+TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
+       DocumentOnLoadCompletedOptimizationGuideNotFromGWS) {
+  base::HistogramTester histogram_tester;
+
+  EXPECT_CALL(
+      *mock_optimization_guide_keyed_service_,
+      CanApplyOptimizationAsync(_, optimization_guide::proto::LOADING_PREDICTOR,
+                                base::test::IsNotNullCallback()))
+      .Times(0);
+  NavigateAndCommitInMainFrameAndVerifyMetrics("http://test.org");
+  auto navigation_id =
+      CreateNavigationID(GetTabID(), "http://test.org",
+                         web_contents()->GetMainFrame()->GetPageUkmSourceId());
+
+  // Adding subframe navigation to ensure that the committed main frame url will
+  // be used.
+  auto* subframe =
+      content::RenderFrameHostTester::For(main_rfh())->AppendChild("subframe");
+  NavigateAndCommitInFrame("http://sub.test.org", subframe);
+
+  const base::Optional<OptimizationGuidePrediction>
+      null_optimization_guide_prediction;
+  EXPECT_CALL(*mock_collector_,
+              RecordMainFrameLoadComplete(navigation_id,
+                                          null_optimization_guide_prediction));
+  tab_helper_->DocumentOnLoadCompletedInMainFrame();
+
+  histogram_tester.ExpectTotalCount(
+      "LoadingPredictor.OptimizationHintsReceiveStatus", 0);
+}
+
 // Tests that document on load completed is recorded with correct navigation
 // id and optimization guide prediction.
 TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
        DocumentOnLoadCompletedOptimizationGuide) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   optimization_guide::OptimizationMetadata optimization_metadata;
@@ -398,6 +437,8 @@ TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
 // id and optimization guide prediction.
 TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
        DocumentOnLoadCompletedOptimizationGuidePredictionComesAfterCommit) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   optimization_guide::OptimizationMetadata optimization_metadata;
@@ -457,6 +498,8 @@ TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
 // has redirects.
 TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
        DocumentOnLoadCompletedOptimizationGuidePredictionArrivedAfterRedirect) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   auto navigation = content::NavigationSimulator::CreateRendererInitiated(
@@ -521,6 +564,8 @@ TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
 // id and optimization guide prediction when the prediction has not arrived.
 TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
        DocumentOnLoadCompletedOptimizationGuidePredictionHasNotArrived) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   EXPECT_CALL(
@@ -557,6 +602,8 @@ TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
 TEST_F(
     LoadingPredictorTabHelperOptimizationGuideDeciderTest,
     DocumentOnLoadCompletedOptimizationGuidePredictionComesAfterDocumentOnLoad) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   optimization_guide::OptimizationMetadata optimization_metadata;
@@ -608,6 +655,8 @@ TEST_F(
 // id and optimization guide prediction with no prediction..
 TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
        DocumentOnLoadCompletedOptimizationGuidePredictionArrivedNoPrediction) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   // The problem here is that mock_collector_ is a strict mock, which expects
@@ -652,6 +701,8 @@ TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderTest,
 TEST_F(
     LoadingPredictorTabHelperOptimizationGuideDeciderTest,
     DocumentOnLoadCompletedOptimizationGuidePredictionArrivedNoLoadingPredictorMetadata) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   // The problem here is that mock_collector_ is a strict mock, which expects
@@ -714,6 +765,8 @@ class LoadingPredictorTabHelperOptimizationGuideDeciderWithPrefetchTest
 // id and optimization guide prediction.
 TEST_F(LoadingPredictorTabHelperOptimizationGuideDeciderWithPrefetchTest,
        DocumentOnLoadCompletedOptimizationGuide) {
+  NavigateToGwsInMainFrame();
+
   base::HistogramTester histogram_tester;
 
   optimization_guide::OptimizationMetadata optimization_metadata;

chrome/browser/predictors/predictors_switches.cc

@@ -11,4 +11,9 @@ namespace switches {
 const char kLoadingPredictorAllowLocalRequestForTesting[] =
     "loading-predictor-allow-local-request-for-testing";
 
+// Allows the loading predictor to consult the optimization guide on non-GWS
+// page loads.
+const char kLoadingPredictorOptimizationGuideAllowNonGwsForTesting[] =
+    "loading-predictor-optimization-guide-allow-non-gws";
+
 }  // namespace switches

chrome/browser/predictors/predictors_switches.h

@@ -9,6 +9,8 @@ namespace switches {
 
 extern const char kLoadingPredictorAllowLocalRequestForTesting[];
 
+extern const char kLoadingPredictorOptimizationGuideAllowNonGwsForTesting[];
+
 }  // namespace switches
 
 #endif  // CHROME_BROWSER_PREDICTORS_PREDICTORS_SWITCHES_H_