ash: Remove expired flag BirchWeather/birch-weather

The flag is enabled by default and there are no Finch experiments
controlling it. Remove the feature flag, the about:flags entry and
clean up the metadata.json file.

Delete some tests that disable weather. These tests have complimentary
versions with weather enabled, so we aren't losing coverage.

Bug: 394308595
Change-Id: If3bb53841f4a999ffcacd67727f28f59594aa6d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6227644
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@google.com>
Cr-Commit-Position: refs/heads/main@{#1415759}

ash/birch/birch_model.cc

@@ -68,10 +68,8 @@ BirchModel::BirchModel()
       release_notes_data_(prefs::kBirchUseReleaseNotes, "ReleaseNotes"),
       weather_data_(prefs::kBirchUseWeather, "Weather"),
       coral_data_(prefs::kBirchUseCoral, "Coral"),
-      icon_cache_(std::make_unique<BirchIconCache>()) {
-  if (features::IsBirchWeatherEnabled()) {
-    weather_provider_ = std::make_unique<BirchWeatherProvider>(this);
-  }
+      icon_cache_(std::make_unique<BirchIconCache>()),
+      weather_provider_(std::make_unique<BirchWeatherProvider>(this)) {
   if (features::IsCoralFeatureEnabled()) {
     auto coral_provider = std::make_unique<BirchCoralProvider>();
     coral_provider->AddObserver(this);

ash/birch/birch_model_unittest.cc

@@ -143,8 +143,8 @@ class BirchModelTest : public AshTestBase {
   BirchModelTest()
       : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
     feature_list_.InitWithFeatures(
-        {features::kForestFeature, features::kBirchWeather,
-         features::kBirchVideoConferenceSuggestions, features::kCoralFeature},
+        {features::kForestFeature, features::kBirchVideoConferenceSuggestions,
+         features::kCoralFeature},
         {});
   }
 
@@ -204,34 +204,6 @@ class BirchModelTest : public AshTestBase {
   base::SimpleTestClock test_clock_;
 };
 
-class BirchModelWithoutWeatherTest : public AshTestBase {
- public:
-  BirchModelWithoutWeatherTest()
-      : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
-    feature_list_.InitWithFeatures({features::kForestFeature},
-                                   {features::kBirchWeather});
-  }
-  void SetUp() override {
-    AshTestBase::SetUp();
-    Shell::Get()->birch_model()->SetClientAndInit(&stub_birch_client_);
-    base::RunLoop run_loop;
-    Shell::Get()
-        ->birch_model()
-        ->GetItemRemoverForTest()
-        ->SetProtoInitCallbackForTest(run_loop.QuitClosure());
-    run_loop.Run();
-  }
-
-  void TearDown() override {
-    Shell::Get()->birch_model()->SetClientAndInit(nullptr);
-    AshTestBase::TearDown();
-  }
-
- protected:
-  base::test::ScopedFeatureList feature_list_;
-  StubBirchClient stub_birch_client_;
-};
-
 // Test that requesting data and adding all fresh items to the model will run
 // the callback.
 TEST_F(BirchModelTest, AddItemNotifiesCallback) {
@@ -820,57 +792,6 @@ TEST_F(BirchModelTest, MAYBE_DataFetchTimeout) {
   EXPECT_FALSE(model->IsDataFresh());
 }
 
-TEST_F(BirchModelWithoutWeatherTest, MAYBE_DataFetchTimeout) {
-  BirchModel* model = Shell::Get()->birch_model();
-  TestModelConsumer consumer;
-  EXPECT_TRUE(model);
-
-  // Passing time and setting data before requesting a birch data fetch will
-  // not notify consumer.
-  task_environment()->FastForwardBy(base::Milliseconds(1000));
-  model->SetRecentTabItems(std::vector<BirchTabItem>());
-  model->SetLastActiveItems({});
-  model->SetMostVisitedItems({});
-  model->SetSelfShareItems(std::vector<BirchSelfShareItem>());
-  model->SetLostMediaItems({});
-  model->SetFileSuggestItems(MakeFileItemList(/*item_count=*/1));
-  model->SetCalendarItems({});
-  model->SetAttachmentItems({});
-  model->SetReleaseNotesItems({});
-  model->SetCoralItems({});
-
-  EXPECT_TRUE(model->IsDataFresh());
-  EXPECT_THAT(consumer.items_ready_responses(), testing::IsEmpty());
-
-  model->RequestBirchDataFetch(/*is_post_login=*/false,
-                               base::BindOnce(&TestModelConsumer::OnItemsReady,
-                                              base::Unretained(&consumer),
-                                              /*id=*/"0"));
-  EXPECT_FALSE(model->IsDataFresh());
-  EXPECT_THAT(consumer.items_ready_responses(), testing::IsEmpty());
-
-  // Test that passing a short amount of time and setting some data does not
-  // notify that items are ready.
-  task_environment()->FastForwardBy(base::Milliseconds(500));
-  std::vector<BirchTabItem> tab_item_list;
-  tab_item_list.emplace_back(u"tab title", GURL("example.com"),
-                             base::Time::Now(), GURL("example.com/favicon_url"),
-                             "session_name",
-                             BirchTabItem::DeviceFormFactor::kDesktop);
-  model->SetRecentTabItems(tab_item_list);
-  EXPECT_THAT(consumer.items_ready_responses(), testing::IsEmpty());
-
-  // Test that passing enough time notifies that items are ready.
-  task_environment()->FastForwardBy(base::Milliseconds(500));
-  EXPECT_THAT(consumer.items_ready_responses(), testing::ElementsAre("0"));
-
-  std::vector<std::unique_ptr<BirchItem>> all_items = model->GetAllItems();
-  EXPECT_EQ(all_items.size(), 2u);
-  EXPECT_EQ(all_items[0]->GetType(), BirchItemType::kTab);
-  EXPECT_EQ(all_items[1]->GetType(), BirchItemType::kFile);
-  EXPECT_FALSE(model->IsDataFresh());
-}
-
 // Test that the data fetch timeout is longer when requesting directly after
 // login.
 TEST_F(BirchModelTest, PostLoginDataFetchTimeout) {
@@ -934,65 +855,6 @@ TEST_F(BirchModelTest, PostLoginDataFetchTimeout) {
   EXPECT_FALSE(model->IsDataFresh());
 }
 
-TEST_F(BirchModelWithoutWeatherTest, AddItemNotifiesCallback) {
-  BirchModel* model = Shell::Get()->birch_model();
-  TestModelConsumer consumer;
-  EXPECT_TRUE(model);
-
-  // Setting items in the model does not notify when no request has occurred.
-  model->SetRecentTabItems(std::vector<BirchTabItem>());
-  model->SetLastActiveItems({});
-  model->SetMostVisitedItems(std::vector<BirchMostVisitedItem>());
-  model->SetSelfShareItems(std::vector<BirchSelfShareItem>());
-  model->SetLostMediaItems(std::vector<BirchLostMediaItem>());
-  model->SetFileSuggestItems(std::vector<BirchFileItem>());
-  EXPECT_THAT(consumer.items_ready_responses(), testing::IsEmpty());
-
-  // Make a data fetch request and set fresh tab data.
-  model->RequestBirchDataFetch(/*is_post_login=*/false,
-                               base::BindOnce(&TestModelConsumer::OnItemsReady,
-                                              base::Unretained(&consumer),
-                                              /*id=*/"0"));
-  model->SetRecentTabItems(std::vector<BirchTabItem>());
-  model->SetLastActiveItems({});
-  model->SetMostVisitedItems(std::vector<BirchMostVisitedItem>());
-  model->SetSelfShareItems(std::vector<BirchSelfShareItem>());
-  model->SetLostMediaItems(std::vector<BirchLostMediaItem>());
-  // Consumer is not notified until all data sources have responded.
-  EXPECT_THAT(consumer.items_ready_responses(), testing::IsEmpty());
-
-  model->SetFileSuggestItems(MakeFileItemList(/*item_count=*/1));
-  model->SetWeatherItems({});
-  model->SetCalendarItems({});
-  model->SetAttachmentItems({});
-  model->SetReleaseNotesItems({});
-
-  // Adding file items sets all data as fresh, notifying consumers.
-  EXPECT_THAT(consumer.items_ready_responses(), testing::ElementsAre("0"));
-
-  // Setting the file suggest items should not trigger items ready again, since
-  // no data fetch was requested.
-  model->SetFileSuggestItems(MakeFileItemList(/*item_count=*/2));
-  EXPECT_THAT(consumer.items_ready_responses(), testing::ElementsAre("0"));
-
-  // Request another data fetch and expect the consumer to be notified once
-  // items are set again.
-  model->RequestBirchDataFetch(/*is_post_login=*/false,
-                               base::BindOnce(&TestModelConsumer::OnItemsReady,
-                                              base::Unretained(&consumer),
-                                              /*id=*/"1"));
-  model->SetRecentTabItems(std::vector<BirchTabItem>());
-  model->SetLastActiveItems({});
-  model->SetMostVisitedItems(std::vector<BirchMostVisitedItem>());
-  model->SetSelfShareItems(std::vector<BirchSelfShareItem>());
-  model->SetLostMediaItems(std::vector<BirchLostMediaItem>());
-  model->SetFileSuggestItems(MakeFileItemList(/*item_count=*/2));
-  model->SetCalendarItems({});
-  model->SetAttachmentItems({});
-  model->SetReleaseNotesItems({});
-  EXPECT_THAT(consumer.items_ready_responses(), testing::ElementsAre("0", "1"));
-}
-
 TEST_F(BirchModelTest, MultipleRequestsHaveIndependentTimeouts) {
   BirchModel* model = Shell::Get()->birch_model();
   TestModelConsumer consumer;

ash/birch/birch_weather_provider_unittest.cc

@@ -39,8 +39,7 @@ BirchWeatherProvider* GetWeatherProvider() {
 class BirchWeatherProviderTest : public AshTestBase {
  public:
   BirchWeatherProviderTest() : clock_override_(&GetTestTime, nullptr, nullptr) {
-    feature_list_.InitWithFeatures(
-        {features::kForestFeature, features::kBirchWeather}, {});
+    feature_list_.InitAndEnableFeature(features::kForestFeature);
     // Ensure the time is morning (7 AM) so weather will be fetched.
     SetTestTime(base::Time::Now().LocalMidnight() + base::Hours(7));
   }

ash/constants/ash_features.cc

@@ -176,9 +176,6 @@ BASE_FEATURE(kBatterySaverAlwaysOn,
              "CrosBatterySaverAlwaysOn",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
-// Display weather information in birch UI.
-BASE_FEATURE(kBirchWeather, "BirchWeather", base::FEATURE_ENABLED_BY_DEFAULT);
-
 // Enables or disables the usage of fixed Bluetooth A2DP packet size to improve
 // audio performance in noisy environment.
 BASE_FEATURE(kBluetoothFixA2dpPacketSize,
@@ -3328,10 +3325,6 @@ bool IsBatterySaverAlwaysOn() {
   return base::FeatureList::IsEnabled(kBatterySaverAlwaysOn);
 }
 
-bool IsBirchWeatherEnabled() {
-  return base::FeatureList::IsEnabled(kBirchWeather);
-}
-
 bool IsBluetoothQualityReportEnabled() {
   return base::FeatureList::IsEnabled(kBluetoothQualityReport);
 }

ash/constants/ash_features.h

@@ -54,7 +54,6 @@ COMPONENT_EXPORT(ASH_CONSTANTS)
 BASE_DECLARE_FEATURE(kAutozoomNudgeSessionReset);
 COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kBabelOrca);
 COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kBatterySaver);
-COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kBirchWeather);
 COMPONENT_EXPORT(ASH_CONSTANTS)
 BASE_DECLARE_FEATURE(kBirchVideoConferenceSuggestions);
 enum BatterySaverNotificationBehavior { kBSMAutoEnable, kBSMOptIn };
@@ -1032,7 +1031,6 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBabelOrcaAvailable();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBatterySaverAvailable();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBatterySaverAlwaysOn();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBirchVideoConferenceSuggestionsEnabled();
-COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBirchWeatherEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBluetoothQualityReportEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBocaEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsBocaConsumerEnabled();

ash/wm/overview/birch/birch_bar_pixeltest.cc

@@ -135,8 +135,7 @@ class BirchBarPixelTest : public AshTestBase,
                           public testing::WithParamInterface<TestParams> {
  public:
   BirchBarPixelTest() {
-    scoped_feature_list_.InitWithFeatures(
-        {features::kForestFeature, features::kBirchWeather}, {});
+    scoped_feature_list_.InitAndEnableFeature(features::kForestFeature);
   }
 
   // AshTestBase:

ash/wm/overview/birch/birch_bar_unittest.cc

@@ -119,9 +119,7 @@ class BirchBarTestBase : public AshTestBase {
  public:
   BirchBarTestBase() {
     feature_list_.InitWithFeatures(
-        {features::kForestFeature, features::kBirchWeather,
-         features::kCoralFeature},
-        {});
+        {features::kForestFeature, features::kCoralFeature}, {});
   }
 
   BirchBarTestBase(const BirchBarTestBase&) = delete;

chrome/browser/about_flags.cc

@@ -10682,9 +10682,6 @@ const FeatureEntry kFeatureEntries[] = {
     {"ash-forest-feature", flag_descriptions::kForestFeatureName,
      flag_descriptions::kForestFeatureDescription, kOsCrOS,
      FEATURE_VALUE_TYPE(ash::features::kForestFeature)},
-    {"birch-weather", flag_descriptions::kBirchWeatherName,
-     flag_descriptions::kBirchWeatherDescription, kOsCrOS,
-     FEATURE_VALUE_TYPE(ash::features::kBirchWeather)},
 #endif  // BUILDFLAG(IS_CHROMEOS)
 
     {"enable-standard-device-bound-session-credentials",

chrome/browser/flag-metadata.json

@@ -915,11 +915,6 @@
     "owners": [ "ioanap@chromium.org", "izuzic@google.com"],
     "expiry_milestone": 125
   },
-  {
-    "name": "birch-weather",
-    "owners": [ "jamescook@chromium.org", "chromeos-launcher@google.com" ],
-    "expiry_milestone": 135
-  },
   {
     "name": "blink-extension",
     "owners": ["ortuno@chromium.org", "cros-apps-platform-core@google.com" ],

chrome/browser/flag_descriptions.cc

@@ -183,14 +183,6 @@ const char kBiometricAuthIdentityCheckDescription[] =
     "biometric checks.";
 #endif  // BUILDFLAG(IS_ANDROID)
 
-#if BUILDFLAG(IS_CHROMEOS)
-const char kBirchWeatherName[] = "Birch weather";
-const char kBirchWeatherDescription[] =
-    "Shows the weather suggestion chip in the birch suggestions UI. Note that "
-    "the chip only shows certain times of day, so it may not immediately "
-    "appear after enabling this feature.";
-#endif  // BUILDFLAG(IS_CHROMEOS)
-
 const char kBlinkSchedulerDiscreteInputMatchesResponsivenessMetricsName[] =
     "Blink Scheduler Discrete Input Matches Responsiveness Metrics";
 const char

chrome/browser/flag_descriptions.h

@@ -137,9 +137,6 @@ extern const char kBookmarksTreeViewDescription[];
 #endif
 
 #if BUILDFLAG(IS_CHROMEOS)
-extern const char kBirchWeatherName[];
-extern const char kBirchWeatherDescription[];
-
 extern const char kCampaignsComponentUpdaterTestTagName[];
 extern const char kCampaignsComponentUpdaterTestTagDescription[];
 extern const char kCampaignsOverrideName[];

chrome/browser/ui/ash/birch/birch_browsertest.cc

@@ -256,8 +256,7 @@ void ClickOnView(views::View* target_view) {
 class BirchBrowserTest : public InProcessBrowserTest {
  public:
   BirchBrowserTest() {
-    feature_list_.InitWithFeatures(
-        {features::kForestFeature, features::kBirchWeather}, {});
+    feature_list_.InitAndEnableFeature(features::kForestFeature);
   }
   ~BirchBrowserTest() override = default;
   BirchBrowserTest(const BirchBrowserTest&) = delete;