Commit 640f197a authored by Bret Sepulveda's avatar Bret Sepulveda Committed by Commit Bot

Reland: Various refactoring in TabStripModel tests.

This patch refactors tab_strip_model_unittest.cc and some associated
files in several ways:
* Removes ChromeRenderViewHostTestHarness as a superclass of
  TabStripModelTest, which includes some heavyweight dependencies that
  are unused. This speeds up the suite by about 20% (~6500 ms runtime
  vs. ~8000ms before this patch).
* Changes TestTabStripModelDelegate::RunUnloadListenerBeforeClosing to
  return false, and deletes subclasses that exist solely to override
  that method (all of them, except TabStripDummyDelegate, which is
  renamed to UnloadListenerTabStripModelDelegate).
* Moved several helper classes to be defined immediately before the
  test that uses them.
* Deletes member variables from MockTabStripModelObserver, which are
  unused.
* Fixes lint errors and cleans up unused includes.

Change-Id: Ibb93ff41a21aab4c0b9b9f075df315381959c35c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538575Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Reviewed-by: default avatarErik Chen <erikchen@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644437}
parent 7e0371b6
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h" #include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h"
#include <memory> #include <memory>
#include <utility>
#include <vector>
#include "base/bind.h" #include "base/bind.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -41,18 +43,6 @@ namespace { ...@@ -41,18 +43,6 @@ namespace {
constexpr base::TimeDelta kShortDelay = base::TimeDelta::FromSeconds(1); constexpr base::TimeDelta kShortDelay = base::TimeDelta::FromSeconds(1);
class NoUnloadListenerTabStripModelDelegate : public TestTabStripModelDelegate {
public:
NoUnloadListenerTabStripModelDelegate() = default;
bool RunUnloadListenerBeforeClosing(content::WebContents* contents) override {
// The default TestTabStripModelDelegate prevents tabs from being closed.
return false;
}
private:
DISALLOW_COPY_AND_ASSIGN(NoUnloadListenerTabStripModelDelegate);
};
class MockLifecycleUnitSourceObserver : public LifecycleUnitSourceObserver { class MockLifecycleUnitSourceObserver : public LifecycleUnitSourceObserver {
public: public:
MockLifecycleUnitSourceObserver() = default; MockLifecycleUnitSourceObserver() = default;
...@@ -290,7 +280,7 @@ class TabLifecycleUnitSourceTest ...@@ -290,7 +280,7 @@ class TabLifecycleUnitSourceTest
ExpectCanDiscardFalseTrivialAllReasons(first_lifecycle_unit); ExpectCanDiscardFalseTrivialAllReasons(first_lifecycle_unit);
// Create a second tab strip. // Create a second tab strip.
NoUnloadListenerTabStripModelDelegate other_tab_strip_model_delegate; TestTabStripModelDelegate other_tab_strip_model_delegate;
TabStripModel other_tab_strip_model(&other_tab_strip_model_delegate, TabStripModel other_tab_strip_model(&other_tab_strip_model_delegate,
profile()); profile());
other_tab_strip_model.AddObserver(source_); other_tab_strip_model.AddObserver(source_);
...@@ -457,7 +447,7 @@ class TabLifecycleUnitSourceTest ...@@ -457,7 +447,7 @@ class TabLifecycleUnitSourceTest
return web_contents; return web_contents;
} }
NoUnloadListenerTabStripModelDelegate tab_strip_model_delegate_; TestTabStripModelDelegate tab_strip_model_delegate_;
ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_; ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
DISALLOW_COPY_AND_ASSIGN(TabLifecycleUnitSourceTest); DISALLOW_COPY_AND_ASSIGN(TabLifecycleUnitSourceTest);
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <algorithm> #include <algorithm>
#include <map> #include <map>
#include <memory> #include <memory>
#include <utility>
#include <vector> #include <vector>
#include "base/bind.h" #include "base/bind.h"
...@@ -98,14 +99,6 @@ class NonResumingBackgroundTabNavigationThrottle ...@@ -98,14 +99,6 @@ class NonResumingBackgroundTabNavigationThrottle
DISALLOW_COPY_AND_ASSIGN(NonResumingBackgroundTabNavigationThrottle); DISALLOW_COPY_AND_ASSIGN(NonResumingBackgroundTabNavigationThrottle);
}; };
class TabStripModelSimpleDelegate : public TestTabStripModelDelegate {
public:
TabStripModelSimpleDelegate() = default;
private:
DISALLOW_COPY_AND_ASSIGN(TabStripModelSimpleDelegate);
};
enum TestIndicies { enum TestIndicies {
kAutoDiscardable, kAutoDiscardable,
kPinned, kPinned,
......
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
#include "chrome/browser/ui/tabs/tab_strip_model_stats_recorder.h" #include "chrome/browser/ui/tabs/tab_strip_model_stats_recorder.h"
#include <memory>
#include <utility>
#include "base/macros.h" #include "base/macros.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
...@@ -21,21 +24,8 @@ using content::WebContents; ...@@ -21,21 +24,8 @@ using content::WebContents;
class TabStripModelStatsRecorderTest : public ChromeRenderViewHostTestHarness { class TabStripModelStatsRecorderTest : public ChromeRenderViewHostTestHarness {
}; };
class NoUnloadListenerTabStripModelDelegate : public TestTabStripModelDelegate {
public:
NoUnloadListenerTabStripModelDelegate() {}
~NoUnloadListenerTabStripModelDelegate() override {}
bool RunUnloadListenerBeforeClosing(WebContents* contents) override {
return false;
}
private:
DISALLOW_COPY_AND_ASSIGN(NoUnloadListenerTabStripModelDelegate);
};
TEST_F(TabStripModelStatsRecorderTest, BasicTabLifecycle) { TEST_F(TabStripModelStatsRecorderTest, BasicTabLifecycle) {
NoUnloadListenerTabStripModelDelegate delegate; TestTabStripModelDelegate delegate;
TabStripModel tabstrip(&delegate, profile()); TabStripModel tabstrip(&delegate, profile());
TabStripModelStatsRecorder recorder; TabStripModelStatsRecorder recorder;
...@@ -98,7 +88,7 @@ TEST_F(TabStripModelStatsRecorderTest, BasicTabLifecycle) { ...@@ -98,7 +88,7 @@ TEST_F(TabStripModelStatsRecorderTest, BasicTabLifecycle) {
} }
TEST_F(TabStripModelStatsRecorderTest, ObserveMultipleTabStrips) { TEST_F(TabStripModelStatsRecorderTest, ObserveMultipleTabStrips) {
NoUnloadListenerTabStripModelDelegate delegate; TestTabStripModelDelegate delegate;
TabStripModel tabstrip1(&delegate, profile()); TabStripModel tabstrip1(&delegate, profile());
TabStripModel tabstrip2(&delegate, profile()); TabStripModel tabstrip2(&delegate, profile());
...@@ -167,7 +157,7 @@ TEST_F(TabStripModelStatsRecorderTest, ObserveMultipleTabStrips) { ...@@ -167,7 +157,7 @@ TEST_F(TabStripModelStatsRecorderTest, ObserveMultipleTabStrips) {
TEST_F(TabStripModelStatsRecorderTest, TEST_F(TabStripModelStatsRecorderTest,
NumberOfOtherTabsActivatedBeforeMadeActive) { NumberOfOtherTabsActivatedBeforeMadeActive) {
NoUnloadListenerTabStripModelDelegate delegate; TestTabStripModelDelegate delegate;
TabStripModel tabstrip(&delegate, profile()); TabStripModel tabstrip(&delegate, profile());
TabStripModelStatsRecorder recorder; TabStripModelStatsRecorder recorder;
...@@ -200,7 +190,7 @@ TEST_F(TabStripModelStatsRecorderTest, ...@@ -200,7 +190,7 @@ TEST_F(TabStripModelStatsRecorderTest,
TEST_F(TabStripModelStatsRecorderTest, TEST_F(TabStripModelStatsRecorderTest,
NumberOfOtherTabsActivatedBeforeMadeActive_CycleTabs) { NumberOfOtherTabsActivatedBeforeMadeActive_CycleTabs) {
NoUnloadListenerTabStripModelDelegate delegate; TestTabStripModelDelegate delegate;
TabStripModel tabstrip(&delegate, profile()); TabStripModel tabstrip(&delegate, profile());
TabStripModelStatsRecorder recorder; TabStripModelStatsRecorder recorder;
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h" #include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h"
#include <vector>
#include "chrome/browser/extensions/tab_helper.h" #include "chrome/browser/extensions/tab_helper.h"
#include "chrome/browser/ui/tab_contents/core_tab_helper.h" #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
...@@ -55,7 +57,7 @@ bool TestTabStripModelDelegate::ShouldRunUnloadListenerBeforeClosing( ...@@ -55,7 +57,7 @@ bool TestTabStripModelDelegate::ShouldRunUnloadListenerBeforeClosing(
bool TestTabStripModelDelegate::RunUnloadListenerBeforeClosing( bool TestTabStripModelDelegate::RunUnloadListenerBeforeClosing(
content::WebContents* contents) { content::WebContents* contents) {
return true; return false;
} }
TabStripModelDelegate::RestoreTabType TabStripModelDelegate::RestoreTabType
......
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