Clean up reading list and side panel feature flag code.

-Remove usage of read later flag on desktop. -Remove unused read later new badge promo flag and use. -Remove read later add from dialog flag and use (not used with side panel). -Remove side panel flag as the side panel has launched. -Remove side panel flag from chrome labs. -Remove code for stand-alone reading list bubble since it is not used with the side panel. -Remove kShowReadingListInBookmarkBar pref according to instructions at chrome/browser/prefs/README.md Bug: 1141224 Change-Id: I8408926c217df357b80be786a3d590d9cac6b4ce Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3557809 Reviewed-by: Dana Fried <dfried@chromium.org> Reviewed-by: Alex Gough <ajgo@chromium.org> Reviewed-by: Caitlin Fischer <caitlinfischer@google.com> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Mikel Astiz <mastiz@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Commit-Queue: Caroline Rising <corising@chromium.org> Cr-Commit-Position: refs/heads/main@{#987938}
chromium · Apr 1, 2022 · 663480d · 663480d
1 parent 0b89b16
commit 663480d
Show file tree

Hide file tree

Showing 54 changed files with 72 additions and 1,490 deletions.
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -5107,17 +5107,8 @@ const FeatureEntry kFeatureEntries[] = {
      FEATURE_WITH_PARAMS_VALUE_TYPE(reading_list::switches::kReadLater,
                                     kReadLaterVariations,
                                     "Collections")},
-#else
-    {flag_descriptions::kReadLaterFlagId, flag_descriptions::kReadLaterName,
-     flag_descriptions::kReadLaterDescription, kOsDesktop,
-     FEATURE_VALUE_TYPE(reading_list::switches::kReadLater)},
 #endif
 
-    {"read-later-new-badge-promo",
-     flag_descriptions::kReadLaterNewBadgePromoName,
-     flag_descriptions::kReadLaterNewBadgePromoDescription, kOsDesktop,
-     FEATURE_VALUE_TYPE(features::kReadLaterNewBadgePromo)},
-
 #if BUILDFLAG(IS_ANDROID)
     {"read-later-reminder-notification",
      flag_descriptions::kReadLaterReminderNotificationName,
@@ -5156,10 +5147,6 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kScrollableTabStripButtonsDescription, kOsDesktop,
      FEATURE_VALUE_TYPE(features::kScrollableTabStripButtons)},
 
-    {flag_descriptions::kSidePanelFlagId, flag_descriptions::kSidePanelName,
-     flag_descriptions::kSidePanelDescription, kOsDesktop,
-     FEATURE_VALUE_TYPE(features::kSidePanel)},
-
     {flag_descriptions::kSidePanelDragAndDropFlagId,
      flag_descriptions::kSidePanelDragAndDropName,
      flag_descriptions::kSidePanelDragAndDropDescription, kOsDesktop,

diff --git a/chrome/browser/chrome_browser_interface_binders.cc b/chrome/browser/chrome_browser_interface_binders.cc
@@ -884,20 +884,16 @@ void PopulateChromeWebUIFrameBinders(
         task_module::mojom::TaskModuleHandler, NewTabPageUI>(map);
   }
 
-  if (reading_list::switches::IsReadingListEnabled()) {
-    RegisterWebUIControllerInterfaceBinder<
-        read_later::mojom::PageHandlerFactory, ReadLaterUI>(map);
-  }
+  RegisterWebUIControllerInterfaceBinder<read_later::mojom::PageHandlerFactory,
+                                         ReadLaterUI>(map);
 
-  if (base::FeatureList::IsEnabled(features::kSidePanel)) {
-    if (base::FeatureList::IsEnabled(features::kUnifiedSidePanel)) {
-      RegisterWebUIControllerInterfaceBinder<
-          side_panel::mojom::BookmarksPageHandlerFactory, BookmarksSidePanelUI>(
-          map);
-    } else {
-      RegisterWebUIControllerInterfaceBinder<
-          side_panel::mojom::BookmarksPageHandlerFactory, ReadLaterUI>(map);
-    }
+  if (base::FeatureList::IsEnabled(features::kUnifiedSidePanel)) {
+    RegisterWebUIControllerInterfaceBinder<
+        side_panel::mojom::BookmarksPageHandlerFactory, BookmarksSidePanelUI>(
+        map);
+  } else {
+    RegisterWebUIControllerInterfaceBinder<
+        side_panel::mojom::BookmarksPageHandlerFactory, ReadLaterUI>(map);
   }
 
   if (features::IsReadAnythingEnabled()) {

diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
@@ -5031,11 +5031,6 @@
     "owners": [ "chrome-desktop-ui-sea@google.com", "corising", "wylieb", "chrome-collections@google.com" ],
     "expiry_milestone": 102
   },
-  {
-    "name": "read-later-new-badge-promo",
-    "owners": [ "chrome-desktop-ui-sea@google.com", "corising" ],
-    "expiry_milestone": 93
-  },
   {
     "name": "read-later-reminder-notification",
     "owners": [ "wylieb" ],

diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
@@ -2176,11 +2176,6 @@ const char kSettingsAppNotificationSettingsDescription[] =
     "lacros-chrome browser's notification permission page "
     "and the ChromeOS settings app.";
 
-const char kReadLaterNewBadgePromoName[] = "Reading list 'New' badge promo";
-const char kReadLaterNewBadgePromoDescription[] =
-    "Causes a 'New' badge to appear on the entry point for adding to the "
-    "reading list in the tab context menu.";
-
 const char kRecordWebAppDebugInfoName[] = "Record web app debug info";
 const char kRecordWebAppDebugInfoDescription[] =
     "Enables recording additional web app related debugging data to be "
@@ -2274,12 +2269,6 @@ const char kSendTabToSelfSigninPromoDescription[] =
 const char kShoppingListName[] = "Shopping List";
 const char kShoppingListDescription[] = "Enable shopping list in bookmarks.";
 
-const char kSidePanelFlagId[] = "side-panel";
-const char kSidePanelName[] = "Side panel";
-const char kSidePanelDescription[] =
-    "Enables a browser-level side panel for a useful and persistent way to "
-    "access your Reading List and Bookmarks.";
-
 const char kSidePanelDragAndDropFlagId[] = "side-panel-drag-and-drop";
 const char kSidePanelDragAndDropName[] = "Side panel drag and drop";
 const char kSidePanelDragAndDropDescription[] =
@@ -3856,12 +3845,6 @@ const char kOmniboxShortBookmarkSuggestionsDescription[] =
     "Match very short input words to beginning of words in bookmark "
     "suggestions.";
 
-const char kReadLaterFlagId[] = "read-later";
-const char kReadLaterName[] = "Reading List";
-const char kReadLaterDescription[] =
-    "Click on the Bookmark icon or right click on a tab to add tabs to a "
-    "reading list.";
-
 const char kScreenAIName[] = "Screen AI";
 const char kScreenAIDescription[] =
     "Enables Screen AI local machine intelligence library to use the screen "

diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
@@ -1229,9 +1229,6 @@ extern const char kReadLaterFlagId[];
 extern const char kReadLaterName[];
 extern const char kReadLaterDescription[];
 
-extern const char kReadLaterNewBadgePromoName[];
-extern const char kReadLaterNewBadgePromoDescription[];
-
 extern const char kRecordWebAppDebugInfoName[];
 extern const char kRecordWebAppDebugInfoDescription[];
 
@@ -1284,10 +1281,6 @@ extern const char kSendTabToSelfSigninPromoDescription[];
 extern const char kShoppingListName[];
 extern const char kShoppingListDescription[];
 
-extern const char kSidePanelFlagId[];
-extern const char kSidePanelName[];
-extern const char kSidePanelDescription[];
-
 extern const char kSidePanelDragAndDropFlagId[];
 extern const char kSidePanelDragAndDropName[];
 extern const char kSidePanelDragAndDropDescription[];

diff --git a/chrome/browser/metrics/desktop_platform_features_metrics_provider.cc b/chrome/browser/metrics/desktop_platform_features_metrics_provider.cc
@@ -15,7 +15,6 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/read_later/reading_list_model_factory.h"
 #include "components/reading_list/core/reading_list_model.h"
-#include "components/reading_list/features/reading_list_switches.h"
 #include "ui/native_theme/native_theme.h"
 
 namespace {
@@ -55,18 +54,16 @@ void DesktopPlatformFeaturesMetricsProvider::ProvideCurrentSessionData(
   UMA_HISTOGRAM_ENUMERATION("Browser.DarkModeStatus", status);
 
   // Record how many items are in the reading list.
-  if (reading_list::switches::IsReadingListEnabled()) {
-    std::vector<Profile*> profiles =
-        g_browser_process->profile_manager()->GetLoadedProfiles();
-    for (Profile* profile : profiles) {
-      ReadingListModel* model =
-          ReadingListModelFactory::GetForBrowserContext(profile);
-      if (model && model->loaded()) {
-        UMA_HISTOGRAM_COUNTS_1000("ReadingList.Unread.Count.OnUMAUpload",
-                                  model->unread_size());
-        UMA_HISTOGRAM_COUNTS_1000("ReadingList.Read.Count.OnUMAUpload",
-                                  model->size() - model->unread_size());
-      }
+  std::vector<Profile*> profiles =
+      g_browser_process->profile_manager()->GetLoadedProfiles();
+  for (Profile* profile : profiles) {
+    ReadingListModel* model =
+        ReadingListModelFactory::GetForBrowserContext(profile);
+    if (model && model->loaded()) {
+      UMA_HISTOGRAM_COUNTS_1000("ReadingList.Unread.Count.OnUMAUpload",
+                                model->unread_size());
+      UMA_HISTOGRAM_COUNTS_1000("ReadingList.Read.Count.OnUMAUpload",
+                                model->size() - model->unread_size());
     }
   }
 

diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
@@ -743,6 +743,7 @@ const char kStabilityLaunchCount[] =
 #endif
 const char kStabilityExtensionRendererLaunchCount[] =
     "user_experience_metrics.stability.extension_renderer_launch_count";
+const char kShowReadingListInBookmarkBar[] = "bookmark_bar.show_reading_list";
 
 // Register local state used only for migration (clearing or moving to a new
 // key).
@@ -967,6 +968,8 @@ void RegisterProfilePrefsForMigration(
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   registry->RegisterIntegerPref(kPhoneHubCameraRollPendingStatePrefName, 0);
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
+  registry->RegisterBooleanPref(kShowReadingListInBookmarkBar, true);
 }
 
 }  // namespace
@@ -1910,6 +1913,9 @@ void MigrateObsoleteProfilePrefs(Profile* profile) {
   crostini::RemoveDuplicateContainerEntries(profile_prefs);
 #endif
 
+  // Added 03/2022
+  profile_prefs->ClearPref(kShowReadingListInBookmarkBar);
+
   // Please don't delete the following line. It is used by PRESUBMIT.py.
   // END_MIGRATE_OBSOLETE_PROFILE_PREFS
 

diff --git a/chrome/browser/prefs/pref_service_incognito_allowlist.cc b/chrome/browser/prefs/pref_service_incognito_allowlist.cc
@@ -88,7 +88,6 @@ const char* const kPersistentPrefNames[] = {
     bookmarks::prefs::kManagedBookmarks,
     bookmarks::prefs::kManagedBookmarksFolderName,
     bookmarks::prefs::kShowAppsShortcutInBookmarkBar,
-    bookmarks::prefs::kShowReadingListInBookmarkBar,
     bookmarks::prefs::kShowManagedBookmarksInBookmarkBar,
     bookmarks::prefs::kShowBookmarkBar,
 #if BUILDFLAG(IS_ANDROID)

diff --git a/chrome/browser/resources/read_later/app.ts b/chrome/browser/resources/read_later/app.ts
@@ -172,18 +172,11 @@ export class ReadLaterAppElement extends PolymerElement {
         item.displayTimeSinceUpdate}`;
   }
 
-  private shouldShowCurrentPageActionButton_(): boolean {
-    return loadTimeData.getBoolean('currentPageActionButtonEnabled');
-  }
-
   /**
    * @return The appropriate text for the empty state subheader
    */
   private getEmptyStateSubheaderText_(): string {
-    if (this.shouldShowCurrentPageActionButton_()) {
-      return loadTimeData.getString('emptyStateAddFromDialogSubheader');
-    }
-    return loadTimeData.getString('emptyStateSubheader');
+    return loadTimeData.getString('emptyStateAddFromDialogSubheader');
   }
 
   /**

diff --git a/chrome/browser/sync/test/integration/local_sync_test.cc b/chrome/browser/sync/test/integration/local_sync_test.cc
@@ -17,7 +17,6 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/browser_sync/browser_sync_switches.h"
-#include "components/reading_list/features/reading_list_switches.h"
 #include "components/sync/base/command_line_switches.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/driver/sync_service_impl.h"
@@ -95,11 +94,12 @@ IN_PROC_BROWSER_TEST_F(LocalSyncTest, ShouldStart) {
   // TODO(crbug.com/1109640): Consider whether all of these types should really
   // be enabled in Local Sync mode.
   syncer::ModelTypeSet expected_active_data_types = syncer::ModelTypeSet(
-      syncer::BOOKMARKS, syncer::PREFERENCES, syncer::PASSWORDS,
-      syncer::AUTOFILL_PROFILE, syncer::AUTOFILL, syncer::AUTOFILL_WALLET_DATA,
-      syncer::AUTOFILL_WALLET_METADATA, syncer::THEMES, syncer::TYPED_URLS,
-      syncer::EXTENSIONS, syncer::SEARCH_ENGINES, syncer::SESSIONS,
-      syncer::APPS, syncer::APP_SETTINGS, syncer::EXTENSION_SETTINGS,
+      syncer::BOOKMARKS, syncer::READING_LIST, syncer::PREFERENCES,
+      syncer::PASSWORDS, syncer::AUTOFILL_PROFILE, syncer::AUTOFILL,
+      syncer::AUTOFILL_WALLET_DATA, syncer::AUTOFILL_WALLET_METADATA,
+      syncer::THEMES, syncer::TYPED_URLS, syncer::EXTENSIONS,
+      syncer::SEARCH_ENGINES, syncer::SESSIONS, syncer::APPS,
+      syncer::APP_SETTINGS, syncer::EXTENSION_SETTINGS,
       syncer::HISTORY_DELETE_DIRECTIVES, syncer::DEVICE_INFO,
       syncer::PRIORITY_PREFERENCES, syncer::WEB_APPS, syncer::PROXY_TABS,
       syncer::NIGORI);
@@ -114,10 +114,6 @@ IN_PROC_BROWSER_TEST_F(LocalSyncTest, ShouldStart) {
   expected_active_data_types.Put(syncer::DICTIONARY);
 #endif
 
-  if (base::FeatureList::IsEnabled(reading_list::switches::kReadLater)) {
-    expected_active_data_types.Put(syncer::READING_LIST);
-  }
-
   EXPECT_EQ(service->GetActiveDataTypes(), expected_active_data_types);
 
   // Verify certain features are disabled.

diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
@@ -4384,8 +4384,6 @@ static_library("ui") {
       "views/location_bar/permission_request_chip.h",
       "views/location_bar/selected_keyword_view.cc",
       "views/location_bar/selected_keyword_view.h",
-      "views/location_bar/star_menu_model.cc",
-      "views/location_bar/star_menu_model.h",
       "views/location_bar/star_view.cc",
       "views/location_bar/star_view.h",
       "views/location_bar/zoom_bubble_view.cc",
@@ -4617,8 +4615,6 @@ static_library("ui") {
       "views/qrcode_generator/qrcode_generator_bubble.h",
       "views/qrcode_generator/qrcode_generator_icon_view.cc",
       "views/qrcode_generator/qrcode_generator_icon_view.h",
-      "views/read_later/read_later_button.cc",
-      "views/read_later/read_later_button.h",
       "views/reader_mode/reader_mode_icon_view.cc",
       "views/reader_mode/reader_mode_icon_view.h",
       "views/relaunch_notification/relaunch_notification_controller.cc",

diff --git a/chrome/browser/ui/bookmarks/bookmark_context_menu_controller.cc b/chrome/browser/ui/bookmarks/bookmark_context_menu_controller.cc
@@ -35,7 +35,6 @@
 #include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/bookmarks/managed/managed_bookmark_service.h"
 #include "components/prefs/pref_service.h"
-#include "components/reading_list/features/reading_list_switches.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/tab_groups/tab_group_visual_data.h"
 #include "components/undo/bookmark_undo_service.h"
@@ -109,20 +108,6 @@ const UserMetricsAction* GetActionForLocationAndDisposition(
   }
 }
 
-// Returns true if |command_id| corresponds to a command related to bookmark bar
-// management.
-bool IsBookmarkBarManagementCommand(int command_id) {
-  switch (command_id) {
-    case IDC_BOOKMARK_MANAGER:
-    case IDC_BOOKMARK_BAR_SHOW_APPS_SHORTCUT:
-    case IDC_BOOKMARK_BAR_SHOW_READING_LIST:
-    case IDC_BOOKMARK_BAR_SHOW_MANAGED_BOOKMARKS:
-    case IDC_BOOKMARK_BAR_ALWAYS_SHOW:
-      return true;
-  }
-  return false;
-}
-
 }  // namespace
 
 BookmarkContextMenuController::BookmarkContextMenuController(
@@ -215,11 +200,6 @@ void BookmarkContextMenuController::BuildMenu() {
     AddCheckboxItem(IDC_BOOKMARK_BAR_SHOW_APPS_SHORTCUT,
                     IDS_BOOKMARK_BAR_SHOW_APPS_SHORTCUT);
   }
-  if (reading_list::switches::IsReadingListEnabled() &&
-      !base::FeatureList::IsEnabled(features::kSidePanel)) {
-    AddCheckboxItem(IDC_BOOKMARK_BAR_SHOW_READING_LIST,
-                    IDS_BOOKMARK_BAR_SHOW_READING_LIST);
-  }
   AddCheckboxItem(IDC_BOOKMARK_BAR_SHOW_MANAGED_BOOKMARKS,
                   IDS_BOOKMARK_BAR_SHOW_MANAGED_BOOKMARKS_DEFAULT_NAME);
   AddCheckboxItem(IDC_BOOKMARK_BAR_ALWAYS_SHOW, IDS_SHOW_BOOKMARK_BAR);
@@ -364,14 +344,6 @@ void BookmarkContextMenuController::ExecuteCommand(int id, int event_flags) {
       break;
     }
 
-    case IDC_BOOKMARK_BAR_SHOW_READING_LIST: {
-      PrefService* prefs = profile_->GetPrefs();
-      prefs->SetBoolean(
-          bookmarks::prefs::kShowReadingListInBookmarkBar,
-          !prefs->GetBoolean(bookmarks::prefs::kShowReadingListInBookmarkBar));
-      break;
-    }
-
     case IDC_BOOKMARK_BAR_SHOW_MANAGED_BOOKMARKS: {
       PrefService* prefs = profile_->GetPrefs();
       prefs->SetBoolean(
@@ -460,9 +432,6 @@ bool BookmarkContextMenuController::IsCommandIdChecked(int command_id) const {
     return prefs->GetBoolean(
         bookmarks::prefs::kShowManagedBookmarksInBookmarkBar);
   }
-  if (command_id == IDC_BOOKMARK_BAR_SHOW_READING_LIST) {
-    return prefs->GetBoolean(bookmarks::prefs::kShowReadingListInBookmarkBar);
-  }
 
   DCHECK_EQ(IDC_BOOKMARK_BAR_SHOW_APPS_SHORTCUT, command_id);
   return prefs->GetBoolean(bookmarks::prefs::kShowAppsShortcutInBookmarkBar);
@@ -471,12 +440,6 @@ bool BookmarkContextMenuController::IsCommandIdChecked(int command_id) const {
 bool BookmarkContextMenuController::IsCommandIdEnabled(int command_id) const {
   PrefService* prefs = profile_->GetPrefs();
 
-  // If the context menu is being shown from the reading list button then only
-  // the bookmark bar management options should be enabled.
-  if (!parent_ && selection_.empty()) {
-    return IsBookmarkBarManagementCommand(command_id);
-  }
-
   bool is_root_node = selection_.size() == 1 &&
                       selection_[0]->parent() == model_->root_node();
   bool can_edit = prefs->GetBoolean(bookmarks::prefs::kEditBookmarksEnabled) &&