[ntp-xdtr] Test(s) cleanup
Enables a disabled test that had a memory leak issue.
Low-Coverage-Reason: HARD_TO_TEST
Bug: none
Change-Id: Ieca24c97d9da859b2123c753258828066013f9c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5057061
Reviewed-by: Marlon Facey <mfacey@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Marlon Facey <mfacey@chromium.org>
Auto-Submit: Roman Arora <romanarora@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1229350}
diff --git a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.cc b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.cc
index 573270dc..6bb9ec8 100644
--- a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.cc
+++ b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.cc
@@ -101,9 +101,8 @@
TabResumptionPageHandler::TabResumptionPageHandler(
mojo::PendingReceiver<ntp::tab_resumption::mojom::PageHandler>
pending_page_handler,
- Profile* profile,
content::WebContents* web_contents)
- : profile_(profile),
+ : profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())),
web_contents_(web_contents),
page_handler_(this, std::move(pending_page_handler)) {
DCHECK(profile_);
diff --git a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.h b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.h
index 16fb7b76..693c9c6 100644
--- a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.h
+++ b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler.h
@@ -35,7 +35,6 @@
TabResumptionPageHandler(
mojo::PendingReceiver<ntp::tab_resumption::mojom::PageHandler>
pending_page_handler,
- Profile* profile,
content::WebContents* web_contents);
TabResumptionPageHandler(const TabResumptionPageHandler&) = delete;
@@ -43,6 +42,7 @@
~TabResumptionPageHandler() override;
+ // tab_resumption::mojom::PageHandler:
void GetTabs(GetTabsCallback callback) override;
sync_sessions::OpenTabsUIDelegate* GetOpenTabsUIDelegate();
diff --git a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler_unittest.cc b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler_unittest.cc
index 06fe7a83..bff053f 100644
--- a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler_unittest.cc
+++ b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_page_handler_unittest.cc
@@ -28,18 +28,19 @@
void SetUp() override {
BrowserWithTestWindowTest::SetUp();
+ mock_session_sync_service_ = static_cast<MockSessionSyncService*>(
+ SessionSyncServiceFactory::GetForProfile(profile()));
web_contents_ = content::WebContents::Create(
content::WebContents::CreateParams(profile()));
handler_ = std::make_unique<TabResumptionPageHandler>(
mojo::PendingReceiver<ntp::tab_resumption::mojom::PageHandler>(),
- profile(), web_contents_.get());
- mock_session_sync_service_ = static_cast<MockSessionSyncService*>(
- SessionSyncServiceFactory::GetForProfile(profile()));
+ web_contents_.get());
}
void TearDown() override {
handler_.reset();
web_contents_.reset();
+ mock_session_sync_service_ = nullptr;
BrowserWithTestWindowTest::TearDown();
}
@@ -49,8 +50,6 @@
TabResumptionPageHandler& handler() { return *handler_; }
- void ResetHandler() { handler_.reset(); }
-
private:
// BrowserWithTestWindowTest:
TestingProfile::TestingFactories GetTestingFactories() override {
@@ -61,19 +60,17 @@
})}};
}
- raw_ptr<MockSessionSyncService, DisableDanglingPtrDetection>
- mock_session_sync_service_;
+ raw_ptr<MockSessionSyncService> mock_session_sync_service_;
std::unique_ptr<content::WebContents> web_contents_;
std::unique_ptr<TabResumptionPageHandler> handler_;
};
-// TODO(mfacey): Figure out why test breaks on exit.
-TEST_F(TabResumptionPageHandlerTest, DISABLED_GetTabs) {
+TEST_F(TabResumptionPageHandlerTest, GetTabs) {
const int kSampleSessionsCount = 3;
- std::vector<const sync_sessions::SyncedSession*> sample_sessions;
+ std::vector<std::unique_ptr<sync_sessions::SyncedSession>> sample_sessions;
for (int i = 0; i < kSampleSessionsCount; i++) {
- sample_sessions.push_back(
- SampleSession(("Test Tag " + base::NumberToString(i)).c_str(), 3));
+ sample_sessions.push_back(SampleSession(
+ "Test Name", ("Test Tag " + base::NumberToString(i)).c_str(), 3));
}
EXPECT_CALL(*mock_session_sync_service().GetOpenTabsUIDelegate(),
@@ -81,7 +78,9 @@
.WillOnce(testing::Invoke(
[&sample_sessions](
std::vector<const sync_sessions::SyncedSession*>* sessions) {
- *sessions = sample_sessions;
+ for (auto& sample_session : sample_sessions) {
+ sessions->push_back(sample_session.get());
+ }
return true;
}));
@@ -109,7 +108,5 @@
ASSERT_EQ(GURL(kSampleUrl), tabs[0]->url);
}
}
-
- ResetHandler();
}
} // namespace
diff --git a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.cc b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.cc
index d1a52cf..1fcd8d76 100644
--- a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.cc
+++ b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.cc
@@ -40,16 +40,16 @@
return nullptr;
}
-sync_sessions::SyncedSession* SampleSession(const char session_tag[],
- int num_windows) {
- auto* sample_session = new sync_sessions::SyncedSession();
+std::unique_ptr<sync_sessions::SyncedSession> SampleSession(
+ const char session_name[],
+ const char session_tag[],
+ int num_windows) {
+ auto sample_session = std::make_unique<sync_sessions::SyncedSession>();
for (int i = 0; i < num_windows; i++) {
sample_session->windows[SessionID::FromSerializedValue(i)] =
SampleSessionWindow(3);
}
- constexpr char session_name[] = "Test Name";
-
sample_session->SetSessionTag(session_tag);
sample_session->SetSessionName(session_name);
sample_session->SetModifiedTime(base::Time::Now());
diff --git a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.h b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.h
index 281a5203..70c31d886 100644
--- a/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.h
+++ b/chrome/browser/new_tab_page/modules/v2/tab_resumption/tab_resumption_test_support.h
@@ -69,8 +69,10 @@
MockOpenTabsUIDelegate mock_open_tabs_ui_delegate_;
};
-sync_sessions::SyncedSession* SampleSession(const char session_tag[],
- int num_windows);
+std::unique_ptr<sync_sessions::SyncedSession> SampleSession(
+ const char session_name[],
+ const char session_tag[],
+ int num_windows);
std::unique_ptr<sync_sessions::SyncedSessionWindow> SampleSessionWindow(
int num_tabs);
std::unique_ptr<sessions::SessionTab> SampleSessionTab(int tab_id);
diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.cc b/chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.cc
index 0b03b1f..12f9457 100644
--- a/chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.cc
+++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.cc
@@ -897,7 +897,7 @@
mojo::PendingReceiver<ntp::tab_resumption::mojom::PageHandler>
pending_page_handler) {
tab_resumption_handler_ = std::make_unique<TabResumptionPageHandler>(
- std::move(pending_page_handler), profile_, web_contents());
+ std::move(pending_page_handler), web_contents());
}
void NewTabPageUI::BindInterface(
