Skip to content

Commit

Permalink
[iOS] Remove SF Symbol feature flag
Browse files Browse the repository at this point in the history
This CL removes the feature flag, making the function return true.

Bug: 1425175
Change-Id: I96d94e4cbb7ebb33d94872c00c96865621639ed2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4346614
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Auto-Submit: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118279}
  • Loading branch information
Gauthier Ambard authored and Chromium LUCI CQ committed Mar 16, 2023
1 parent a6cf408 commit 688e2bb
Show file tree
Hide file tree
Showing 12 changed files with 3 additions and 88 deletions.
5 changes: 0 additions & 5 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -7168,11 +7168,6 @@
"owners": [ "zentaro", "jimmyxgong", "cros-peripherals@google.com" ],
"expiry_milestone": 120
},
{
"name": "use-sf-symbols",
"owners": [ "ewannpv", "gambard", "bling-flags@google.com" ],
"expiry_milestone": 113
},
{
"name": "use-sf-symbols-omnibox",
"owners": [ "ewannpv", "gambard", "bling-flags@google.com" ],
Expand Down
3 changes: 0 additions & 3 deletions ios/chrome/browser/flags/about_flags.mm
Original file line number Diff line number Diff line change
Expand Up @@ -950,9 +950,6 @@
flag_descriptions::kMediaPermissionsControlName,
flag_descriptions::kMediaPermissionsControlDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(web::features::kMediaPermissionsControl)},
{"use-sf-symbols", flag_descriptions::kUseSFSymbolsName,
flag_descriptions::kUseSFSymbolsDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(kUseSFSymbols)},
{"enable-password-grouping", flag_descriptions::kPasswordsGroupingName,
flag_descriptions::kPasswordsGroupingDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(password_manager::features::kPasswordsGrouping)},
Expand Down
4 changes: 0 additions & 4 deletions ios/chrome/browser/flags/ios_chrome_flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -988,10 +988,6 @@ const char kUseLoadSimulatedRequestForOfflinePageDescription[] =
"When enabled, the offline pages uses the iOS 15 "
"loadSimulatedRequest:responseHTMLString: API";

const char kUseSFSymbolsName[] = "Replace Image by SFSymbols";
const char kUseSFSymbolsDescription[] =
"When enabled, images are replaced by SFSymbols";

const char kUseSFSymbolsInOmniboxName[] =
"Replace Image by SFSymbols in Omnibox";
const char kUseSFSymbolsInOmniboxDescription[] =
Expand Down
5 changes: 0 additions & 5 deletions ios/chrome/browser/flags/ios_chrome_flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -856,11 +856,6 @@ extern const char kTabInactivityThresholdDescription[];
extern const char kUseLoadSimulatedRequestForOfflinePageName[];
extern const char kUseLoadSimulatedRequestForOfflinePageDescription[];

// Title and description for the flag to enable the replacement of images
// by SFSymbols.
extern const char kUseSFSymbolsName[];
extern const char kUseSFSymbolsDescription[];

// Title and description for the flag to enable the replacement of images
// by SFSymbols in omnibox.
extern const char kUseSFSymbolsInOmniboxName[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@
// Tests the SyncErrorInfobarBannerOverlayRequestConfig then SF symbol is
// enabled.
TEST_F(SyncErrorInfobarBannerOverlayRequestConfigTest, IconConfigsForSFSymbol) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kUseSFSymbols);

std::unique_ptr<OverlayRequest> request =
OverlayRequest::CreateWithConfig<SyncErrorBannerRequestConfig>(
infobar_.get());
Expand All @@ -83,24 +80,3 @@
EXPECT_EQ(kMessageText, config->message_text());
EXPECT_EQ(kButtonLabelText, config->button_label_text());
}

// Tests the SyncErrorInfobarBannerOverlayRequestConfig when SF symbol is not
// enabled.
TEST_F(SyncErrorInfobarBannerOverlayRequestConfigTest,
IconConfigsForLegacyAsset) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kUseSFSymbols);

std::unique_ptr<OverlayRequest> request =
OverlayRequest::CreateWithConfig<SyncErrorBannerRequestConfig>(
infobar_.get());
SyncErrorBannerRequestConfig* config =
request->GetConfig<SyncErrorBannerRequestConfig>();

EXPECT_NSEQ(nullptr, config->icon_image_tint_color());
EXPECT_NSEQ(nullptr, config->background_tint_color());
EXPECT_EQ(true, config->use_icon_background_tint());
EXPECT_EQ(kTitleText, config->title_text());
EXPECT_EQ(kMessageText, config->message_text());
EXPECT_EQ(kButtonLabelText, config->button_label_text());
}
2 changes: 0 additions & 2 deletions ios/chrome/browser/shared/public/features/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ BASE_FEATURE(kEnableShortenedPasswordAutoFillInstruction,
"EnableShortenedPasswordAutoFillInstruction",
base::FEATURE_ENABLED_BY_DEFAULT);

BASE_FEATURE(kUseSFSymbols, "UseSFSymbols", base::FEATURE_ENABLED_BY_DEFAULT);

BASE_FEATURE(kUseSFSymbolsInOmnibox,
"UseSFSymbolsInOmnibox",
base::FEATURE_DISABLED_BY_DEFAULT);
Expand Down
3 changes: 0 additions & 3 deletions ios/chrome/browser/shared/public/features/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ BASE_DECLARE_FEATURE(kRemoveExcessNTPs);
// Chrome.
BASE_DECLARE_FEATURE(kEnableShortenedPasswordAutoFillInstruction);

// Feature flag to switch images to SFSymbols when enabled.
BASE_DECLARE_FEATURE(kUseSFSymbols);

// Feature flag to switch images to SFSymbols in the omnibox when enabled.
BASE_DECLARE_FEATURE(kUseSFSymbolsInOmnibox);

Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/browser/ui/icons/symbol_helpers.mm
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
} // namespace

bool UseSymbols() {
return base::FeatureList::IsEnabled(kUseSFSymbols);
return true;
}

bool UseSymbolsInOmnibox() {
Expand Down
4 changes: 1 addition & 3 deletions ios/chrome/browser/ui/menu/action_factory_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@
// Test fixture for the ActionFactory.
class ActionFactoryTest : public PlatformTest {
protected:
ActionFactoryTest() : test_title_(@"SomeTitle") {
feature_list_.InitAndEnableFeature(kUseSFSymbols);
}
ActionFactoryTest() : test_title_(@"SomeTitle") {}

// Creates a blue square.
UIImage* CreateMockImage() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@
protected:
BrowserActionFactoryTest()
: test_title_(@"SomeTitle"),
scene_state_([[SceneState alloc] initWithAppState:nil]) {
feature_list_.InitAndEnableFeature(kUseSFSymbols);
}
scene_state_([[SceneState alloc] initWithAppState:nil]) {}

void SetUp() override {
TestChromeBrowserState::Builder test_cbs_builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ @protocol SyncPresenter;
// consumer's icon using SF symbol.
TEST_F(SyncErrorInfobarBannerOverlayMediatorTest,
SetUpConsumerWithIconSettingsUseSFSymbol) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kUseSFSymbols);

mediator_.consumer = consumer_mock_;
// Verify that the infobar's icon was set up properly.
OCMExpect([consumer_mock_
Expand All @@ -108,17 +105,3 @@ @protocol SyncPresenter;
setIconBackgroundColor:[UIColor colorNamed:kRed500Color]]);
OCMExpect([consumer_mock_ setUseIconBackgroundTint:true]);
}

// Tests that a SyncErrorInfobarBannerOverlayMediator correctly sets up its
// consumer's icon using legacy asset.
TEST_F(SyncErrorInfobarBannerOverlayMediatorTest,
SetUpConsumerWithIconSettingsUseLegacyAsset) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kUseSFSymbols);

mediator_.consumer = consumer_mock_;
// Verify that the infobar's icon was set up properly.
OCMExpect([consumer_mock_ setIconImageTintColor:nullptr]);
OCMExpect([consumer_mock_ setIconBackgroundColor:nullptr]);
OCMExpect([consumer_mock_ setUseIconBackgroundTint:false]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ void SetUp() override {
// Tests that the delegate's icon configurations is correct when UseSymbol is
// enabled.
TEST_F(SyncErrorInfobarDelegateTest, IconConfigsUseSymbol) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kUseSFSymbols);

id presenter = OCMStrictProtocolMock(@protocol(SyncPresenter));
std::unique_ptr<SyncErrorInfoBarDelegate> delegate(
new SyncErrorInfoBarDelegate(chrome_browser_state_.get(), presenter));
Expand All @@ -71,21 +68,6 @@ void SetUp() override {
delegate->GetIcon().GetImage().ToUIImage());
}

// Tests that the delegate's icon configurations is correct when legacy image
// asset is used.
TEST_F(SyncErrorInfobarDelegateTest, IconConfigsNotUseSymbol) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kUseSFSymbols);

id presenter = OCMStrictProtocolMock(@protocol(SyncPresenter));
std::unique_ptr<SyncErrorInfoBarDelegate> delegate(
new SyncErrorInfoBarDelegate(chrome_browser_state_.get(), presenter));

EXPECT_FALSE(delegate->UseIconBackgroundTint());
EXPECT_EQ(nullptr, delegate->GetIconImageTintColor());
EXPECT_EQ(nullptr, delegate->GetIconBackgroundColor());
}

TEST_F(SyncErrorInfobarDelegateTest, SyncServiceSignInNeedsUpdate) {
ON_CALL(*mock_sync_service(), GetUserActionableError())
.WillByDefault(
Expand Down

0 comments on commit 688e2bb

Please sign in to comment.