Commit 9572b02d authored by Caroline Rising's avatar Caroline Rising Committed by Commit Bot

Add read later entry point in tab context menus.

This change is behind the kReadLater feature flag. Add new tab context
menu option for adding one or more tabs to read later.

Bug: 1131030
Change-Id: Id902a40ccb3c3bc86a28fc8b9aa5ab7d92878896
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426343
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: default avatarConnie Wan <connily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810036}
parent 45edf801
...@@ -6612,6 +6612,11 @@ the Bookmarks menu."> ...@@ -6612,6 +6612,11 @@ the Bookmarks menu.">
<message name="IDS_TAB_CXMENU_SOUND_UNMUTE_SITE" desc="The label of the tab context menu item for unmuting one or more sites. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]"> <message name="IDS_TAB_CXMENU_SOUND_UNMUTE_SITE" desc="The label of the tab context menu item for unmuting one or more sites. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]">
{NUM_TABS, plural, =1 {Unmute site} other {Unmute sites}} {NUM_TABS, plural, =1 {Unmute site} other {Unmute sites}}
</message> </message>
<message name="IDS_TAB_CXMENU_READ_LATER" desc="The label of the tab context menu item for adding one or more tabs to Read later.">
{NUM_TABS, plural,
=1 {Read tab later}
other {Read tabs later}}
</message>
<message name="IDS_TAB_CXMENU_ADD_TAB_TO_GROUP" desc="The label of the tab context menu submenu for adding one or more tabs to either a new group or an existing tab group. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]"> <message name="IDS_TAB_CXMENU_ADD_TAB_TO_GROUP" desc="The label of the tab context menu submenu for adding one or more tabs to either a new group or an existing tab group. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]">
{NUM_TABS, plural, {NUM_TABS, plural,
=1 {Add tab to group} =1 {Add tab to group}
...@@ -6679,6 +6684,11 @@ the Bookmarks menu."> ...@@ -6679,6 +6684,11 @@ the Bookmarks menu.">
<message name="IDS_TAB_CXMENU_SOUND_UNMUTE_SITE" desc="In Title Case: The label of the tab context menu item for unmuting one or more sites. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]"> <message name="IDS_TAB_CXMENU_SOUND_UNMUTE_SITE" desc="In Title Case: The label of the tab context menu item for unmuting one or more sites. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]">
{NUM_TABS, plural, =1 {Unmute Site} other {Unmute Sites}} {NUM_TABS, plural, =1 {Unmute Site} other {Unmute Sites}}
</message> </message>
<message name="IDS_TAB_CXMENU_READ_LATER" desc="In Title Case: The label of the tab context menu item for adding one or more tabs to Read later.">
{NUM_TABS, plural,
=1 {Read Tab Later}
other {Read Tabs Later}}
</message>
<message name="IDS_TAB_CXMENU_ADD_TAB_TO_GROUP" desc="In Title Case: The label of the tab context menu submenu for adding one or more tabs to either a new group or an existing tab group. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]"> <message name="IDS_TAB_CXMENU_ADD_TAB_TO_GROUP" desc="In Title Case: The label of the tab context menu submenu for adding one or more tabs to either a new group or an existing tab group. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]">
{NUM_TABS, plural, {NUM_TABS, plural,
=1 {Add Tab to Group} =1 {Add Tab to Group}
......
32037f188850423832839e0d04e11fbf5a9dd6f1
\ No newline at end of file
...@@ -40,6 +40,13 @@ void TabMenuModel::Build(TabStripModel* tab_strip, int index) { ...@@ -40,6 +40,13 @@ void TabMenuModel::Build(TabStripModel* tab_strip, int index) {
int num_affected_tabs = affected_indices.size(); int num_affected_tabs = affected_indices.size();
AddItemWithStringId(TabStripModel::CommandNewTabToRight, AddItemWithStringId(TabStripModel::CommandNewTabToRight,
IDS_TAB_CXMENU_NEWTABTORIGHT); IDS_TAB_CXMENU_NEWTABTORIGHT);
if (base::FeatureList::IsEnabled(features::kReadLater)) {
AddItem(TabStripModel::CommandAddToReadLater,
l10n_util::GetPluralStringFUTF16(IDS_TAB_CXMENU_READ_LATER,
num_affected_tabs));
SetEnabledAt(GetItemCount() - 1,
tab_strip->IsReadLaterSupportedForAny(affected_indices));
}
if (base::FeatureList::IsEnabled(features::kTabGroups)) { if (base::FeatureList::IsEnabled(features::kTabGroups)) {
if (ExistingTabGroupSubMenuModel::ShouldShowSubmenu(tab_strip, index)) { if (ExistingTabGroupSubMenuModel::ShouldShowSubmenu(tab_strip, index)) {
// Create submenu with existing groups // Create submenu with existing groups
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/numerics/ranges.h" #include "base/numerics/ranges.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
...@@ -26,8 +27,10 @@ ...@@ -26,8 +27,10 @@
#include "chrome/browser/resource_coordinator/tab_helper.h" #include "chrome/browser/resource_coordinator/tab_helper.h"
#include "chrome/browser/send_tab_to_self/send_tab_to_self_desktop_util.h" #include "chrome/browser/send_tab_to_self/send_tab_to_self_desktop_util.h"
#include "chrome/browser/send_tab_to_self/send_tab_to_self_util.h" #include "chrome/browser/send_tab_to_self/send_tab_to_self_util.h"
#include "chrome/browser/ui/bookmarks/bookmark_utils.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/read_later/reading_list_model_factory.h"
#include "chrome/browser/ui/tab_ui_helper.h" #include "chrome/browser/ui/tab_ui_helper.h"
#include "chrome/browser/ui/tabs/tab_group.h" #include "chrome/browser/ui/tabs/tab_group.h"
#include "chrome/browser/ui/tabs/tab_group_model.h" #include "chrome/browser/ui/tabs/tab_group_model.h"
...@@ -40,6 +43,7 @@ ...@@ -40,6 +43,7 @@
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/reading_list/core/reading_list_model.h"
#include "components/tab_groups/tab_group_id.h" #include "components/tab_groups/tab_group_id.h"
#include "components/tab_groups/tab_group_visual_data.h" #include "components/tab_groups/tab_group_visual_data.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h" #include "components/web_modal/web_contents_modal_dialog_manager.h"
...@@ -1119,6 +1123,25 @@ void TabStripModel::RemoveFromGroup(const std::vector<int>& indices) { ...@@ -1119,6 +1123,25 @@ void TabStripModel::RemoveFromGroup(const std::vector<int>& indices) {
} }
} }
bool TabStripModel::IsReadLaterSupportedForAny(const std::vector<int> indices) {
ReadingListModel* model =
ReadingListModelFactory::GetForBrowserContext(profile_);
if (!model || !model->loaded())
return false;
for (int index : indices) {
if (model->IsUrlSupported(
chrome::GetURLToBookmark(GetWebContentsAt(index))))
return true;
}
return false;
}
void TabStripModel::AddToReadLater(const std::vector<int>& indices) {
ReentrancyCheck reentrancy_check(&reentrancy_guard_);
AddToReadLaterImpl(indices);
}
void TabStripModel::CreateTabGroup(const tab_groups::TabGroupId& group) { void TabStripModel::CreateTabGroup(const tab_groups::TabGroupId& group) {
TabGroupChange change(group, TabGroupChange::kCreated); TabGroupChange change(group, TabGroupChange::kCreated);
for (auto& observer : observers_) for (auto& observer : observers_)
...@@ -1214,6 +1237,9 @@ bool TabStripModel::IsContextMenuCommandEnabled( ...@@ -1214,6 +1237,9 @@ bool TabStripModel::IsContextMenuCommandEnabled(
case CommandSendTabToSelfSingleTarget: case CommandSendTabToSelfSingleTarget:
return true; return true;
case CommandAddToReadLater:
return true;
case CommandAddToNewGroup: case CommandAddToNewGroup:
return true; return true;
...@@ -1367,6 +1393,11 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index, ...@@ -1367,6 +1393,11 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index,
break; break;
} }
case CommandAddToReadLater: {
AddToReadLater(GetIndicesForCommand(context_index));
break;
}
case CommandAddToNewGroup: { case CommandAddToNewGroup: {
base::RecordAction(UserMetricsAction("TabContextMenu_AddToNewGroup")); base::RecordAction(UserMetricsAction("TabContextMenu_AddToNewGroup"));
...@@ -2056,6 +2087,28 @@ void TabStripModel::MoveAndSetGroup( ...@@ -2056,6 +2087,28 @@ void TabStripModel::MoveAndSetGroup(
MoveWebContentsAtImpl(index, new_index, false); MoveWebContentsAtImpl(index, new_index, false);
} }
void TabStripModel::AddToReadLaterImpl(const std::vector<int>& indices) {
ReadingListModel* model =
ReadingListModelFactory::GetForBrowserContext(profile_);
std::vector<WebContents*> closing_contents;
if (!model || !model->loaded())
return;
for (int index : indices) {
WebContents* contents = GetWebContentsAt(index);
GURL url;
base::string16 title;
chrome::GetURLAndTitleToBookmark(contents, &url, &title);
if (model->IsUrlSupported(url)) {
model->AddEntry(url, base::UTF16ToUTF8(title),
reading_list::EntrySource::ADDED_VIA_CURRENT_APP);
closing_contents.push_back(contents);
}
}
InternalCloseTabs(closing_contents,
CLOSE_CREATE_HISTORICAL_TAB | CLOSE_USER_GESTURE);
}
base::Optional<tab_groups::TabGroupId> TabStripModel::UngroupTab(int index) { base::Optional<tab_groups::TabGroupId> TabStripModel::UngroupTab(int index) {
base::Optional<tab_groups::TabGroupId> group = GetTabGroupForTab(index); base::Optional<tab_groups::TabGroupId> group = GetTabGroupForTab(index);
if (!group.has_value()) if (!group.has_value())
......
...@@ -461,6 +461,13 @@ class TabStripModel : public TabGroupController { ...@@ -461,6 +461,13 @@ class TabStripModel : public TabGroupController {
TabGroupModel* group_model() const { return group_model_.get(); } TabGroupModel* group_model() const { return group_model_.get(); }
// Returns true if one or more of the tabs pointed to by |indices| are
// supported by read later.
bool IsReadLaterSupportedForAny(const std::vector<int> indices);
// Saves tabs with url supported by Read Later and closes those tabs.
void AddToReadLater(const std::vector<int>& indices);
// TabGroupController: // TabGroupController:
void CreateTabGroup(const tab_groups::TabGroupId& group) override; void CreateTabGroup(const tab_groups::TabGroupId& group) override;
void OpenTabGroupEditor(const tab_groups::TabGroupId& group) override; void OpenTabGroupEditor(const tab_groups::TabGroupId& group) override;
...@@ -489,6 +496,7 @@ class TabStripModel : public TabGroupController { ...@@ -489,6 +496,7 @@ class TabStripModel : public TabGroupController {
CommandToggleSiteMuted, CommandToggleSiteMuted,
CommandSendTabToSelf, CommandSendTabToSelf,
CommandSendTabToSelfSingleTarget, CommandSendTabToSelfSingleTarget,
CommandAddToReadLater,
CommandAddToNewGroup, CommandAddToNewGroup,
CommandAddToExistingGroup, CommandAddToExistingGroup,
CommandRemoveFromGroup, CommandRemoveFromGroup,
...@@ -721,6 +729,8 @@ class TabStripModel : public TabGroupController { ...@@ -721,6 +729,8 @@ class TabStripModel : public TabGroupController {
int new_index, int new_index,
base::Optional<tab_groups::TabGroupId> new_group); base::Optional<tab_groups::TabGroupId> new_group);
void AddToReadLaterImpl(const std::vector<int>& indices);
// Helper function for MoveAndSetGroup. Removes the tab at |index| from the // Helper function for MoveAndSetGroup. Removes the tab at |index| from the
// group that contains it, if any. Also deletes that group, if it now contains // group that contains it, if any. Also deletes that group, if it now contains
// no tabs. Returns that group. // no tabs. Returns that group.
......
...@@ -21,11 +21,19 @@ ...@@ -21,11 +21,19 @@
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/read_later/read_later_test_utils.h"
#include "chrome/browser/ui/read_later/reading_list_model_factory.h"
#include "chrome/browser/ui/tabs/tab_group.h" #include "chrome/browser/ui/tabs/tab_group.h"
#include "chrome/browser/ui/tabs/tab_group_model.h" #include "chrome/browser/ui/tabs/tab_group_model.h"
#include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h" #include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/reading_list/core/reading_list_model.h"
#include "components/reading_list/core/reading_list_model_observer.h"
#include "components/tab_groups/tab_group_color.h" #include "components/tab_groups/tab_group_color.h"
#include "components/tab_groups/tab_group_id.h" #include "components/tab_groups/tab_group_id.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h" #include "components/web_modal/web_contents_modal_dialog_manager.h"
...@@ -4113,3 +4121,61 @@ TEST_F(TabStripModelTest, MoveTabsToNewWindow) { ...@@ -4113,3 +4121,61 @@ TEST_F(TabStripModelTest, MoveTabsToNewWindow) {
strip.CloseAllTabs(); strip.CloseAllTabs();
} }
class TabStripModelTestWithReadLaterEnabled : public BrowserWithTestWindowTest {
public:
TabStripModelTestWithReadLaterEnabled() {
feature_list_.InitAndEnableFeature(features::kReadLater);
}
TabStripModelTestWithReadLaterEnabled(
const TabStripModelTestWithReadLaterEnabled&) = delete;
TabStripModelTestWithReadLaterEnabled& operator=(
const TabStripModelTestWithReadLaterEnabled&) = delete;
~TabStripModelTestWithReadLaterEnabled() override = default;
void SetUp() override {
BrowserWithTestWindowTest::SetUp();
BrowserList::SetLastActive(browser());
AddTabWithTitle(browser(), GURL("http://foo/1"), "Tab 1");
AddTabWithTitle(browser(), GURL("http://foo/2"), "Tab 2");
}
void TearDown() override {
browser()->tab_strip_model()->CloseAllTabs();
BrowserWithTestWindowTest::TearDown();
}
TestingProfile::TestingFactories GetTestingFactories() override {
return {{ReadingListModelFactory::GetInstance(),
ReadingListModelFactory::GetDefaultFactoryForTesting()}};
}
protected:
void AddTabWithTitle(Browser* browser,
const GURL url,
const std::string title) {
AddTab(browser, url);
NavigateAndCommitActiveTabWithTitle(browser, url,
base::ASCIIToUTF16(title));
}
private:
base::test::ScopedFeatureList feature_list_;
};
TEST_F(TabStripModelTestWithReadLaterEnabled, AddToReadLater) {
ReadingListModel* reading_list_model =
ReadingListModelFactory::GetForBrowserContext(profile());
test::ReadingListLoadObserver(reading_list_model).Wait();
TabStripModel* tabstrip = browser()->tab_strip_model();
EXPECT_EQ(tabstrip->count(), 2);
// Add first tab to Read Later and verify it has been added and the tab has
// been closed.
GURL expected_url = tabstrip->GetWebContentsAt(0)->GetURL();
tabstrip->AddToReadLater({0});
EXPECT_EQ(reading_list_model->size(), 1u);
EXPECT_NE(reading_list_model->GetEntryByURL(expected_url), nullptr);
EXPECT_EQ(tabstrip->count(), 1);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment