Reland "Reland "[CSC] Create chrome flags for Companion UXR.""

This reverts commit f91001b. Reason for revert: Re-land to fix the enums and histogram checks. Original change's description: > Revert "Reland "[CSC] Create chrome flags for Companion UXR."" > > This reverts commit 86f8ba8. > > Reason for revert: Tests still failing for feature: > https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome/33141/test-results?sortby=&groupby= > > Bug: 1449587 > > Original change's description: > > Reland "[CSC] Create chrome flags for Companion UXR." > > > > This reverts commit d90aaa6. > > > > Reason for revert: Fixing the enums generation issue. > > > > Original change's description: > > > Revert "[CSC] Create chrome flags for Companion UXR." > > > > > > This reverts commit 8b1641f. > > > > > > Reason for revert: Failing AboutFlagsHistogram on linux-chromeos-chrome. > > > https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome/33089/overview > > > PTAL: https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histograms/README.md#flag-histograms > > > > > > Original change's description: > > > > [CSC] Create chrome flags for Companion UXR. > > > > > > > > This requires us to be able to explicitly control the clobber vs new > > > > tab behavior. We also need to be able to control the pin AND ignore > > > > the labs experiment state (e.g., when unpinned and labs is on, > > > > the UI will attempt to force the state to pinned, this overrides > > > > that behavior). > > > > > > > > Bug: b:283109243 > > > > Change-Id: I981ad573ee38cb4b69bb39f008c134c4e14bbf56 > > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4553173 > > > > Reviewed-by: Caroline Rising <corising@chromium.org> > > > > Commit-Queue: Michael Crouse <mcrouse@chromium.org> > > > > Reviewed-by: Tarun Bansal <tbansal@chromium.org> > > > > Cr-Commit-Position: refs/heads/main@{#1148554} > > > > > > Bug: b:283109243 > > > Change-Id: Ic0fcdcc8512a5e38ffbaef7b5492c766b1961294 > > > No-Presubmit: true > > > No-Tree-Checks: true > > > No-Try: true > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4564057 > > > Owners-Override: Ana Salazar Maldonado <anasalazar@google.com> > > > Reviewed-by: Ana Salazar Maldonado <anasalazar@google.com> > > > Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > > > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > > > Auto-Submit: Ana Salazar Maldonado <anasalazar@google.com> > > > Cr-Commit-Position: refs/heads/main@{#1148779} > > > > Bug: b:283109243 > > Change-Id: I1785c9de3aee67a87a19ce51cc8591fa99f7f40a > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4569767 > > Reviewed-by: Tarun Bansal <tbansal@chromium.org> > > Commit-Queue: Michael Crouse <mcrouse@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1149978} > > Bug: b:283109243 > Change-Id: I43be3f4a73c604e601d40bcf7a16745e2cc04f79 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571762 > Owners-Override: Ana Salazar Maldonado <anasalazar@google.com> > Auto-Submit: Ana Salazar Maldonado <anasalazar@google.com> > Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Cr-Commit-Position: refs/heads/main@{#1150204} (cherry picked from commit ff4a6cf) Bug: 1449587 Bug: b:283109243 Change-Id: I60082380b29022de4ca58417020c74a695b00772 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4567442 Commit-Queue: Michael Crouse <mcrouse@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Auto-Submit: Michael Crouse <mcrouse@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1151070} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4590411 Cr-Commit-Position: refs/branch-heads/5790@{#375} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · Jun 5, 2023 · d20d1b3 · d20d1b3
1 parent 0dce5c9
commit d20d1b3
Show file tree

Hide file tree

Showing 11 changed files with 189 additions and 3 deletions.
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -3240,14 +3240,34 @@ constexpr FeatureEntry::FeatureVariation kLensPingVariations[] = {
 };
 
 #if BUILDFLAG(ENABLE_LENS_DESKTOP_GOOGLE_BRANDED_FEATURES)
-constexpr FeatureEntry::FeatureParam kCscVariation[] = {
+constexpr FeatureEntry::FeatureParam kCscStagingEnvVariation[] = {
     {"companion-homepage-url",
      "https://lens-staging.corp.google.com/companion"},
     {"companion-image-upload-url",
      "https://lens-staging.corp.google.com/v2/upload"}};
+constexpr FeatureEntry::FeatureParam kCscClobberVariation[] = {
+    {"open-links-in-current-tab", "true"},
+};
+constexpr FeatureEntry::FeatureParam kCscNewTabVariation[] = {
+    {"open-links-in-current-tab", "false"},
+};
 
 constexpr FeatureEntry::FeatureVariation kCscVariations[] = {
-    {"with staging URL", kCscVariation, std::size(kCscVariation), nullptr}};
+    {"with staging URL", kCscStagingEnvVariation,
+     std::size(kCscStagingEnvVariation), nullptr},
+    {"with clobber", kCscClobberVariation, std::size(kCscClobberVariation),
+     nullptr},
+    {"with new tab", kCscNewTabVariation, std::size(kCscNewTabVariation),
+     nullptr},
+};
+
+const FeatureEntry::Choice kSidePanelPinnedStateChoices[] = {
+    {flags_ui::kGenericExperimentChoiceDefault, "", ""},
+    {"Forced Pinned", companion::switches::kForceCompanionPinnedState,
+     "pinned"},
+    {"Forced Unpinned", companion::switches::kForceCompanionPinnedState,
+     "unpinned"},
+};
 #endif  // BUILDFLAG(ENABLE_LENS_DESKTOP_GOOGLE_BRANDED_FEATURES)
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -8838,6 +8858,14 @@ const FeatureEntry kFeatureEntries[] = {
                                     kCscVariations,
                                     "CSC")},
 
+    {"csc-pinned-state", flag_descriptions::kCscPinnedName,
+     flag_descriptions::kCscPinnedDescription, kOsDesktop,
+     MULTI_VALUE_TYPE(kSidePanelPinnedStateChoices)},
+
+    {"csc-vss", flag_descriptions::kCscVssName,
+     flag_descriptions::kCscVssDescription, kOsDesktop,
+     FEATURE_VALUE_TYPE(visual_search::features::kVisualSearchSuggestions)},
+
     {"enable-lens-region-search-static-page",
      flag_descriptions::kLensRegionSearchStaticPageName,
      flag_descriptions::kLensRegionSearchStaticPageDescription, kOsDesktop,

diff --git a/chrome/browser/companion/core/BUILD.gn b/chrome/browser/companion/core/BUILD.gn
@@ -17,7 +17,7 @@ static_library("core") {
     "promo_handler.h",
     "signin_delegate.h",
   ]
-
+  public_deps = [ "//third_party/abseil-cpp:absl" ]
   deps = [
     "mojom:mojo_bindings",
     "proto",

diff --git a/chrome/browser/companion/core/features.cc b/chrome/browser/companion/core/features.cc
@@ -39,10 +39,32 @@ namespace switches {
 const char kDisableCheckUserPermissionsForCompanion[] =
     "disable-checking-companion-user-permissions";
 
+const char kForceCompanionPinnedState[] = "force-companion-pinned-state";
+
 bool ShouldOverrideCheckingUserPermissionsForCompanion() {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   return command_line->HasSwitch(kDisableCheckUserPermissionsForCompanion);
 }
 
+absl::optional<bool> ShouldForceOverrideCompanionPinState() {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (!command_line->HasSwitch(kForceCompanionPinnedState)) {
+    return absl::nullopt;
+  }
+
+  std::string pinned_state =
+      command_line->GetSwitchValueASCII(kForceCompanionPinnedState);
+  if (pinned_state == "pinned") {
+    return true;
+  }
+  if (pinned_state == "unpinned") {
+    return false;
+  }
+
+  NOTREACHED() << "Invalid Companion pin state command line switch value: "
+               << pinned_state;
+  return absl::nullopt;
+}
+
 }  // namespace switches
 }  // namespace companion
diff --git a/chrome/browser/companion/core/features.h b/chrome/browser/companion/core/features.h
@@ -8,6 +8,7 @@
 #include "base/feature_list.h"
 #include "base/metrics/field_trial_params.h"
 #include "build/build_config.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace companion {
 namespace features {
@@ -23,11 +24,16 @@ extern const base::FeatureParam<bool> kOpenLinksInCurrentTab;
 
 namespace switches {
 extern const char kDisableCheckUserPermissionsForCompanion[];
+extern const char kForceCompanionPinnedState[];
 
 // Returns true if checking of the user's permissions to share page information
 // with the Companion server should be ignored. Returns true only in tests.
 bool ShouldOverrideCheckingUserPermissionsForCompanion();
 
+// Returns whether the Companion pin state should force overridden, regardless
+// of prefs or labs state.
+absl::optional<bool> ShouldForceOverrideCompanionPinState();
+
 }  // namespace switches
 }  // namespace companion
 

diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
@@ -1327,6 +1327,16 @@
     "owners": ["stanfield@google.com", "mcrouse@google.com", "lens-chrome@google.com"],
     "expiry_milestone": 120
   },
+  {
+    "name": "csc-pinned-state",
+    "owners": ["stanfield@google.com", "mcrouse@google.com", "lens-chrome@google.com"],
+    "expiry_milestone": 120
+  },
+  {
+    "name": "csc-vss",
+    "owners": ["pstjuste@google.com", "stanfield@google.com", "mcrouse@google.com", "lens-chrome@google.com"],
+    "expiry_milestone": 120
+  },
   {
     "name": "cups-ipp-printing-backend",
     "owners": [ "awscreen", "//printing/OWNERS" ],

diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
@@ -1986,6 +1986,12 @@ const char kEnableLensPingDescription[] =
 const char kCscName[] = "CSC";
 const char kCscDescription[] = "";
 
+const char kCscPinnedName[] = "CSC Pin State";
+const char kCscPinnedDescription[] = "";
+
+const char kCscVssName[] = "CSC-VSS";
+const char kCscVssDescription[] = "";
+
 const char kLogJsConsoleMessagesName[] =
     "Log JS console messages in system logs";
 const char kLogJsConsoleMessagesDescription[] =

diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
@@ -1107,6 +1107,12 @@ extern const char kEnableLensPingDescription[];
 extern const char kCscName[];
 extern const char kCscDescription[];
 
+extern const char kCscPinnedName[];
+extern const char kCscPinnedDescription[];
+
+extern const char kCscVssName[];
+extern const char kCscVssDescription[];
+
 extern const char kLensOnQuickActionSearchWidgetName[];
 extern const char kLensOnQuickActionSearchWidgetDescription[];
 

diff --git a/chrome/browser/ui/side_panel/companion/companion_utils.cc b/chrome/browser/ui/side_panel/companion/companion_utils.cc
@@ -71,6 +71,14 @@ bool IsSearchImageInCompanionSidePanelSupported(const Browser* browser) {
 }
 
 void UpdateCompanionDefaultPinnedToToolbarState(PrefService* pref_service) {
+  absl::optional<bool> should_force_pin =
+      switches::ShouldForceOverrideCompanionPinState();
+  if (should_force_pin) {
+    pref_service->SetBoolean(prefs::kSidePanelCompanionEntryPinnedToToolbar,
+                             *should_force_pin);
+    return;
+  }
+
   bool companion_should_be_default_pinned =
       base::FeatureList::IsEnabled(
           ::features::kSidePanelCompanionDefaultPinned) ||

diff --git a/chrome/browser/ui/side_panel/companion/companion_utils_unittest.cc b/chrome/browser/ui/side_panel/companion/companion_utils_unittest.cc
@@ -0,0 +1,96 @@
+// 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/ui/side_panel/companion/companion_utils.h"
+
+#include "base/command_line.h"
+#include "base/feature_list.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/companion/core/constants.h"
+#include "chrome/browser/companion/core/features.h"
+#include "chrome/browser/ui/ui_features.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace companion {
+
+void RegisterPrefs(TestingPrefServiceSimple* pref_service) {
+  pref_service->registry()->RegisterBooleanPref(
+      prefs::kSidePanelCompanionEntryPinnedToToolbar, false);
+  pref_service->registry()->RegisterBooleanPref(
+      companion::kExpsOptInStatusGrantedPref, false);
+}
+
+TEST(CompanionUtilsTest, PinnedStateCommandlineOverridePinned) {
+  TestingPrefServiceSimple pref_service;
+  RegisterPrefs(&pref_service);
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kForceCompanionPinnedState, "pinned");
+
+  UpdateCompanionDefaultPinnedToToolbarState(&pref_service);
+  EXPECT_EQ(
+      pref_service.GetBoolean(prefs::kSidePanelCompanionEntryPinnedToToolbar),
+      true);
+}
+
+TEST(CompanionUtilsTest, PinnedStateCommandlineOverrideUnpinned) {
+  TestingPrefServiceSimple pref_service;
+  RegisterPrefs(&pref_service);
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kForceCompanionPinnedState, "unpinned");
+
+  UpdateCompanionDefaultPinnedToToolbarState(&pref_service);
+  EXPECT_EQ(
+      pref_service.GetBoolean(prefs::kSidePanelCompanionEntryPinnedToToolbar),
+      false);
+}
+
+TEST(CompanionUtilsTest, UpdatePinnedStateDefaultUnpinnedLabsOverride) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  TestingPrefServiceSimple pref_service;
+  RegisterPrefs(&pref_service);
+
+  scoped_feature_list.InitAndDisableFeature(
+      ::features::kSidePanelCompanionDefaultPinned);
+  pref_service.SetBoolean(companion::kExpsOptInStatusGrantedPref, true);
+
+  UpdateCompanionDefaultPinnedToToolbarState(&pref_service);
+  EXPECT_EQ(
+      pref_service.GetBoolean(prefs::kSidePanelCompanionEntryPinnedToToolbar),
+      true);
+}
+
+TEST(CompanionUtilsTest, UpdatePinnedStateDefaultPinned) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  TestingPrefServiceSimple pref_service;
+  RegisterPrefs(&pref_service);
+
+  scoped_feature_list.InitAndEnableFeature(
+      ::features::kSidePanelCompanionDefaultPinned);
+  pref_service.SetBoolean(companion::kExpsOptInStatusGrantedPref, false);
+
+  UpdateCompanionDefaultPinnedToToolbarState(&pref_service);
+  EXPECT_EQ(
+      pref_service.GetBoolean(prefs::kSidePanelCompanionEntryPinnedToToolbar),
+      true);
+}
+
+TEST(CompanionUtilsTest, UpdatePinnedStateDefaultUnPinned) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  TestingPrefServiceSimple pref_service;
+  RegisterPrefs(&pref_service);
+
+  scoped_feature_list.InitAndDisableFeature(
+      ::features::kSidePanelCompanionDefaultPinned);
+  pref_service.SetBoolean(companion::kExpsOptInStatusGrantedPref, false);
+
+  UpdateCompanionDefaultPinnedToToolbarState(&pref_service);
+  EXPECT_EQ(
+      pref_service.GetBoolean(prefs::kSidePanelCompanionEntryPinnedToToolbar),
+      false);
+}
+
+}  // namespace companion
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
@@ -7265,6 +7265,7 @@ test("unit_tests") {
       "../browser/ui/search/search_ipc_router_policy_unittest.cc",
       "../browser/ui/search/search_ipc_router_unittest.cc",
       "../browser/ui/serial/serial_chooser_controller_unittest.cc",
+      "../browser/ui/side_panel/companion/companion_utils_unittest.cc",
       "../browser/ui/singleton_tabs_unittest.cc",
       "../browser/ui/startup/launch_mode_recorder_unittest.cc",
       "../browser/ui/sync/sync_promo_ui_unittest.cc",

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -60699,6 +60699,7 @@ from previous Chrome versions.
   <int value="-1113373128" label="WebUIDownloadShelf:enabled"/>
   <int value="-1112782121" label="AndroidSigninPromos:disabled"/>
   <int value="-1111094917" label="Cormorant:enabled"/>
+  <int value="-1110928012" label="force-companion-pinned-state"/>
   <int value="-1109826787" label="AccessibilityExposeDisplayNone:enabled"/>
   <int value="-1109489088" label="LauncherFuzzyMatchForOmnibox:disabled"/>
   <int value="-1109309340" label="ContextualPageActionReaderMode:enabled"/>
@@ -61522,6 +61523,7 @@ from previous Chrome versions.
   <int value="-680589442" label="MacRTL:disabled"/>
   <int value="-679585025" label="CaptureModeSelfieCamera:disabled"/>
   <int value="-679500267" label="UseXpsForPrinting:disabled"/>
+  <int value="-678281465" label="force-companion-pinned-state:enabled"/>
   <int value="-678184617" label="TranslateSubFrames:disabled"/>
   <int value="-677978627" label="ZeroCopyTabCapture:enabled"/>
   <int value="-677938988" label="AssistantConsentSimplifiedText:disabled"/>
@@ -65003,6 +65005,7 @@ from previous Chrome versions.
   <int value="1196670943" label="WebAuthFlowInBrowserTab:enabled"/>
   <int value="1196834473" label="disable-smart-virtual-keyboard"/>
   <int value="1197336536" label="printing-ppd-channel"/>
+  <int value="1197363734" label="force-companion-pinned-state:disabled"/>
   <int value="1198604672" label="DiscardExceptionsImprovements:disabled"/>
   <int value="1198839129" label="OfflinePagesLivePageSharing:enabled"/>
   <int value="1199276782" label="HighEfficiencyMultistateMode:enabled"/>