Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(149)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/sessions/persistent_tab_restore_service_unittest.cc

Issue 2200993004: Make TabRestoreService::Entry noncopyable and fix up surrounding code. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@tab-test-cleanup
Patch Set: Add back NOTREACHED() and a return for gcc Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/browser/extensions/api/sessions/sessions_api.cc ('K') | « chrome/browser/extensions/api/sessions/sessions_api.cc ('k') | chrome/browser/ui/browser_commands.cc » ('j') | chrome/browser/ui/cocoa/history_menu_bridge.mm » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sessions/persistent_tab_restore_service_unittest.cc
diff --git a/chrome/browser/sessions/persistent_tab_restore_service_unittest.cc b/chrome/browser/sessions/persistent_tab_restore_service_unittest.cc
index 2e0825b4b952e0886a8bba2676970007e6da1e8b..65d5881bcacdd5fed8b2d1919efb6820cd619eba 100644
--- a/chrome/browser/sessions/persistent_tab_restore_service_unittest.cc
+++ b/chrome/browser/sessions/persistent_tab_restore_service_unittest.cc
@@ -226,8 +226,8 @@ TEST_F(PersistentTabRestoreServiceTest, Basic) {
ASSERT_EQ(1U, service_->entries().size());
// Make sure the entry matches.
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ sessions::TabRestoreService::Entry* entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
EXPECT_FALSE(tab->pinned);
EXPECT_TRUE(tab->extension_app_id.empty());
@@ -236,7 +236,7 @@ TEST_F(PersistentTabRestoreServiceTest, Basic) {
EXPECT_TRUE(url2_ == tab->navigations[1].virtual_url());
EXPECT_TRUE(url3_ == tab->navigations[2].virtual_url());
EXPECT_EQ("", tab->user_agent_override);
- EXPECT_EQ(2, tab->current_navigation_index);
+ EXPECT_EQ(2U, tab->current_navigation_index);
EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
tab->timestamp.ToInternalValue());
@@ -250,8 +250,8 @@ TEST_F(PersistentTabRestoreServiceTest, Basic) {
ASSERT_EQ(2U, service_->entries().size());
// Make sure the entry matches.
- entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
tab = static_cast<Tab*>(entry);
EXPECT_FALSE(tab->pinned);
ASSERT_EQ(3U, tab->navigations.size());
@@ -259,7 +259,7 @@ TEST_F(PersistentTabRestoreServiceTest, Basic) {
EXPECT_EQ(url2_, tab->navigations[1].virtual_url());
EXPECT_EQ(url3_, tab->navigations[2].virtual_url());
EXPECT_EQ(user_agent_override_, tab->user_agent_override);
- EXPECT_EQ(1, tab->current_navigation_index);
+ EXPECT_EQ(1U, tab->current_navigation_index);
EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
tab->timestamp.ToInternalValue());
}
@@ -286,15 +286,15 @@ TEST_F(PersistentTabRestoreServiceTest, Restore) {
// And verify the entry.
sessions::PersistentTabRestoreService::Entry* entry =
- service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
EXPECT_FALSE(tab->pinned);
ASSERT_EQ(3U, tab->navigations.size());
EXPECT_TRUE(url1_ == tab->navigations[0].virtual_url());
EXPECT_TRUE(url2_ == tab->navigations[1].virtual_url());
EXPECT_TRUE(url3_ == tab->navigations[2].virtual_url());
- EXPECT_EQ(2, tab->current_navigation_index);
+ EXPECT_EQ(2U, tab->current_navigation_index);
EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
tab->timestamp.ToInternalValue());
}
@@ -311,8 +311,8 @@ TEST_F(PersistentTabRestoreServiceTest, RestorePinnedAndApp) {
// We have to explicitly mark the tab as pinned as there is no browser for
// these tests.
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ sessions::TabRestoreService::Entry* entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
tab->pinned = true;
const std::string extension_app_id("test");
@@ -325,15 +325,15 @@ TEST_F(PersistentTabRestoreServiceTest, RestorePinnedAndApp) {
ASSERT_EQ(1U, service_->entries().size());
// And verify the entry.
- entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
tab = static_cast<Tab*>(entry);
EXPECT_TRUE(tab->pinned);
ASSERT_EQ(3U, tab->navigations.size());
EXPECT_TRUE(url1_ == tab->navigations[0].virtual_url());
EXPECT_TRUE(url2_ == tab->navigations[1].virtual_url());
EXPECT_TRUE(url3_ == tab->navigations[2].virtual_url());
- EXPECT_EQ(2, tab->current_navigation_index);
+ EXPECT_EQ(2U, tab->current_navigation_index);
EXPECT_TRUE(extension_app_id == tab->extension_app_id);
}
@@ -355,8 +355,8 @@ TEST_F(PersistentTabRestoreServiceTest, DontPersistPostData) {
ASSERT_EQ(1U, service_->entries().size());
const sessions::TabRestoreService::Entry* restored_entry =
- service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, restored_entry->type);
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, restored_entry->type());
const Tab* restored_tab =
static_cast<const Tab*>(restored_entry);
@@ -402,17 +402,18 @@ TEST_F(PersistentTabRestoreServiceTest, LoadPreviousSession) {
// Make sure we get back one entry with one tab whose url is url1.
ASSERT_EQ(1U, service_->entries().size());
- sessions::TabRestoreService::Entry* entry2 = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry2->type);
+ sessions::TabRestoreService::Entry* entry2 =
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry2->type());
sessions::TabRestoreService::Window* window =
static_cast<sessions::TabRestoreService::Window*>(entry2);
ASSERT_EQ(1U, window->tabs.size());
EXPECT_EQ(0, window->timestamp.ToInternalValue());
- EXPECT_EQ(0, window->selected_tab_index);
- ASSERT_EQ(1U, window->tabs[0].navigations.size());
- EXPECT_EQ(0, window->tabs[0].current_navigation_index);
- EXPECT_EQ(0, window->tabs[0].timestamp.ToInternalValue());
- EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].virtual_url());
+ EXPECT_EQ(0U, window->selected_tab_index);
+ ASSERT_EQ(1U, window->tabs[0]->navigations.size());
+ EXPECT_EQ(0U, window->tabs[0]->current_navigation_index);
+ EXPECT_EQ(0, window->tabs[0]->timestamp.ToInternalValue());
+ EXPECT_TRUE(url1_ == window->tabs[0]->navigations[0].virtual_url());
}
// Makes sure we don't attempt to load previous sessions after a restore.
@@ -461,25 +462,25 @@ TEST_F(PersistentTabRestoreServiceTest, LoadPreviousSessionAndTabs) {
// the tab restore service. The previous session entry should be first.
ASSERT_EQ(2U, service_->entries().size());
// The first entry should come from the session service.
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type);
+ sessions::TabRestoreService::Entry* entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type());
sessions::TabRestoreService::Window* window =
static_cast<sessions::TabRestoreService::Window*>(entry);
ASSERT_EQ(1U, window->tabs.size());
- EXPECT_EQ(0, window->selected_tab_index);
+ EXPECT_EQ(0U, window->selected_tab_index);
EXPECT_EQ(0, window->timestamp.ToInternalValue());
- ASSERT_EQ(1U, window->tabs[0].navigations.size());
- EXPECT_EQ(0, window->tabs[0].current_navigation_index);
- EXPECT_EQ(0, window->tabs[0].timestamp.ToInternalValue());
- EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].virtual_url());
+ ASSERT_EQ(1U, window->tabs[0]->navigations.size());
+ EXPECT_EQ(0U, window->tabs[0]->current_navigation_index);
+ EXPECT_EQ(0, window->tabs[0]->timestamp.ToInternalValue());
+ EXPECT_TRUE(url1_ == window->tabs[0]->navigations[0].virtual_url());
// Then the closed tab.
- entry = *(++service_->entries().begin());
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ entry = (++service_->entries().begin())->get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
ASSERT_FALSE(tab->pinned);
ASSERT_EQ(3U, tab->navigations.size());
- EXPECT_EQ(2, tab->current_navigation_index);
+ EXPECT_EQ(2U, tab->current_navigation_index);
EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
tab->timestamp.ToInternalValue());
EXPECT_TRUE(url1_ == tab->navigations[0].virtual_url());
@@ -504,24 +505,24 @@ TEST_F(PersistentTabRestoreServiceTest, LoadPreviousSessionAndTabsPinned) {
// the tab restore service. The previous session entry should be first.
ASSERT_EQ(2U, service_->entries().size());
// The first entry should come from the session service.
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type);
+ sessions::TabRestoreService::Entry* entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type());
sessions::TabRestoreService::Window* window =
static_cast<sessions::TabRestoreService::Window*>(entry);
ASSERT_EQ(1U, window->tabs.size());
- EXPECT_EQ(0, window->selected_tab_index);
- EXPECT_TRUE(window->tabs[0].pinned);
- ASSERT_EQ(1U, window->tabs[0].navigations.size());
- EXPECT_EQ(0, window->tabs[0].current_navigation_index);
- EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].virtual_url());
+ EXPECT_EQ(0U, window->selected_tab_index);
+ EXPECT_TRUE(window->tabs[0]->pinned);
+ ASSERT_EQ(1U, window->tabs[0]->navigations.size());
+ EXPECT_EQ(0U, window->tabs[0]->current_navigation_index);
+ EXPECT_TRUE(url1_ == window->tabs[0]->navigations[0].virtual_url());
// Then the closed tab.
- entry = *(++service_->entries().begin());
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ entry = (++service_->entries().begin())->get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
ASSERT_FALSE(tab->pinned);
ASSERT_EQ(3U, tab->navigations.size());
- EXPECT_EQ(2, tab->current_navigation_index);
+ EXPECT_EQ(2U, tab->current_navigation_index);
EXPECT_TRUE(url1_ == tab->navigations[0].virtual_url());
EXPECT_TRUE(url2_ == tab->navigations[1].virtual_url());
EXPECT_TRUE(url3_ == tab->navigations[2].virtual_url());
@@ -549,17 +550,17 @@ TEST_F(PersistentTabRestoreServiceTest, ManyWindowsInSessionService) {
ASSERT_EQ(kMaxEntries, service_->entries().size());
// The first entry should come from the session service.
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type);
+ sessions::TabRestoreService::Entry* entry = service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type());
sessions::TabRestoreService::Window* window =
static_cast<sessions::TabRestoreService::Window*>(entry);
ASSERT_EQ(1U, window->tabs.size());
- EXPECT_EQ(0, window->selected_tab_index);
+ EXPECT_EQ(0U, window->selected_tab_index);
EXPECT_EQ(0, window->timestamp.ToInternalValue());
- ASSERT_EQ(1U, window->tabs[0].navigations.size());
- EXPECT_EQ(0, window->tabs[0].current_navigation_index);
- EXPECT_EQ(0, window->tabs[0].timestamp.ToInternalValue());
- EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].virtual_url());
+ ASSERT_EQ(1U, window->tabs[0]->navigations.size());
+ EXPECT_EQ(0U, window->tabs[0]->current_navigation_index);
+ EXPECT_EQ(0, window->tabs[0]->timestamp.ToInternalValue());
+ EXPECT_TRUE(url1_ == window->tabs[0]->navigations[0].virtual_url());
}
// Makes sure we restore timestamps correctly.
@@ -578,8 +579,9 @@ TEST_F(PersistentTabRestoreServiceTest, TimestampSurvivesRestore) {
std::vector<SerializedNavigationEntry> old_navigations;
{
// |entry|/|tab| doesn't survive after RecreateService().
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ sessions::TabRestoreService::Entry* entry =
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
tab->timestamp = tab_timestamp;
old_navigations = tab->navigations;
@@ -600,8 +602,8 @@ TEST_F(PersistentTabRestoreServiceTest, TimestampSurvivesRestore) {
// And verify the entry.
sessions::TabRestoreService::Entry* restored_entry =
- service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, restored_entry->type);
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, restored_entry->type());
Tab* restored_tab =
static_cast<Tab*>(restored_entry);
EXPECT_EQ(tab_timestamp.ToInternalValue(),
@@ -627,8 +629,9 @@ TEST_F(PersistentTabRestoreServiceTest, StatusCodesSurviveRestore) {
std::vector<sessions::SerializedNavigationEntry> old_navigations;
{
// |entry|/|tab| doesn't survive after RecreateService().
- sessions::TabRestoreService::Entry* entry = service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type);
+ sessions::TabRestoreService::Entry* entry =
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, entry->type());
Tab* tab = static_cast<Tab*>(entry);
old_navigations = tab->navigations;
}
@@ -648,8 +651,8 @@ TEST_F(PersistentTabRestoreServiceTest, StatusCodesSurviveRestore) {
// And verify the entry.
sessions::TabRestoreService::Entry* restored_entry =
- service_->entries().front();
- ASSERT_EQ(sessions::TabRestoreService::TAB, restored_entry->type);
+ service_->entries().front().get();
+ ASSERT_EQ(sessions::TabRestoreService::TAB, restored_entry->type());
Tab* restored_tab =
static_cast<Tab*>(restored_entry);
ASSERT_EQ(old_navigations.size(), restored_tab->navigations.size());
@@ -669,11 +672,11 @@ TEST_F(PersistentTabRestoreServiceTest, PruneEntries) {
base::StringPrintf("http://%d", static_cast<int>(i)),
base::SizeTToString(i));
- Tab* tab = new Tab();
+ auto tab = base::MakeUnique<Tab>();
tab->navigations.push_back(navigation);
tab->current_navigation_index = 0;
- mutable_entries()->push_back(tab);
+ mutable_entries()->push_back(std::move(tab));
}
// Only keep kMaxEntries around.
@@ -689,61 +692,62 @@ TEST_F(PersistentTabRestoreServiceTest, PruneEntries) {
SerializedNavigationEntry navigation =
SerializedNavigationEntryTestHelper::CreateNavigation(kRecentUrl,
"Most recent");
- Tab* tab = new Tab();
+ auto tab = base::MakeUnique<Tab>();
tab->navigations.push_back(navigation);
tab->current_navigation_index = 0;
- mutable_entries()->push_front(tab);
+ mutable_entries()->push_front(std::move(tab));
EXPECT_EQ(max_entries + 1, service_->entries().size());
PruneEntries();
EXPECT_EQ(max_entries, service_->entries().size());
- EXPECT_EQ(GURL(kRecentUrl),
- static_cast<Tab*>(service_->entries().front())->
- navigations[0].virtual_url());
+ EXPECT_EQ(GURL(kRecentUrl), static_cast<Tab&>(*service_->entries().front())
+ .navigations[0]
+ .virtual_url());
// Ignore NTPs.
navigation = SerializedNavigationEntryTestHelper::CreateNavigation(
chrome::kChromeUINewTabURL, "New tab");
- tab = new Tab();
+ tab = base::MakeUnique<Tab>();
tab->navigations.push_back(navigation);
tab->current_navigation_index = 0;
- mutable_entries()->push_front(tab);
+ mutable_entries()->push_front(std::move(tab));
EXPECT_EQ(max_entries + 1, service_->entries().size());
PruneEntries();
EXPECT_EQ(max_entries, service_->entries().size());
- EXPECT_EQ(GURL(kRecentUrl),
- static_cast<Tab*>(service_->entries().front())->
- navigations[0].virtual_url());
+ EXPECT_EQ(GURL(kRecentUrl), static_cast<Tab&>(*service_->entries().front())
+ .navigations[0]
+ .virtual_url());
// Don't prune pinned NTPs.
- tab = new Tab();
+ tab = base::MakeUnique<Tab>();
tab->pinned = true;
tab->current_navigation_index = 0;
tab->navigations.push_back(navigation);
- mutable_entries()->push_front(tab);
+ mutable_entries()->push_front(std::move(tab));
EXPECT_EQ(max_entries + 1, service_->entries().size());
PruneEntries();
EXPECT_EQ(max_entries, service_->entries().size());
EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
- static_cast<Tab*>(service_->entries().front())->
- navigations[0].virtual_url());
+ static_cast<Tab&>(*service_->entries().front())
+ .navigations[0]
+ .virtual_url());
// Don't prune NTPs that have multiple navigations.
// (Erase the last NTP first.)
- delete service_->entries().front();
mutable_entries()->erase(mutable_entries()->begin());
- tab = new Tab();
+ tab = base::MakeUnique<Tab>();
tab->current_navigation_index = 1;
tab->navigations.push_back(navigation);
tab->navigations.push_back(navigation);
- mutable_entries()->push_front(tab);
+ mutable_entries()->push_front(std::move(tab));
EXPECT_EQ(max_entries, service_->entries().size());
PruneEntries();
EXPECT_EQ(max_entries, service_->entries().size());
EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
- static_cast<Tab*>(service_->entries().front())->
- navigations[1].virtual_url());
+ static_cast<Tab&>(*service_->entries().front())
+ .navigations[1]
+ .virtual_url());
}
// Regression test for crbug.com/106082
« chrome/browser/extensions/api/sessions/sessions_api.cc ('K') | « chrome/browser/extensions/api/sessions/sessions_api.cc ('k') | chrome/browser/ui/browser_commands.cc » ('j') | chrome/browser/ui/cocoa/history_menu_bridge.mm » ('J')

Powered by Google App Engine
This is Rietveld 408576698