Skip to content

Commit

Permalink
No longer need to explicitly remove history clusters menu item
Browse files Browse the repository at this point in the history
Upstream was modified to remove the history clusters menu item when the feature
is disabled, so we no longer need to do this ourselves.

Chromium change:
https://source.chromium.org/chromium/chromium/src/+/4821730fe67e2284cb9bcb0e0d2038aa41c0dcbe

commit 4821730fe67e2284cb9bcb0e0d2038aa41c0dcbe
Author: Erik Chen <[email protected]>
Date:   Tue Oct 22 21:20:55 2024 +0000

    Don't show history clusters menu item if history clusters is disabled.

    Prior to this CL, history clusters could be disabled but Chrome would
    continue to show the history clusters menu item.

    Bug: 374013418
  • Loading branch information
emerick authored and cdesouza-chromium committed Oct 25, 2024
1 parent 6c56efb commit 8f0de9e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,9 @@ constexpr char kBraveSyncedTabsUrl[] = "brave://history/syncedTabs";
BraveRecentTabsSubMenuModel::BraveRecentTabsSubMenuModel(
ui::AcceleratorProvider* accelerator_provider,
Browser* browser)
: RecentTabsSubMenuModel(accelerator_provider, browser) {
// We disable history clusters feature, so this command won't work.
std::optional<size_t> show_history_clusters_index =
GetIndexOfCommandId(IDC_SHOW_HISTORY_CLUSTERS_SIDE_PANEL);
CHECK(show_history_clusters_index);
RemoveItemAt(show_history_clusters_index.value());
}
: RecentTabsSubMenuModel(accelerator_provider, browser) {}

BraveRecentTabsSubMenuModel::~BraveRecentTabsSubMenuModel() {}
BraveRecentTabsSubMenuModel::~BraveRecentTabsSubMenuModel() = default;

void BraveRecentTabsSubMenuModel::ExecuteCommand(int command_id,
int event_flags) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,25 @@
#define RecentlyClosedGroupsFromCurrentSession \
DISABLED_RecentlyClosedGroupsFromCurrentSession

// Disabling these tests because they reference items in menu explicitly by
// index which doesn't match our menu since we insert an additional "Clear
// browsing data" item ahead of the entries of interest to the test.
// Disabling this test because it references items in menu explicitly by index
// which doesn't match our menu since we insert an additional "Clear browsing
// data" item ahead of the entries of interest to the test.
#define MaxSessionsAndRecency DISABLED_MaxSessionsAndRecency

// Disabling these tests because our "More..." item doesn't match the test's
// Disabling this test because our "More..." item doesn't match the test's
// expectation of the tab name.
#define MaxTabsPerSessionAndRecency DISABLED_MaxTabsPerSessionAndRecency

// Disabling this because we have refresh disabled, but it fails because
// ExtensionWebContentsObserver::GetForWebContents returns nullptr and makes
// the test crash.
// Disabling this test because we have refresh disabled, but it fails because
// ExtensionWebContentsObserver::GetForWebContents returns nullptr and makes the
// test crash.
#define RecentlyClosedTabsAndWindowsFromLastSessionWithRefresh \
DISABLED_RecentlyClosedTabsAndWindowsFromLastSessionWithRefresh

// Disabling this test because we disable history clusters
#define LogMenuMetricsForShowGroupedHistory \
DISABLED_LogMenuMetricsForShowGroupedHistory

#define BRAVE_RECENT_TABS_SUB_MENU_MODEL_TEST \
void VerifyModel(const RecentTabsSubMenuModel& model, \
base::span<const ModelData> data); \
Expand Down Expand Up @@ -63,6 +67,9 @@ void RecentTabsSubMenuModelTest::VerifyModel(
// We have to copy it over as we can not modify the input.
auto data = base::ToVector(input);

// We disable history clusters, so remove it as the second item
data.erase(std::next(data.begin()));

// We replace the "Sign in to see tabs from other devices" menu command with
// the non-command string "No tabs from other devices" and need to adjust the
// data
Expand Down

0 comments on commit 8f0de9e

Please sign in to comment.