Commit 25a2c0a3 authored by Caroline Rising's avatar Caroline Rising Committed by Commit Bot

Prevent tab from closing after being added to Read Later.

This change is behind kReadLater feature flag. Originally closing the
tab after it was added to read later was the desired behavior. Based on
fishfood feedback we have changed this decision and want tabs to remain
open after being added to read later.

Bug: 1117023, 1117023
Change-Id: I84bcf1d2203a950c62c56ac4fb1f0e8cb2f99f1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2498103Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Commit-Queue: Caroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821194}
parent 2a55c4f5
...@@ -1045,11 +1045,6 @@ bool MoveCurrentTabToReadLater(Browser* browser) { ...@@ -1045,11 +1045,6 @@ bool MoveCurrentTabToReadLater(Browser* browser) {
return false; return false;
model->AddEntry(url, base::UTF16ToUTF8(title), model->AddEntry(url, base::UTF16ToUTF8(title),
reading_list::EntrySource::ADDED_VIA_CURRENT_APP); reading_list::EntrySource::ADDED_VIA_CURRENT_APP);
// Close current tab.
int index = browser->tab_strip_model()->active_index();
browser->tab_strip_model()->CloseWebContentsAt(
index, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB |
TabStripModel::CLOSE_USER_GESTURE);
return true; return true;
} }
......
...@@ -2090,7 +2090,6 @@ void TabStripModel::MoveAndSetGroup( ...@@ -2090,7 +2090,6 @@ void TabStripModel::MoveAndSetGroup(
void TabStripModel::AddToReadLaterImpl(const std::vector<int>& indices) { void TabStripModel::AddToReadLaterImpl(const std::vector<int>& indices) {
ReadingListModel* model = ReadingListModel* model =
ReadingListModelFactory::GetForBrowserContext(profile_); ReadingListModelFactory::GetForBrowserContext(profile_);
std::vector<WebContents*> closing_contents;
if (!model || !model->loaded()) if (!model || !model->loaded())
return; return;
...@@ -2102,11 +2101,8 @@ void TabStripModel::AddToReadLaterImpl(const std::vector<int>& indices) { ...@@ -2102,11 +2101,8 @@ void TabStripModel::AddToReadLaterImpl(const std::vector<int>& indices) {
if (model->IsUrlSupported(url)) { if (model->IsUrlSupported(url)) {
model->AddEntry(url, base::UTF16ToUTF8(title), model->AddEntry(url, base::UTF16ToUTF8(title),
reading_list::EntrySource::ADDED_VIA_CURRENT_APP); 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) {
......
...@@ -465,7 +465,7 @@ class TabStripModel : public TabGroupController { ...@@ -465,7 +465,7 @@ class TabStripModel : public TabGroupController {
// supported by read later. // supported by read later.
bool IsReadLaterSupportedForAny(const std::vector<int> indices); bool IsReadLaterSupportedForAny(const std::vector<int> indices);
// Saves tabs with url supported by Read Later and closes those tabs. // Saves tabs with url supported by Read Later.
void AddToReadLater(const std::vector<int>& indices); void AddToReadLater(const std::vector<int>& indices);
// TabGroupController: // TabGroupController:
......
...@@ -4172,11 +4172,10 @@ TEST_F(TabStripModelTestWithReadLaterEnabled, AddToReadLater) { ...@@ -4172,11 +4172,10 @@ TEST_F(TabStripModelTestWithReadLaterEnabled, AddToReadLater) {
TabStripModel* tabstrip = browser()->tab_strip_model(); TabStripModel* tabstrip = browser()->tab_strip_model();
EXPECT_EQ(tabstrip->count(), 2); EXPECT_EQ(tabstrip->count(), 2);
// Add first tab to Read Later and verify it has been added and the tab has // Add first tab to Read Later and verify it has been added.
// been closed.
GURL expected_url = tabstrip->GetWebContentsAt(0)->GetURL(); GURL expected_url = tabstrip->GetWebContentsAt(0)->GetURL();
tabstrip->AddToReadLater({0}); tabstrip->AddToReadLater({0});
EXPECT_EQ(reading_list_model->size(), 1u); EXPECT_EQ(reading_list_model->size(), 1u);
EXPECT_NE(reading_list_model->GetEntryByURL(expected_url), nullptr); EXPECT_NE(reading_list_model->GetEntryByURL(expected_url), nullptr);
EXPECT_EQ(tabstrip->count(), 1); EXPECT_EQ(tabstrip->count(), 2);
} }
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