Remove experiment name and excluded experiment name from hints processing

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

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -137,11 +137,6 @@ bool IsOptimizationTypeAllowed(
     if (optimization_type != optimization.optimization_type())
       continue;
 
-    if (optimization_guide::IsDisabledPerOptimizationHintExperiment(
-            optimization)) {
-      continue;
-    }
-
     if (optimization.has_tuning_version()) {
       *tuning_version = optimization.tuning_version();
 

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -357,14 +357,6 @@ class OptimizationGuideHintsManagerTest
     hint1->set_version("someversion");
     optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
     page_hint1->set_page_pattern("/news/");
-    optimization_guide::proto::Optimization* experimental_opt =
-        page_hint1->add_whitelisted_optimizations();
-    experimental_opt->set_optimization_type(
-        optimization_guide::proto::NOSCRIPT);
-    experimental_opt->set_experiment_name("experiment");
-    optimization_guide::proto::PreviewsMetadata* experimental_opt_metadata =
-        experimental_opt->mutable_previews_metadata();
-    experimental_opt_metadata->set_inflation_percent(12345);
     optimization_guide::proto::Optimization* default_opt =
         page_hint1->add_whitelisted_optimizations();
     default_opt->set_optimization_type(optimization_guide::proto::NOSCRIPT);
@@ -1977,46 +1969,6 @@ TEST_F(OptimizationGuideHintsManagerTest,
             optimization_type_decision);
 }
 
-class OptimizationGuideHintsManagerExperimentTest
-    : public OptimizationGuideHintsManagerTest {
- public:
-  OptimizationGuideHintsManagerExperimentTest() {
-    scoped_list_.InitAndEnableFeatureWithParameters(
-        optimization_guide::features::kOptimizationHintsExperiments,
-        {{"experiment_name", "experiment"}});
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_list_;
-};
-
-TEST_F(OptimizationGuideHintsManagerExperimentTest,
-       CanApplyOptimizationAndPopulatesMetadataWithFirstOptThatMatchesWithExp) {
-  InitializeWithDefaultConfig("1.0.0.0");
-
-  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
-      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
-          url_with_hints());
-  base::RunLoop run_loop;
-  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
-                                               run_loop.QuitClosure());
-  run_loop.Run();
-
-  hints_manager()->RegisterOptimizationTypes(
-      {optimization_guide::proto::NOSCRIPT});
-  optimization_guide::OptimizationMetadata optimization_metadata;
-  optimization_guide::OptimizationTypeDecision optimization_type_decision =
-      hints_manager()->CanApplyOptimization(
-          navigation_handle->GetURL(), /*navigation_id=*/base::nullopt,
-          optimization_guide::proto::NOSCRIPT, &optimization_metadata);
-
-  EXPECT_EQ(
-      12345,
-      optimization_metadata.previews_metadata().value().inflation_percent());
-  EXPECT_EQ(optimization_guide::OptimizationTypeDecision::kAllowedByHint,
-            optimization_type_decision);
-}
-
 class OptimizationGuideHintsManagerFetchingDisabledTest
     : public OptimizationGuideHintsManagerTest {
  public:

chrome/browser/previews/resource_loading_hints/resource_loading_hints_browsertest.cc

@@ -214,42 +214,6 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
     LoadHintsForUrl(hint_setup_url);
   }
 
-  // Sets the resource loading hints in optimization guide service. The hints
-  // are set as experimental.
-  void SetExperimentOnlyResourceLoadingHints(const GURL& hint_setup_url) {
-    std::vector<std::string> resource_patterns;
-    resource_patterns.push_back("foo.jpg");
-    resource_patterns.push_back("png");
-    resource_patterns.push_back("woff2");
-
-    ProcessHintsComponent(
-        test_hints_component_creator_
-            .CreateHintsComponentInfoWithExperimentalPageHints(
-                optimization_guide::proto::RESOURCE_LOADING,
-                {hint_setup_url.host()}, resource_patterns));
-    LoadHintsForUrl(hint_setup_url);
-  }
-
-  // Sets the resource loading hints in optimization guide service. Some hints
-  // are set as experimental, while others are set as default.
-  void SetMixResourceLoadingHints(const GURL& hint_setup_url) {
-    std::vector<std::string> experimental_resource_patterns;
-    experimental_resource_patterns.push_back("foo.jpg");
-    experimental_resource_patterns.push_back("png");
-    experimental_resource_patterns.push_back("woff2");
-
-    std::vector<std::string> default_resource_patterns;
-    default_resource_patterns.push_back("bar.jpg");
-    default_resource_patterns.push_back("woff2");
-
-    ProcessHintsComponent(
-        test_hints_component_creator_.CreateHintsComponentInfoWithMixPageHints(
-            optimization_guide::proto::RESOURCE_LOADING,
-            {hint_setup_url.host()}, experimental_resource_patterns,
-            default_resource_patterns));
-    LoadHintsForUrl(hint_setup_url);
-  }
-
   virtual const GURL& https_url() const { return https_url_; }
 
   // URL that loads blocked resources uses link-rel preload in <head>.

components/optimization_guide/hints_processing_util.cc

@@ -56,29 +56,6 @@ std::string GetStringNameForOptimizationType(
   return std::string();
 }
 
-bool IsDisabledPerOptimizationHintExperiment(
-    const proto::Optimization& optimization) {
-  // First check if optimization depends on an experiment being enabled.
-  if (optimization.has_experiment_name() &&
-      !optimization.experiment_name().empty() &&
-      optimization.experiment_name() !=
-          base::GetFieldTrialParamValueByFeature(
-              features::kOptimizationHintsExperiments,
-              features::kOptimizationHintsExperimentNameParam)) {
-    return true;
-  }
-  // Now check if optimization depends on an experiment not being enabled.
-  if (optimization.has_excluded_experiment_name() &&
-      !optimization.excluded_experiment_name().empty() &&
-      optimization.excluded_experiment_name() ==
-          base::GetFieldTrialParamValueByFeature(
-              features::kOptimizationHintsExperiments,
-              features::kOptimizationHintsExperimentNameParam)) {
-    return true;
-  }
-  return false;
-}
-
 const proto::PageHint* FindPageHintForURL(const GURL& gurl,
                                           const proto::Hint* hint) {
   if (!hint) {

components/optimization_guide/hints_processing_util.h

@@ -18,15 +18,6 @@ namespace optimization_guide {
 std::string GetStringNameForOptimizationType(
     proto::OptimizationType optimization_type);
 
-// Returns whether |optimization| is disabled subject to it being part of
-// an optimization hint experiment. |optimization| could be disabled either
-// because: it is only to be used with a named optimization experiment; or it
-// is not to be used with a named excluded experiment. One experiment name
-// may be configured for the client with the experiment_name parameter to the
-// kOptimizationHintsExperiments feature.
-bool IsDisabledPerOptimizationHintExperiment(
-    const proto::Optimization& optimization);
-
 // Returns the matching PageHint for |gurl| if found in |hint|.
 const proto::PageHint* FindPageHintForURL(const GURL& gurl,
                                           const proto::Hint* hint);

components/optimization_guide/hints_processing_util_unittest.cc

@@ -57,6 +57,7 @@ TEST(HintsProcessingUtilTest, FindPageHintForSubstringPagePattern) {
   EXPECT_EQ(page_hint3, FindPageHintForURL(
                             GURL("https://www.foo.org/bar/three.jpg"), &hint1));
 }
+
 TEST(HintsProcessingUtilTest, ConvertProtoEffectiveConnectionType) {
   EXPECT_EQ(
       ConvertProtoEffectiveConnectionType(

components/optimization_guide/optimization_guide_features.cc

@@ -31,18 +31,6 @@ const base::Feature kOptimizationHints {
 #endif  // defined(OS_ANDROID)
 };
 
-// Enables Optimization Hints that are marked as experimental. Optimizations are
-// marked experimental by setting an experiment name in the "experiment_name"
-// field of the Optimization proto. This allows experiments at the granularity
-// of a single PreviewType for a single host (or host suffix). The intent is
-// that optimizations that may not work properly for certain sites can be tried
-// at a small scale via Finch experiments. Experimental optimizations can be
-// activated by enabling this feature and passing an experiment name as a
-// parameter called "experiment_name" that matches the experiment name in the
-// Optimization proto.
-const base::Feature kOptimizationHintsExperiments{
-    "OptimizationHintsExperiments", base::FEATURE_DISABLED_BY_DEFAULT};
-
 // Feature flag that contains a feature param that specifies the field trials
 // that are allowed to be sent up to the Optimization Guide Server.
 const base::Feature kOptimizationHintsFieldTrials{

components/optimization_guide/optimization_guide_features.h

@@ -20,9 +20,7 @@ namespace optimization_guide {
 namespace features {
 
 extern const base::Feature kOptimizationHints;
-extern const base::Feature kOptimizationHintsExperiments;
 extern const base::Feature kOptimizationHintsFieldTrials;
-constexpr char kOptimizationHintsExperimentNameParam[] = "experiment_name";
 extern const base::Feature kRemoteOptimizationGuideFetching;
 extern const base::Feature kRemoteOptimizationGuideFetchingAnonymousDataConsent;
 extern const base::Feature kOptimizationTargetPrediction;

components/optimization_guide/proto/hints.proto

@@ -153,29 +153,9 @@ enum KeyRepresentation {
 }
 
 message Optimization {
-  reserved 2, 3, 14;
+  reserved 2, 3, 4, 5, 14;
   // The type of optimization the hint applies to.
   optional OptimizationType optimization_type = 1;
-  // The experiment name that activates the optimization.
-  //
-  // If a non-empty name is provided, the optimization will be disabled unless
-  // an experiment of that same name is running. An empty name is not
-  // experimental.
-  //
-  // Optimization Hints experiments are enabled and configured by passing an
-  // experiment_name parameter to the OptimizationHintsExperiments feature.
-  // For example, if experiment_name is my_exp, it could be enabled with the
-  // following command-line flags:
-  //   --enable-features=OptimizationHintsExperiments<MyFieldTrial
-  //   --force-fieldtrial-params="MyFieldTrial.Enabled:experiment_name/my_exp"
-  //   --force-fieldtrials=MyFieldTrial/Enabled/
-  optional string experiment_name = 4;
-  // The experiment name that should be excluded from using this optimization.
-  //
-  // If a non-empty name is provided and the client is in an experiment of that
-  // same name, the optimization will be disabled and skipped for that client.
-  // An empty name is not experimental.
-  optional string excluded_experiment_name = 5;
   // The version of the tuning group for the optimization type.
   //
   // This will only be populated if this optimization is being autotuned.

components/optimization_guide/test_hints_component_creator.cc

@@ -85,113 +85,6 @@ TestHintsComponentCreator::CreateHintsComponentInfoWithPageHints(
   return WriteConfigToFileAndReturnHintsComponentInfo(config);
 }
 
-optimization_guide::HintsComponentInfo
-TestHintsComponentCreator::CreateHintsComponentInfoWithExperimentalPageHints(
-    optimization_guide::proto::OptimizationType optimization_type,
-    const std::vector<std::string>& page_hint_hosts,
-    const std::vector<std::string>& experimental_resource_patterns) {
-  optimization_guide::proto::Configuration config;
-  for (const auto& page_hint_site : page_hint_hosts) {
-    optimization_guide::proto::Hint* hint = config.add_hints();
-    hint->set_key(page_hint_site);
-    hint->set_key_representation(optimization_guide::proto::HOST);
-    hint->set_version(GetDefaultHintVersionString());
-
-    optimization_guide::proto::PageHint* page_hint = hint->add_page_hints();
-    page_hint->set_page_pattern("*");
-
-    optimization_guide::proto::Optimization* optimization =
-        page_hint->add_whitelisted_optimizations();
-    optimization->set_optimization_type(optimization_type);
-    optimization->set_experiment_name(kFooExperimentName);
-
-    for (auto resource_blocking_pattern : experimental_resource_patterns) {
-      optimization_guide::proto::ResourceLoadingHint* resource_loading_hint =
-          optimization->mutable_previews_metadata()
-              ->add_resource_loading_hints();
-      resource_loading_hint->set_loading_optimization_type(
-          optimization_guide::proto::LOADING_BLOCK_RESOURCE);
-      resource_loading_hint->set_resource_pattern(resource_blocking_pattern);
-    }
-  }
-  // Always stick something with no hint version in here.
-  optimization_guide::proto::Hint* no_version_hint = config.add_hints();
-  no_version_hint->set_key("noversion.com");
-  no_version_hint->set_key_representation(optimization_guide::proto::HOST);
-  no_version_hint->add_page_hints()->set_page_pattern("*");
-  // Always stick something with a bad hint version in here.
-  optimization_guide::proto::Hint* bad_version_hint = config.add_hints();
-  bad_version_hint->set_key("badversion.com");
-  bad_version_hint->set_key_representation(optimization_guide::proto::HOST);
-  bad_version_hint->set_version("notaversion");
-  bad_version_hint->add_page_hints()->set_page_pattern("*");
-
-  return WriteConfigToFileAndReturnHintsComponentInfo(config);
-}
-
-optimization_guide::HintsComponentInfo
-TestHintsComponentCreator::CreateHintsComponentInfoWithMixPageHints(
-    optimization_guide::proto::OptimizationType optimization_type,
-    const std::vector<std::string>& page_hint_hosts,
-    const std::vector<std::string>& experimental_resource_patterns,
-    const std::vector<std::string>& default_resource_patterns) {
-  optimization_guide::proto::Configuration config;
-  for (const auto& page_hint_site : page_hint_hosts) {
-    optimization_guide::proto::Hint* hint = config.add_hints();
-    hint->set_key(page_hint_site);
-    hint->set_key_representation(optimization_guide::proto::HOST);
-    hint->set_version(GetDefaultHintVersionString());
-
-    optimization_guide::proto::PageHint* page_hint = hint->add_page_hints();
-    page_hint->set_page_pattern("*");
-
-    // Add experimental patterns first so they get higher priority.
-    {
-      optimization_guide::proto::Optimization* optimization =
-          page_hint->add_whitelisted_optimizations();
-      optimization->set_optimization_type(optimization_type);
-      optimization->set_experiment_name(kFooExperimentName);
-
-      for (auto resource_blocking_pattern : experimental_resource_patterns) {
-        optimization_guide::proto::ResourceLoadingHint* resource_loading_hint =
-            optimization->mutable_previews_metadata()
-                ->add_resource_loading_hints();
-        resource_loading_hint->set_loading_optimization_type(
-            optimization_guide::proto::LOADING_BLOCK_RESOURCE);
-        resource_loading_hint->set_resource_pattern(resource_blocking_pattern);
-      }
-    }
-
-    {
-      optimization_guide::proto::Optimization* optimization =
-          page_hint->add_whitelisted_optimizations();
-      optimization->set_optimization_type(optimization_type);
-
-      for (auto resource_blocking_pattern : default_resource_patterns) {
-        optimization_guide::proto::ResourceLoadingHint* resource_loading_hint =
-            optimization->mutable_previews_metadata()
-                ->add_resource_loading_hints();
-        resource_loading_hint->set_loading_optimization_type(
-            optimization_guide::proto::LOADING_BLOCK_RESOURCE);
-        resource_loading_hint->set_resource_pattern(resource_blocking_pattern);
-      }
-    }
-  }
-  // Always stick something with no hint version in here.
-  optimization_guide::proto::Hint* no_version_hint = config.add_hints();
-  no_version_hint->set_key("noversion.com");
-  no_version_hint->set_key_representation(optimization_guide::proto::HOST);
-  no_version_hint->add_page_hints()->set_page_pattern("*");
-  // Always stick something with a bad hint version in here.
-  optimization_guide::proto::Hint* bad_version_hint = config.add_hints();
-  bad_version_hint->set_key("badversion.com");
-  bad_version_hint->set_key_representation(optimization_guide::proto::HOST);
-  bad_version_hint->set_version("notaversion");
-  bad_version_hint->add_page_hints()->set_page_pattern("*");
-
-  return WriteConfigToFileAndReturnHintsComponentInfo(config);
-}
-
 base::FilePath TestHintsComponentCreator::GetFilePath(
     std::string file_path_suffix) {
   base::ScopedAllowBlockingForTesting allow_blocking;

components/optimization_guide/test_hints_component_creator.h

@@ -17,9 +17,6 @@
 namespace optimization_guide {
 namespace testing {
 
-// Experiment name used for experimental resource loading hints.
-static const char kFooExperimentName[] = "foo_experiment";
-
 // Helper class to create test OptimizationHints components for testing.
 //
 // All temporary files and paths are cleaned up when this instance goes out of
@@ -39,34 +36,6 @@ class TestHintsComponentCreator {
       const std::string& page_pattern,
       const std::vector<std::string>& resource_patterns);
 
-  // Creates component data based on |whitelisted_hosts| with page hints
-  // for type |optimization_type| blocking resources specified by
-  // |experimental_resource_patterns|, and returns the HintsComponentInfo for
-  // it. The loading hints are set as experimental with experiment name set to
-  // kFooExperimentName.
-
-  // Creates component data for testing with experimental optimizations. It
-  // creates a PageHint (with page pattern "*" for each key in
-  // |whitelisted_hosts| that each has resource blocking patterns from
-  // |experimental_resource_patterns|.
-  optimization_guide::HintsComponentInfo
-  CreateHintsComponentInfoWithExperimentalPageHints(
-      optimization_guide::proto::OptimizationType optimization_type,
-      const std::vector<std::string>& whitelisted_hosts,
-      const std::vector<std::string>& experimental_resource_patterns);
-
-  // Creates component data for testing with both default and experimental
-  // optimizations. It creates a PageHint (with page pattern "*" for each key in
-  // |whitelisted_hosts| that each has resource blocking patterns from
-  // |default_resource_patterns| and |experimental_resource_patterns|. The
-  // experimental hints are guarded behind experiment kFooExperimentName.
-  optimization_guide::HintsComponentInfo
-  CreateHintsComponentInfoWithMixPageHints(
-      optimization_guide::proto::OptimizationType optimization_type,
-      const std::vector<std::string>& whitelisted_hosts,
-      const std::vector<std::string>& experimental_resource_patterns,
-      const std::vector<std::string>& default_resource_patterns);
-
  private:
   // Returns the scoped temp directory path with the |file_path_suffix| that is
   // valid for the lifetime of this instance.