[CSC] Refactor feature params

Refactor feature params to a separate file in //c/b.

Also, move the feature params inside a function call
instead of FeatureParam.
This enables us to modify the return value based
on multiple different field trial experiments which
is needed in the next CL.

Functionally, this is a no-op change.

(cherry picked from commit 110df0c)

Bug: b/286111571,1449021
Change-Id: I697e00379749cb5275701db50fd7f2c064d084bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4606829
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1155942}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4610473
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Auto-Submit: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#726}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}

chrome/browser/companion/core/BUILD.gn

@@ -16,6 +16,8 @@ static_library("core") {
     "promo_handler.cc",
     "promo_handler.h",
     "signin_delegate.h",
+    "utils.cc",
+    "utils.h",
   ]
   public_deps = [ "//third_party/abseil-cpp:absl" ]
   deps = [
@@ -39,6 +41,7 @@ source_set("unit_tests") {
     "companion_metrics_logger_unittest.cc",
     "companion_url_builder_unittest.cc",
     "promo_handler_unittest.cc",
+    "utils_unittest.cc",
   ]
 
   deps = [

chrome/browser/companion/core/companion_url_builder.cc

@@ -7,9 +7,9 @@
 #include "base/base64.h"
 #include "chrome/browser/companion/core/companion_permission_utils.h"
 #include "chrome/browser/companion/core/constants.h"
-#include "chrome/browser/companion/core/features.h"
 #include "chrome/browser/companion/core/proto/companion_url_params.pb.h"
 #include "chrome/browser/companion/core/signin_delegate.h"
+#include "chrome/browser/companion/core/utils.h"
 #include "components/prefs/pref_service.h"
 #include "net/base/url_util.h"
 #include "url/gurl.h"
@@ -64,19 +64,19 @@ CompanionUrlBuilder::CompanionUrlBuilder(PrefService* pref_service,
 
 CompanionUrlBuilder::~CompanionUrlBuilder() = default;
 
-GURL CompanionUrlBuilder::BuildCompanionURL(GURL page_url) {
+GURL CompanionUrlBuilder::BuildCompanionURL(const GURL& page_url) {
   return BuildCompanionURL(page_url, /*text_query=*/"");
 }
 
-GURL CompanionUrlBuilder::BuildCompanionURL(GURL page_url,
+GURL CompanionUrlBuilder::BuildCompanionURL(const GURL& page_url,
                                             const std::string& text_query) {
-  return AppendCompanionParamsToURL(GetHomepageURLForCompanion(), page_url,
-                                    text_query);
+  return AppendCompanionParamsToURL(GURL(GetHomepageURLForCompanion()),
+                                    page_url, text_query);
 }
 
 GURL CompanionUrlBuilder::AppendCompanionParamsToURL(
-    GURL base_url,
-    GURL page_url,
+    const GURL& base_url,
+    const GURL& page_url,
     const std::string& text_query) {
   GURL url_with_query_params = base_url;
   // Fill the protobuf with the required query params.
@@ -105,7 +105,8 @@ GURL CompanionUrlBuilder::AppendCompanionParamsToURL(
   return url_with_query_params;
 }
 
-std::string CompanionUrlBuilder::BuildCompanionUrlParamProto(GURL page_url) {
+std::string CompanionUrlBuilder::BuildCompanionUrlParamProto(
+    const GURL& page_url) {
   // Fill the protobuf with the required query params.
   companion::proto::CompanionUrlParams url_params;
   bool is_msbb_enabled =
@@ -137,8 +138,4 @@ std::string CompanionUrlBuilder::BuildCompanionUrlParamProto(GURL page_url) {
   return base64_encoded_proto;
 }
 
-GURL CompanionUrlBuilder::GetHomepageURLForCompanion() {
-  return GURL(features::kHomepageURLForCompanion.Get());
-}
-
 }  // namespace companion

chrome/browser/companion/core/companion_url_builder.h

@@ -28,24 +28,21 @@ class CompanionUrlBuilder {
   // query parameter set to the protobuf representation of the `page_url` and
   // associated state. Invokes `BuildCompanionUrlParamProto` internally to
   // create the proto.
-  GURL BuildCompanionURL(GURL page_url);
-  GURL BuildCompanionURL(GURL page_url, const std::string& text_query);
+  GURL BuildCompanionURL(const GURL& page_url);
+  GURL BuildCompanionURL(const GURL& page_url, const std::string& text_query);
 
   // Returns the `base_url` with the query parameters set to the protobuf
   // representation of the `page_url` and associated state.
-  GURL AppendCompanionParamsToURL(GURL base_url,
-                                  GURL page_url,
+  GURL AppendCompanionParamsToURL(const GURL& base_url,
+                                  const GURL& page_url,
                                   const std::string& text_query);
 
   // Returns the protobuf representation of the `page_url` and
   // associated state. Used to notify the companion page both during initial
   // load and subsequent state updates.
-  std::string BuildCompanionUrlParamProto(GURL page_url);
+  std::string BuildCompanionUrlParamProto(const GURL& page_url);
 
  private:
-  // The base URL for companion.
-  GURL GetHomepageURLForCompanion();
-
   raw_ptr<PrefService> pref_service_;
   raw_ptr<SigninDelegate> signin_delegate_;
 };

chrome/browser/companion/core/features.cc

@@ -23,24 +23,6 @@ BASE_FEATURE(kCompanionEnabledByObservingExpsNavigations,
              "CompanionEnabledByObservingExpsNavigations",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
-constexpr base::FeatureParam<std::string> kHomepageURLForCompanion{
-    &kSidePanelCompanion, "companion-homepage-url",
-    "https://lens.google.com/companion"};
-
-constexpr base::FeatureParam<std::string> kImageUploadURLForCompanion{
-    &kSidePanelCompanion, "companion-image-upload-url",
-    "https://lens.google.com/upload"};
-constexpr base::FeatureParam<bool> kEnableOpenCompanionForImageSearch{
-    &kSidePanelCompanion, "open-companion-for-image-search", true};
-constexpr base::FeatureParam<bool> kEnableOpenCompanionForWebSearch{
-    &kSidePanelCompanion, "open-companion-for-web-search", true};
-constexpr base::FeatureParam<bool> kOpenLinksInCurrentTab{
-    &kSidePanelCompanion, "open-links-in-current-tab", true};
-
-constexpr base::FeatureParam<std::string> kExpsRegistrationSuccessPageURLs{
-    &kCompanionEnabledByObservingExpsNavigations,
-    "exps-registration-success-page-urls", ""};
-
 }  // namespace features
 
 namespace switches {

chrome/browser/companion/core/features.h

@@ -15,12 +15,6 @@ namespace features {
 
 BASE_DECLARE_FEATURE(kSidePanelCompanion);
 BASE_DECLARE_FEATURE(kCompanionEnabledByObservingExpsNavigations);
-extern const base::FeatureParam<std::string> kHomepageURLForCompanion;
-extern const base::FeatureParam<std::string> kImageUploadURLForCompanion;
-extern const base::FeatureParam<std::string> kExpsRegistrationSuccessPageURLs;
-extern const base::FeatureParam<bool> kEnableOpenCompanionForImageSearch;
-extern const base::FeatureParam<bool> kEnableOpenCompanionForWebSearch;
-extern const base::FeatureParam<bool> kOpenLinksInCurrentTab;
 
 }  // namespace features
 

chrome/browser/companion/core/utils.cc

@@ -0,0 +1,59 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/companion/core/utils.h"
+
+#include "chrome/browser/companion/core/constants.h"
+#include "chrome/browser/companion/core/features.h"
+
+namespace companion {
+
+std::string GetHomepageURLForCompanion() {
+  std::string url = base::GetFieldTrialParamValueByFeature(
+      features::kSidePanelCompanion, "companion-homepage-url");
+  if (url.empty()) {
+    return std::string("https://lens.google.com/companion");
+  }
+  return url;
+}
+
+std::string GetImageUploadURLForCompanion() {
+  std::string url = base::GetFieldTrialParamValueByFeature(
+      features::kSidePanelCompanion, "companion-image-upload-url");
+  if (url.empty()) {
+    return std::string("https://lens.google.com/upload");
+  }
+  return url;
+}
+
+bool ShouldEnableOpenCompanionForImageSearch() {
+  if (base::FeatureList::IsEnabled(features::kSidePanelCompanion)) {
+    return base::GetFieldTrialParamByFeatureAsBool(
+        features::kSidePanelCompanion, "open-companion-for-image-search", true);
+  }
+  return false;
+}
+
+bool ShouldEnableOpenCompanionForWebSearch() {
+  if (base::FeatureList::IsEnabled(features::kSidePanelCompanion)) {
+    return base::GetFieldTrialParamByFeatureAsBool(
+        features::kSidePanelCompanion, "open-companion-for-web-search", true);
+  }
+  return false;
+}
+bool ShouldOpenLinksInCurrentTab() {
+  if (base::FeatureList::IsEnabled(features::kSidePanelCompanion)) {
+    return base::GetFieldTrialParamByFeatureAsBool(
+        features::kSidePanelCompanion, "open-links-in-current-tab", true);
+  }
+  return false;
+}
+
+std::string GetExpsRegistrationSuccessPageURLs() {
+  return base::GetFieldTrialParamValueByFeature(
+      features::kCompanionEnabledByObservingExpsNavigations,
+      "exps-registration-success-page-urls");
+}
+
+}  // namespace companion

chrome/browser/companion/core/utils.h

@@ -0,0 +1,21 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COMPANION_CORE_UTILS_H_
+#define CHROME_BROWSER_COMPANION_CORE_UTILS_H_
+
+#include <string>
+
+namespace companion {
+
+std::string GetHomepageURLForCompanion();
+std::string GetImageUploadURLForCompanion();
+bool ShouldEnableOpenCompanionForImageSearch();
+bool ShouldEnableOpenCompanionForWebSearch();
+bool ShouldOpenLinksInCurrentTab();
+std::string GetExpsRegistrationSuccessPageURLs();
+
+}  // namespace companion
+
+#endif  // CHROME_BROWSER_COMPANION_CORE_UTILS_H_

chrome/browser/companion/core/utils_unittest.cc

@@ -0,0 +1,28 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/companion/core/utils.h"
+
+#include "chrome/browser/companion/core/constants.h"
+#include "chrome/browser/companion/core/features.h"
+#include "content/public/test/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace companion {
+
+namespace {
+
+class CompanionCoreUtilsTest : public testing::Test {};
+
+TEST_F(CompanionCoreUtilsTest, HomepageURLForCompanion) {
+  EXPECT_EQ("https://lens.google.com/companion", GetHomepageURLForCompanion());
+}
+
+TEST_F(CompanionCoreUtilsTest, ImageUploadURLForCompanion) {
+  EXPECT_EQ("https://lens.google.com/upload", GetImageUploadURLForCompanion());
+}
+
+}  // namespace
+
+}  // namespace companion

chrome/browser/ui/side_panel/companion/companion_tab_helper.cc

@@ -9,6 +9,7 @@
 #include "base/strings/strcat.h"
 #include "chrome/browser/companion/core/features.h"
 #include "chrome/browser/companion/core/mojom/companion.mojom.h"
+#include "chrome/browser/companion/core/utils.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
 #include "chrome/browser/ui/side_panel/companion/companion_side_panel_controller_utils.h"
 #include "chrome/browser/ui/webui/side_panel/companion/companion_page_handler.h"
@@ -52,8 +53,7 @@ void CompanionTabHelper::ShowCompanionSidePanelForImage(
   CHECK(delegate_);
 
   // Create upload URL to load in companion.
-  std::string upload_url_string =
-      companion::features::kImageUploadURLForCompanion.Get();
+  std::string upload_url_string = companion::GetImageUploadURLForCompanion();
   base::StrAppend(&upload_url_string, {"?", additional_query_params_modified});
   GURL upload_url = GURL(upload_url_string);
   CHECK(upload_url.is_valid());

chrome/browser/ui/side_panel/companion/companion_utils.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/browser/companion/core/constants.h"
 #include "chrome/browser/companion/core/features.h"
+#include "chrome/browser/companion/core/utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser.h"
@@ -68,15 +69,15 @@ bool IsSearchWebInCompanionSidePanelSupported(const Browser* browser) {
     return false;
   }
   return IsSearchInCompanionSidePanelSupported(browser) &&
-         features::kEnableOpenCompanionForWebSearch.Get();
+         ShouldEnableOpenCompanionForWebSearch();
 }
 
 bool IsSearchImageInCompanionSidePanelSupported(const Browser* browser) {
   if (!browser) {
     return false;
   }
   return IsSearchInCompanionSidePanelSupported(browser) &&
-         features::kEnableOpenCompanionForImageSearch.Get();
+         ShouldEnableOpenCompanionForImageSearch();
 }
 
 void UpdateCompanionDefaultPinnedToToolbarState(PrefService* pref_service) {

chrome/browser/ui/side_panel/companion/exps_registration_success_observer.cc

@@ -7,6 +7,7 @@
 #include "base/strings/string_split.h"
 #include "chrome/browser/companion/core/constants.h"
 #include "chrome/browser/companion/core/features.h"
+#include "chrome/browser/companion/core/utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_contents.h"
@@ -18,9 +19,9 @@ ExpsRegistrationSuccessObserver::ExpsRegistrationSuccessObserver(
     : content::WebContentsObserver(web_contents),
       content::WebContentsUserData<ExpsRegistrationSuccessObserver>(
           *web_contents) {
-  const auto& url_strings_to_match = base::SplitString(
-      companion::features::kExpsRegistrationSuccessPageURLs.Get(), ",",
-      base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  const auto& url_strings_to_match =
+      base::SplitString(companion::GetExpsRegistrationSuccessPageURLs(), ",",
+                        base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
   for (const auto& url_string : url_strings_to_match) {
     urls_to_match_against_.emplace_back(url_string);
   }

chrome/browser/ui/views/side_panel/search_companion/companion_side_panel_controller.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/side_panel/search_companion/companion_side_panel_controller.h"
 
 #include "chrome/browser/companion/core/features.h"
+#include "chrome/browser/companion/core/utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/side_panel/companion/companion_tab_helper.h"
@@ -133,7 +134,7 @@ void CompanionSidePanelController::DidOpenRequestedURL(
     params.initiator_origin = url::Origin::Create(url);
   }
 
-  bool open_in_current_tab = companion::features::kOpenLinksInCurrentTab.Get();
+  bool open_in_current_tab = companion::ShouldOpenLinksInCurrentTab();
   params.disposition = open_in_current_tab
                            ? WindowOpenDisposition::CURRENT_TAB
                            : WindowOpenDisposition::NEW_FOREGROUND_TAB;

chrome/browser/ui/views/side_panel/search_companion/search_companion_side_panel_coordinator.cc

@@ -50,9 +50,9 @@ SearchCompanionSidePanelCoordinator::SearchCompanionSidePanelCoordinator(
     CreateAndRegisterEntriesForExistingWebContents(browser_->tab_strip_model());
   }
 
-  pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
-  pref_change_registrar_->Init(pref_service_);
-  pref_change_registrar_->Add(
+  policy_pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
+  policy_pref_change_registrar_->Init(pref_service_);
+  policy_pref_change_registrar_->Add(
       prefs::kGoogleSearchSidePanelEnabled,
       base::BindRepeating(
           &SearchCompanionSidePanelCoordinator::OnPolicyPrefChanged,

chrome/browser/ui/views/side_panel/search_companion/search_companion_side_panel_coordinator.h

@@ -95,7 +95,7 @@ class SearchCompanionSidePanelCoordinator
   raw_ptr<PrefService> pref_service_;
   bool is_currently_observing_tab_changes_ = false;
 
-  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
+  std::unique_ptr<PrefChangeRegistrar> policy_pref_change_registrar_;
 
   base::ScopedObservation<TemplateURLService, TemplateURLServiceObserver>
       template_url_service_observation_{this};

chrome/browser/ui/webui/side_panel/companion/companion_side_panel_untrusted_ui.cc

@@ -4,7 +4,7 @@
 
 #include "chrome/browser/ui/webui/side_panel/companion/companion_side_panel_untrusted_ui.h"
 
-#include "chrome/browser/companion/core/features.h"
+#include "chrome/browser/companion/core/utils.h"
 #include "chrome/browser/ui/webui/side_panel/companion/companion_page_handler.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/grit/side_panel_companion_resources.h"
@@ -33,12 +33,11 @@ CompanionSidePanelUntrustedUI::CompanionSidePanelUntrustedUI(
       network::mojom::CSPDirectiveName::ScriptSrc,
       "script-src chrome-untrusted://resources 'self';");
   // Allow the companion homepage URL to be embedded in this WebUI.
-  GURL frameSrcUrl = GURL(companion::features::kHomepageURLForCompanion.Get())
-                         .GetWithEmptyPath();
-  std::string frameSrcString =
-      frameSrcUrl.is_valid()
-          ? frameSrcUrl.spec()
-          : companion::features::kHomepageURLForCompanion.Get();
+  GURL frameSrcUrl =
+      GURL(companion::GetHomepageURLForCompanion()).GetWithEmptyPath();
+  std::string frameSrcString = frameSrcUrl.is_valid()
+                                   ? frameSrcUrl.spec()
+                                   : companion::GetHomepageURLForCompanion();
   // Allow iframing accounts page due to potential redirects.
   std::string frameSrcDirective =
       std::string("frame-src https://accounts.google.com ") + frameSrcString +