Skip to content

Commit

Permalink
Revert "Reland "Reland "[CSC] Create chrome flags for Companion UXR."""
Browse files Browse the repository at this point in the history
This reverts commit d20d1b3.

Reason for revert: This has a build issue:

https://bugs.chromium.org/p/chromium/issues/detail?id=1451939


Original change's description:
> 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}

Bug: 1449587
Bug: b:283109243
Change-Id: I37b42be8de8378c2ee1db10a484127ccddae2c9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4593981
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#430}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Michael Crouse authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent ead21ab commit 0329f0e
Show file tree
Hide file tree
Showing 11 changed files with 3 additions and 189 deletions.
32 changes: 2 additions & 30 deletions chrome/browser/about_flags.cc
Expand Up @@ -3240,34 +3240,14 @@ constexpr FeatureEntry::FeatureVariation kLensPingVariations[] = {
};

#if BUILDFLAG(ENABLE_LENS_DESKTOP_GOOGLE_BRANDED_FEATURES)
constexpr FeatureEntry::FeatureParam kCscStagingEnvVariation[] = {
constexpr FeatureEntry::FeatureParam kCscVariation[] = {
{"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", 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"},
};
{"with staging URL", kCscVariation, std::size(kCscVariation), nullptr}};
#endif // BUILDFLAG(ENABLE_LENS_DESKTOP_GOOGLE_BRANDED_FEATURES)

#if BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down Expand Up @@ -8863,14 +8843,6 @@ 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,
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/companion/core/BUILD.gn
Expand Up @@ -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",
Expand Down
22 changes: 0 additions & 22 deletions chrome/browser/companion/core/features.cc
Expand Up @@ -39,32 +39,10 @@ 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
6 changes: 0 additions & 6 deletions chrome/browser/companion/core/features.h
Expand Up @@ -8,7 +8,6 @@
#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 {
Expand All @@ -24,16 +23,11 @@ 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

Expand Down
10 changes: 0 additions & 10 deletions chrome/browser/flag-metadata.json
Expand Up @@ -1327,16 +1327,6 @@
"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" ],
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/flag_descriptions.cc
Expand Up @@ -1993,12 +1993,6 @@ 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[] =
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/flag_descriptions.h
Expand Up @@ -1110,12 +1110,6 @@ 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[];

Expand Down
8 changes: 0 additions & 8 deletions chrome/browser/ui/side_panel/companion/companion_utils.cc
Expand Up @@ -71,14 +71,6 @@ 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) ||
Expand Down
96 changes: 0 additions & 96 deletions chrome/browser/ui/side_panel/companion/companion_utils_unittest.cc

This file was deleted.

1 change: 0 additions & 1 deletion chrome/test/BUILD.gn
Expand Up @@ -7265,7 +7265,6 @@ 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",
Expand Down
3 changes: 0 additions & 3 deletions tools/metrics/histograms/enums.xml
Expand Up @@ -60716,7 +60716,6 @@ 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"/>
Expand Down Expand Up @@ -61540,7 +61539,6 @@ 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"/>
Expand Down Expand Up @@ -65022,7 +65020,6 @@ 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"/>
Expand Down

0 comments on commit 0329f0e

Please sign in to comment.