mac: refactor CocoaTest into CocoaTestHelper

This change:
1) Moves the bulk of CocoaTest to a separate class, CocoaTestHelper, that
   can be held as a member
2) Removes CocoaTest::Init, which was misnamed and mostly unused, and
   replaces it with MarkCurrentWindowsAsInitial for the one use case
   that does need it
3) Removes a lot of testing macros from the CocoaTest header since they
   are either unused or only used in a single place
4) Also creates a //chrome CocoaTestHelper analog that bootstraps the
   Cocoa app bundle, for use in tests that need Cocoa but don't inherit
   from CocoaTest (eg because they inherit from another fixture)

Bug: 39725
Change-Id: I463480fbdbe4942561269478dc2f396db8ea8fb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107766Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752000}

chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm

@@ -26,7 +26,7 @@ class ColorPanelCocoaTest : public CocoaTest {
     // without this step the tests will fail complaining that not all windows
     // were closed.
     [[NSColorPanel sharedColorPanel] makeKeyAndOrderFront:nil];
-    Init();
+    MarkCurrentWindowsAsInitial();
   }
   base::test::TaskEnvironment task_environment_;
 };

chrome/browser/ui/cocoa/test/cocoa_test_helper.h

@@ -9,6 +9,12 @@
 
 #import "ui/base/test/cocoa_helper.h"
 
+class CocoaTestHelper : public ui::CocoaTestHelper {
+ public:
+  CocoaTestHelper();
+  ~CocoaTestHelper() override;
+};
+
 // A test class that all tests that depend on AppKit should inherit from.
 // Sets up paths correctly, and makes sure that any windows created in the test
 // are closed down properly by the test. If you need to inherit from a

chrome/browser/ui/cocoa/test/cocoa_test_helper.mm

@@ -8,10 +8,14 @@
 #include "base/path_service.h"
 #include "chrome/common/chrome_constants.h"
 
+CocoaTestHelper::CocoaTestHelper() {
+  CocoaTest::BootstrapCocoa();
+}
+
+CocoaTestHelper::~CocoaTestHelper() = default;
+
 CocoaTest::CocoaTest() {
   BootstrapCocoa();
-
-  Init();
 }
 
 void CocoaTest::BootstrapCocoa() {

ui/base/cocoa/base_view_unittest.mm

@@ -25,7 +25,12 @@ class BaseViewTest : public ui::CocoaTest {
   BaseView* view_;  // weak
 };
 
-TEST_VIEW(BaseViewTest, view_)
+TEST_F(BaseViewTest, RemoveFromSuperviewWorks) {
+  base::scoped_nsobject<NSView> view([view_ retain]);
+  EXPECT_EQ([test_window() contentView], [view superview]);
+  [view removeFromSuperview];
+  EXPECT_FALSE([view superview]);
+}
 
 // Convert a rect in |view_|'s Cocoa coordinate system to gfx::Rect's top-left
 // coordinate system. Repeat the process in reverse and make sure we come out

ui/base/test/cocoa_helper.h

@@ -69,33 +69,19 @@
 
 namespace ui {
 
-// A test class that all tests that depend on AppKit should inherit from.
-// Sets up paths correctly, and makes sure that any windows created in the test
-// are closed down properly by the test.
-class CocoaTest : public PlatformTest {
+class CocoaTestHelper {
  public:
-  CocoaTest();
-  ~CocoaTest() override;
+  CocoaTestHelper();
+  virtual ~CocoaTestHelper();
 
-  // Must be called by subclasses that override TearDown. We verify that it
-  // is called in our destructor. Takes care of making sure that all windows
-  // are closed off correctly. If your tests open windows, they must be sure
-  // to close them before CocoaTest::TearDown is called. A standard way of doing
-  // this would be to create them in SetUp (after calling CocoaTest::Setup) and
-  // then close them in TearDown before calling CocoaTest::TearDown.
-  void TearDown() override;
+  void MarkCurrentWindowsAsInitial();
 
-  // Retuns a test window that can be used by views and other UI objects
+  // Returns a test window that can be used by views and other UI objects
   // as part of their tests. Is created lazily, and will be closed correctly
   // in CocoaTest::TearDown. Note that it is a CocoaTestHelperWindow which
   // has special handling for being Key.
   CocoaTestHelperWindow* test_window();
 
- protected:
-  // Allows subclasses to do initialization before calling through to the base
-  // class's initialization.
-  void Init();
-
  private:
   // Return a set of currently open windows. Avoiding NSArray so
   // contents aren't retained, the pointer values can only be used for
@@ -107,7 +93,6 @@ class CocoaTest : public PlatformTest {
   // not |initial_windows_|.
   std::set<NSWindow*> WindowsLeft();
 
-  bool called_tear_down_;
   base::mac::ScopedNSAutoreleasePool pool_;
 
   // Windows which existed at the beginning of the test.
@@ -120,53 +105,34 @@ class CocoaTest : public PlatformTest {
   // It isn't wrapped in a different wrapper class to close it because we
   // need to close it at a very specific time; just before we enter our clean
   // up loop in TearDown.
-  CocoaTestHelperWindow* test_window_;
+  CocoaTestHelperWindow* test_window_ = nil;
 };
 
-}  // namespace ui
+// A test class that all tests that depend on AppKit should inherit from.
+// Sets up paths correctly, and makes sure that any windows created in the test
+// are closed down properly by the test.
+class CocoaTest : public PlatformTest {
+ public:
+  CocoaTest();
+  ~CocoaTest() override;
 
-// A macro defining a standard set of tests to run on a view. Since we can't
-// inherit tests, this macro saves us a lot of duplicate code. Handles simply
-// displaying the view to make sure it won't crash, as well as removing it
-// from a window. All tests that work with NSView subclasses and/or
-// NSViewController subclasses should use it.
-#define TEST_VIEW(test_fixture, test_view)                      \
-  TEST_F(test_fixture, test_fixture##_TestViewMacroAddRemove) { \
-    base::scoped_nsobject<NSView> view([test_view retain]);     \
-    EXPECT_EQ([test_window() contentView], [view superview]);   \
-    [view removeFromSuperview];                                 \
-    EXPECT_FALSE([view superview]);                             \
-  }                                                             \
-  TEST_F(test_fixture, test_fixture##_TestViewMacroDisplay) {   \
-    [test_view display];                                        \
-  }
-
-// A macro which determines the proper float epsilon for a CGFloat.
-#if CGFLOAT_IS_DOUBLE
-#define CGFLOAT_EPSILON DBL_EPSILON
-#else
-#define CGFLOAT_EPSILON FLT_EPSILON
-#endif
-
-// A macro which which determines if two CGFloats are equal taking a
-// proper epsilon into consideration.
-#define CGFLOAT_EQ(expected, actual)         \
-  (actual >= (expected - CGFLOAT_EPSILON) && \
-   actual <= (expected + CGFLOAT_EPSILON))
-
-// A test support macro which ascertains if two CGFloats are equal.
-#define EXPECT_CGFLOAT_EQ(expected, actual) \
-  EXPECT_TRUE(CGFLOAT_EQ(expected, actual)) << expected << " != " << actual
-
-// A test support macro which compares two NSRects for equality taking
-// the float epsilon into consideration.
-#define EXPECT_NSRECT_EQ(expected, actual)                          \
-  EXPECT_TRUE(CGFLOAT_EQ(expected.origin.x, actual.origin.x) &&     \
-              CGFLOAT_EQ(expected.origin.y, actual.origin.y) &&     \
-              CGFLOAT_EQ(expected.size.width, actual.size.width) && \
-              CGFLOAT_EQ(expected.size.height, actual.size.height)) \
-      << "Rects do not match: "                                     \
-      << base::SysNSStringToUTF8(NSStringFromRect(expected))        \
-      << " != " << base::SysNSStringToUTF8(NSStringFromRect(actual))
+  // Must be called by subclasses that override TearDown. We verify that it
+  // is called in our destructor. Takes care of making sure that all windows
+  // are closed off correctly. If your tests open windows, they must be sure
+  // to close them before CocoaTest::TearDown is called. A standard way of doing
+  // this would be to create them in SetUp (after calling CocoaTest::Setup) and
+  // then close them in TearDown before calling CocoaTest::TearDown.
+  void TearDown() override;
+
+  CocoaTestHelperWindow* test_window() { return helper_->test_window(); }
+
+ protected:
+  void MarkCurrentWindowsAsInitial();
+
+ private:
+  std::unique_ptr<CocoaTestHelper> helper_;
+};
+
+}  // namespace ui
 
 #endif  // UI_BASE_TEST_COCOA_HELPER_H_

ui/base/test/cocoa_helper.mm

@@ -133,17 +133,8 @@ void NOINLINE ForceSystemLeaks() {
 
 namespace ui {
 
-CocoaTest::CocoaTest() : called_tear_down_(false), test_window_(nil) {
+CocoaTestHelper::CocoaTestHelper() {
   ForceSystemLeaks();
-  Init();
-}
-
-CocoaTest::~CocoaTest() {
-  // Must call CocoaTest's teardown from your overrides.
-  DCHECK(called_tear_down_);
-}
-
-void CocoaTest::Init() {
   // Set the duration of AppKit-evaluated animations (such as frame changes)
   // to zero for testing purposes. That way they take effect immediately.
   [[NSAnimationContext currentContext] setDuration:0.0];
@@ -155,14 +146,10 @@ void CocoaTest::Init() {
   NSDictionary* dict = @{@"NSWindowResizeTime" : @"0.01"};
   [[NSUserDefaults standardUserDefaults] registerDefaults:dict];
 
-  // Collect the list of windows that were open when the test started so
-  // that we don't wait for them to close in TearDown. Has to be done
-  // after BootstrapCocoa is called.
-  initial_windows_ = ApplicationWindows();
+  MarkCurrentWindowsAsInitial();
 }
 
-void CocoaTest::TearDown() {
-  called_tear_down_ = true;
+CocoaTestHelper::~CocoaTestHelper() {
   // Call close on our test_window to clean it up if one was opened.
   [test_window_ clearPretendKeyWindowAndFirstResponder];
   [test_window_ close];
@@ -243,10 +230,16 @@ void CocoaTest::TearDown() {
 
     windows_left = still_left;
   }
-  PlatformTest::TearDown();
 }
 
-std::set<NSWindow*> CocoaTest::ApplicationWindows() {
+void CocoaTestHelper::MarkCurrentWindowsAsInitial() {
+  // Collect the list of windows that were open when the test started so
+  // that we don't wait for them to close in TearDown. Has to be done
+  // after BootstrapCocoa is called.
+  initial_windows_ = ApplicationWindows();
+}
+
+std::set<NSWindow*> CocoaTestHelper::ApplicationWindows() {
   // This must NOT retain the windows it is returning.
   std::set<NSWindow*> windows;
 
@@ -261,14 +254,14 @@ std::set<NSWindow*> CocoaTest::ApplicationWindows() {
   }
 }
 
-std::set<NSWindow*> CocoaTest::WindowsLeft() {
+std::set<NSWindow*> CocoaTestHelper::WindowsLeft() {
   const std::set<NSWindow*> windows(ApplicationWindows());
   std::set<NSWindow*> windows_left =
       base::STLSetDifference<std::set<NSWindow*>>(windows, initial_windows_);
   return windows_left;
 }
 
-CocoaTestHelperWindow* CocoaTest::test_window() {
+CocoaTestHelperWindow* CocoaTestHelper::test_window() {
   if (!test_window_) {
     test_window_ = [[CocoaTestHelperWindow alloc] init];
     if (base::debug::BeingDebugged()) {
@@ -280,4 +273,18 @@ CocoaTestHelperWindow* CocoaTest::test_window() {
   return test_window_;
 }
 
+CocoaTest::CocoaTest() : helper_(std::make_unique<CocoaTestHelper>()) {}
+CocoaTest::~CocoaTest() {
+  CHECK(!helper_);
+}
+
+void CocoaTest::TearDown() {
+  helper_.reset();
+  PlatformTest::TearDown();
+}
+
+void CocoaTest::MarkCurrentWindowsAsInitial() {
+  helper_->MarkCurrentWindowsAsInitial();
+}
+
 }  // namespace ui