Skip to content

GitLab

T
tangled
Commit f16d83f8 authored by Charles Zhao's avatar Charles Zhao Committed by Commit Bot
Browse files
Options

Fix memory leak of TabMetricsLogger unit tests. 

Bug: 961073
Change-Id: Ic4545d4757f00714828d642204b6d5ca1c4cc38a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463085Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Reviewed-by: default avatarThanh Nguyen <thanhdng@chromium.org>
Reviewed-by: default avatarCharles . <charleszhao@chromium.org>
Commit-Queue: Charles . <charleszhao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818270}
parent b20f47d0
Hide whitespace changes
Inline Side-by-side
Showing with 26 additions and 53 deletions
chrome/browser/resource_coordinator/tab_metrics_logger_unittest.cc
View file @ f16d83f8
...@@ -31,35 +31,6 @@ ...@@ -31,35 +31,6 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/ui_base_types.h" #include "ui/base/ui_base_types.h"
// TODO(crbug.com/961073): Fix memory leaks in tests and re-enable on LSAN.
#ifdef LEAK_SANITIZER
#define MAYBE_GetNavigationEntryCount DISABLED_GetNavigationEntryCount
#define MAYBE_GetAudibleState DISABLED_GetAudibleState
#define MAYBE_CreateWindowFeaturesTest DISABLED_CreateWindowFeaturesTest
#define MAYBE_CreateWindowFeaturesTestMoveTabToOtherWindow \
DISABLED_CreateWindowFeaturesTestMoveTabToOtherWindow
#define MAYBE_CreateWindowFeaturesTestReplaceTab \
DISABLED_CreateWindowFeaturesTestReplaceTab
#define MAYBE_GetHasFormEntry DISABLED_GetHasFormEntry
#define MAYBE_GetPinState DISABLED_GetPinState
#define MAYBE_GetSiteEngagementScore DISABLED_GetSiteEngagementScore
#define MAYBE_GetHost DISABLED_GetHost
#define MAYBE_GetTabFeatures DISABLED_GetTabFeatures
#else
#define MAYBE_GetNavigationEntryCount GetNavigationEntryCount
#define MAYBE_GetAudibleState GetAudibleState
#define MAYBE_CreateWindowFeaturesTest CreateWindowFeaturesTest
#define MAYBE_CreateWindowFeaturesTestMoveTabToOtherWindow \
CreateWindowFeaturesTestMoveTabToOtherWindow
#define MAYBE_CreateWindowFeaturesTestReplaceTab \
CreateWindowFeaturesTestReplaceTab
#define MAYBE_GetHasFormEntry GetHasFormEntry
#define MAYBE_GetPinState GetPinState
#define MAYBE_GetSiteEngagementScore GetSiteEngagementScore
#define MAYBE_GetHost GetHost
#define MAYBE_GetTabFeatures GetTabFeatures
#endif
using content::WebContentsTester; using content::WebContentsTester;
using metrics::WindowMetricsEvent; using metrics::WindowMetricsEvent;
using tab_ranker::WindowFeatures; using tab_ranker::WindowFeatures;
...@@ -80,14 +51,16 @@ class FakeBrowserWindow : public TestBrowserWindow { ...@@ -80,14 +51,16 @@ class FakeBrowserWindow : public TestBrowserWindow {
// Helper function to handle FakeBrowserWindow lifetime. Modeled after // Helper function to handle FakeBrowserWindow lifetime. Modeled after
// CreateBrowserWithTestWindowForParams. // CreateBrowserWithTestWindowForParams.
static std::unique_ptr<Browser> CreateBrowserWithFakeWindowForParams( static std::unique_ptr<Browser> CreateBrowserWithFakeWindowForParams(
Browser::CreateParams* params) { const Browser::CreateParams& input) {
// TestBrowserWindowOwner takes ownersip of the window and will destroy the // TestBrowserWindowOwner takes ownersip of the window and will destroy the
// window (along with itself) automatically when the browser is closed. // window (along with itself) automatically when the browser is closed.
FakeBrowserWindow* window = new FakeBrowserWindow; FakeBrowserWindow* window = new FakeBrowserWindow;
new TestBrowserWindowOwner(window); new TestBrowserWindowOwner(window);
params->window = window; // Create a new params with window set.
auto browser = std::make_unique<Browser>(*params); Browser::CreateParams params = input;
params.window = window;
auto browser = std::make_unique<Browser>(params);
window->browser_ = browser.get(); window->browser_ = browser.get();
window->Activate(); window->Activate();
return browser; return browser;
...@@ -145,20 +118,21 @@ class TabMetricsLoggerTest : public ChromeRenderViewHostTestHarness { ...@@ -145,20 +118,21 @@ class TabMetricsLoggerTest : public ChromeRenderViewHostTestHarness {
protected: protected:
void SetUp() override { void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp(); ChromeRenderViewHostTestHarness::SetUp();
Browser::CreateParams params(profile(), true);
params_ = new Browser::CreateParams(profile(), true); browser_ = FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params);
browser_ = CreateBrowserWithTestWindowForParams(params_);
tab_strip_model_ = browser_->tab_strip_model(); tab_strip_model_ = browser_->tab_strip_model();
// Add a foreground tab. // Add a foreground tab.
// This will not cause any leak, because we'll call
// tab_strip_model_->CloseAllTabs() in TearDown.
web_contents_ = tab_activity_simulator_.AddWebContentsAndNavigate( web_contents_ = tab_activity_simulator_.AddWebContentsAndNavigate(
tab_strip_model_, GURL(kChromiumUrl)); tab_strip_model_, GURL(kChromiumUrl));
tab_strip_model_->ActivateTabAt(0); tab_strip_model_->ActivateTabAt(0);
// This will not cause any leak because it's just a static_cast.
web_contents_tester_ = WebContentsTester::For(web_contents_); web_contents_tester_ = WebContentsTester::For(web_contents_);
} }
TabActivitySimulator tab_activity_simulator_; TabActivitySimulator tab_activity_simulator_;
Browser::CreateParams* params_;
std::unique_ptr<Browser> browser_; std::unique_ptr<Browser> browser_;
TabStripModel* tab_strip_model_; TabStripModel* tab_strip_model_;
content::WebContents* web_contents_; content::WebContents* web_contents_;
...@@ -185,7 +159,7 @@ class TabMetricsLoggerTest : public ChromeRenderViewHostTestHarness { ...@@ -185,7 +159,7 @@ class TabMetricsLoggerTest : public ChromeRenderViewHostTestHarness {
}; };
// Tests has_form_entry. // Tests has_form_entry.
TEST_F(TabMetricsLoggerTest, MAYBE_GetHasFormEntry) { TEST_F(TabMetricsLoggerTest, GetHasFormEntry) {
EXPECT_FALSE(CurrentTabFeatures().has_form_entry); EXPECT_FALSE(CurrentTabFeatures().has_form_entry);
FormInteractionTabHelper::CreateForWebContents(web_contents_); FormInteractionTabHelper::CreateForWebContents(web_contents_);
FormInteractionTabHelper::FromWebContents(web_contents_) FormInteractionTabHelper::FromWebContents(web_contents_)
...@@ -194,14 +168,14 @@ TEST_F(TabMetricsLoggerTest, MAYBE_GetHasFormEntry) { ...@@ -194,14 +168,14 @@ TEST_F(TabMetricsLoggerTest, MAYBE_GetHasFormEntry) {
} }
// Tests is_pinned. // Tests is_pinned.
TEST_F(TabMetricsLoggerTest, MAYBE_GetPinState) { TEST_F(TabMetricsLoggerTest, GetPinState) {
EXPECT_FALSE(CurrentTabFeatures().is_pinned); EXPECT_FALSE(CurrentTabFeatures().is_pinned);
tab_strip_model_->SetTabPinned(0, true); tab_strip_model_->SetTabPinned(0, true);
EXPECT_TRUE(CurrentTabFeatures().is_pinned); EXPECT_TRUE(CurrentTabFeatures().is_pinned);
} }
// Tests navigation_entry_count. // Tests navigation_entry_count.
TEST_F(TabMetricsLoggerTest, MAYBE_GetNavigationEntryCount) { TEST_F(TabMetricsLoggerTest, GetNavigationEntryCount) {
EXPECT_EQ(CurrentTabFeatures().navigation_entry_count, 1); EXPECT_EQ(CurrentTabFeatures().navigation_entry_count, 1);
tab_activity_simulator_.Navigate(web_contents_, GURL(kExampleUrl), tab_activity_simulator_.Navigate(web_contents_, GURL(kExampleUrl),
pg_metrics_.page_transition); pg_metrics_.page_transition);
...@@ -212,7 +186,7 @@ TEST_F(TabMetricsLoggerTest, MAYBE_GetNavigationEntryCount) { ...@@ -212,7 +186,7 @@ TEST_F(TabMetricsLoggerTest, MAYBE_GetNavigationEntryCount) {
} }
// Tests site_engagement_score. // Tests site_engagement_score.
TEST_F(TabMetricsLoggerTest, MAYBE_GetSiteEngagementScore) { TEST_F(TabMetricsLoggerTest, GetSiteEngagementScore) {
EXPECT_EQ(CurrentTabFeatures().site_engagement_score, 0); EXPECT_EQ(CurrentTabFeatures().site_engagement_score, 0);
SiteEngagementService::Get(profile())->ResetBaseScoreForURL( SiteEngagementService::Get(profile())->ResetBaseScoreForURL(
GURL(kChromiumUrl), 91); GURL(kChromiumUrl), 91);
...@@ -220,24 +194,24 @@ TEST_F(TabMetricsLoggerTest, MAYBE_GetSiteEngagementScore) { ...@@ -220,24 +194,24 @@ TEST_F(TabMetricsLoggerTest, MAYBE_GetSiteEngagementScore) {
} }
// Tests was_recently_audible. // Tests was_recently_audible.
TEST_F(TabMetricsLoggerTest, MAYBE_GetAudibleState) { TEST_F(TabMetricsLoggerTest, GetAudibleState) {
EXPECT_FALSE(CurrentTabFeatures().was_recently_audible); EXPECT_FALSE(CurrentTabFeatures().was_recently_audible);
web_contents_tester_->SetIsCurrentlyAudible(true); web_contents_tester_->SetIsCurrentlyAudible(true);
EXPECT_TRUE(CurrentTabFeatures().was_recently_audible); EXPECT_TRUE(CurrentTabFeatures().was_recently_audible);
} }
// Tests host. // Tests host.
TEST_F(TabMetricsLoggerTest, MAYBE_GetHost) { TEST_F(TabMetricsLoggerTest, GetHost) {
EXPECT_EQ(CurrentTabFeatures().host, kChromiumDomain); EXPECT_EQ(CurrentTabFeatures().host, kChromiumDomain);
} }
// Tests creating a flat TabFeatures structure for logging a tab and its // Tests creating a flat TabFeatures structure for logging a tab and its
// TabMetrics state. // TabMetrics state.
TEST_F(TabMetricsLoggerTest, MAYBE_GetTabFeatures) { TEST_F(TabMetricsLoggerTest, GetTabFeatures) {
TabActivitySimulator tab_activity_simulator; TabActivitySimulator tab_activity_simulator;
Browser::CreateParams params(profile(), true); Browser::CreateParams params(profile(), true);
std::unique_ptr<Browser> browser = std::unique_ptr<Browser> browser =
CreateBrowserWithTestWindowForParams(&params); FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params);
TabStripModel* tab_strip_model = browser->tab_strip_model(); TabStripModel* tab_strip_model = browser->tab_strip_model();
// Add a foreground tab. // Add a foreground tab.
...@@ -430,10 +404,10 @@ TEST_F(TabMetricsLoggerUKMTest, LogForegroundedOrClosedMetrics) { ...@@ -430,10 +404,10 @@ TEST_F(TabMetricsLoggerUKMTest, LogForegroundedOrClosedMetrics) {
} }
// Tests CreateWindowFeatures of two browser windows. // Tests CreateWindowFeatures of two browser windows.
TEST_F(TabMetricsLoggerTest, MAYBE_CreateWindowFeaturesTest) { TEST_F(TabMetricsLoggerTest, CreateWindowFeaturesTest) {
Browser::CreateParams params(profile(), true); Browser::CreateParams params(profile(), true);
std::unique_ptr<Browser> browser = std::unique_ptr<Browser> browser =
FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(&params); FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params);
AddTab(browser.get()); AddTab(browser.get());
WindowFeatures expected_metrics{WindowMetricsEvent::TYPE_TABBED, WindowFeatures expected_metrics{WindowMetricsEvent::TYPE_TABBED,
...@@ -456,7 +430,7 @@ TEST_F(TabMetricsLoggerTest, MAYBE_CreateWindowFeaturesTest) { ...@@ -456,7 +430,7 @@ TEST_F(TabMetricsLoggerTest, MAYBE_CreateWindowFeaturesTest) {
// Create a second browser. // Create a second browser.
Browser::CreateParams params_2(Browser::TYPE_POPUP, profile(), true); Browser::CreateParams params_2(Browser::TYPE_POPUP, profile(), true);
std::unique_ptr<Browser> browser_2 = std::unique_ptr<Browser> browser_2 =
FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(&params_2); FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params_2);
AddTab(browser_2.get()); AddTab(browser_2.get());
WindowFeatures expected_metrics_2{WindowMetricsEvent::TYPE_POPUP, WindowFeatures expected_metrics_2{WindowMetricsEvent::TYPE_POPUP,
...@@ -482,11 +456,10 @@ TEST_F(TabMetricsLoggerTest, MAYBE_CreateWindowFeaturesTest) { ...@@ -482,11 +456,10 @@ TEST_F(TabMetricsLoggerTest, MAYBE_CreateWindowFeaturesTest) {
} }
// Tests moving a tab between browser windows. // Tests moving a tab between browser windows.
TEST_F(TabMetricsLoggerTest, TEST_F(TabMetricsLoggerTest, CreateWindowFeaturesTestMoveTabToOtherWindow) {
MAYBE_CreateWindowFeaturesTestMoveTabToOtherWindow) {
Browser::CreateParams params(profile(), true); Browser::CreateParams params(profile(), true);
std::unique_ptr<Browser> starting_browser = std::unique_ptr<Browser> starting_browser =
FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(&params); FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params);
AddTab(starting_browser.get()); AddTab(starting_browser.get());
WindowFeatures starting_browser_metrics{WindowMetricsEvent::TYPE_TABBED, WindowFeatures starting_browser_metrics{WindowMetricsEvent::TYPE_TABBED,
WindowMetricsEvent::SHOW_STATE_NORMAL, WindowMetricsEvent::SHOW_STATE_NORMAL,
...@@ -515,7 +488,7 @@ TEST_F(TabMetricsLoggerTest, ...@@ -515,7 +488,7 @@ TEST_F(TabMetricsLoggerTest,
// Create a new Browser for the tab. // Create a new Browser for the tab.
std::unique_ptr<Browser> created_browser = std::unique_ptr<Browser> created_browser =
FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(&params); FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params);
created_browser->window()->Activate(); created_browser->window()->Activate();
created_browser->tab_strip_model()->InsertWebContentsAt( created_browser->tab_strip_model()->InsertWebContentsAt(
0, std::move(dragged_tab), TabStripModel::ADD_ACTIVE); 0, std::move(dragged_tab), TabStripModel::ADD_ACTIVE);
...@@ -531,10 +504,10 @@ TEST_F(TabMetricsLoggerTest, ...@@ -531,10 +504,10 @@ TEST_F(TabMetricsLoggerTest,
} }
// Tests replacing a tab. // Tests replacing a tab.
TEST_F(TabMetricsLoggerTest, MAYBE_CreateWindowFeaturesTestReplaceTab) { TEST_F(TabMetricsLoggerTest, CreateWindowFeaturesTestReplaceTab) {
Browser::CreateParams params(profile(), true); Browser::CreateParams params(profile(), true);
std::unique_ptr<Browser> browser = std::unique_ptr<Browser> browser =
FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(&params); FakeBrowserWindow::CreateBrowserWithFakeWindowForParams(params);
AddTab(browser.get()); AddTab(browser.get());
WindowFeatures expected_metrics{WindowMetricsEvent::TYPE_TABBED, WindowFeatures expected_metrics{WindowMetricsEvent::TYPE_TABBED,
WindowMetricsEvent::SHOW_STATE_NORMAL, true, WindowMetricsEvent::SHOW_STATE_NORMAL, true,
......
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