Moved a whole pile of unittests over to CocoaTest.

Moved a whole pile of unittests over to CocoaTest.
AboutIPC.xib - Connected up controller to its window and the window delegate to the controller.

BUG=26484, 26484
TEST=See repro steps in bugs.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=32551

[rohitrao (ping after 24h), 2009-11-19 06:30:46]

Made it from A-H.  More to come.

http://codereview.chromium.org/402066/diff/1089/45
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/402066/diff/1089/45#newcode696
Line 696: prefsController_ = nil;
Should we have a "// Will autorelease itself" comment here, or is this a
commonly-enough-used pattern that it's unnecessary?

http://codereview.chromium.org/402066/diff/1089/69
File chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/69#newcode49
Line 49: virtual void SetUp() {
Is there any particular reason you're moving all of this code into SetUp()
rather than leaving it in the constructor?  (We looked this up once and they're
roughly equivalent, so it doesn't really matter.  Just wondering.)

http://codereview.chromium.org/402066/diff/1089/69#newcode128
Line 128: TEST_VIEW(AutocompleteTextFieldEditorTest, field_);
Please move this up to be the first test.

http://codereview.chromium.org/402066/diff/1089/54
File chrome/browser/cocoa/autocomplete_text_field_unittest.mm (left):

http://codereview.chromium.org/402066/diff/1089/54#oldcode92
Line 92: [cocoa_helper_.window() setDelegate:nil];
Why don't we need to reset the delegate?  Does the delegate outlive the window
now?

http://codereview.chromium.org/402066/diff/1089/87
File chrome/browser/cocoa/autocomplete_text_field_unittest_helper.mm (left):

http://codereview.chromium.org/402066/diff/1089/87#oldcode14
Line 14: EXPECT_TRUE([anObject isKindOfClass:[AutocompleteTextField class]]);
Is this getting called for non-AutocompleteTextFields?

http://codereview.chromium.org/402066/diff/1089/71
File chrome/browser/cocoa/bookmark_bar_view_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/71#newcode73
Line 73: view_.reset([[BookmarkBarView alloc] init]);
Given that view_ isn't even initialized in the fixture, I would rather just put
the scoped_nsobject into each test:
scoped_nsobject<BookmarkBarView> view([[BookmarkBarView alloc] init]);

http://codereview.chromium.org/402066/diff/1089/71#newcode78
Line 78: view_.reset([[BookmarkBarView alloc] init]);
Also here.

http://codereview.chromium.org/402066/diff/1089/62
File chrome/browser/cocoa/bookmark_button_cell_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/62#newcode16
Line 16: NSButton* view_;
Same here, I don't see the point of putting this into the fixture if it's never
used outside of a test.

http://codereview.chromium.org/402066/diff/1089/64
File chrome/browser/cocoa/browser_window_cocoa_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/64#newcode63
Line 63: controller_);
I would put both arguments on a single line.  Also below.

http://codereview.chromium.org/402066/diff/1089/67
File chrome/browser/cocoa/focus_tracker_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/67#newcode35
Line 35: ASSERT_TRUE([window makeFirstResponder:viewA_]);
Do these tests pass?  We don't need any of that pretend key code here, do we?

http://codereview.chromium.org/402066/diff/1089/67#newcode62
Line 62: scoped_nsobject<NSView> view3([[NSView alloc]
initWithFrame:NSZeroRect]);
Hah, when I wrote this test, I made myself viewA, viewB, and view3.  Go me.

http://codereview.chromium.org/402066/diff/1089/66
File chrome/browser/cocoa/fullscreen_window_unittest.mm (left):

http://codereview.chromium.org/402066/diff/1089/66#oldcode24
Line 24: EXPECT_FALSE([window_ isReleasedWhenClosed]);
Why remove this?  I *think* it was an important feature of the fullscreen
window.

http://codereview.chromium.org/402066/diff/1089/53
File chrome/browser/cocoa/hyperlink_button_cell_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/53#newcode28
Line 28: void TestCellCustomization(HyperlinkButtonCell *cell) {
HyperlinkButtonCell* cell

[dmac, 2009-11-19 18:35:28]

New version up.

http://codereview.chromium.org/402066/diff/1089/45
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/402066/diff/1089/45#newcode696
Line 696: prefsController_ = nil;
On 2009/11/19 06:30:46, rohitrao wrote:
> Should we have a "// Will autorelease itself" comment here, or is this a
> commonly-enough-used pattern that it's unnecessary?

Done.

http://codereview.chromium.org/402066/diff/1089/69
File chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/69#newcode49
Line 49: virtual void SetUp() {
On 2009/11/19 06:30:46, rohitrao wrote:
> Is there any particular reason you're moving all of this code into SetUp()
> rather than leaving it in the constructor?  (We looked this up once and
they're
> roughly equivalent, so it doesn't really matter.  Just wondering.)

I wanted all of the Cocoa Stuff below that had to do with setting up views and
windows to occur after CocoaTest::SetUp was done.

http://codereview.chromium.org/402066/diff/1089/69#newcode128
Line 128: TEST_VIEW(AutocompleteTextFieldEditorTest, field_);
On 2009/11/19 06:30:46, rohitrao wrote:
> Please move this up to be the first test.

Done.

http://codereview.chromium.org/402066/diff/1089/54
File chrome/browser/cocoa/autocomplete_text_field_unittest.mm (left):

http://codereview.chromium.org/402066/diff/1089/54#oldcode92
Line 92: [cocoa_helper_.window() setDelegate:nil];
On 2009/11/19 06:30:46, rohitrao wrote:
> Why don't we need to reset the delegate?  Does the delegate outlive the window
> now?

Yes

http://codereview.chromium.org/402066/diff/1089/87
File chrome/browser/cocoa/autocomplete_text_field_unittest_helper.mm (left):

http://codereview.chromium.org/402066/diff/1089/87#oldcode14
Line 14: EXPECT_TRUE([anObject isKindOfClass:[AutocompleteTextField class]]);
On 2009/11/19 06:30:46, rohitrao wrote:
> Is this getting called for non-AutocompleteTextFields?

Yes.. in the removeview case we will call this with a non-autocompletetextfield
as it attempts to find a new field editor.

http://codereview.chromium.org/402066/diff/1089/71
File chrome/browser/cocoa/bookmark_bar_view_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/71#newcode73
Line 73: view_.reset([[BookmarkBarView alloc] init]);
On 2009/11/19 06:30:46, rohitrao wrote:
> Given that view_ isn't even initialized in the fixture, I would rather just
put
> the scoped_nsobject into each test:
> scoped_nsobject<BookmarkBarView> view([[BookmarkBarView alloc] init]);

Done.

http://codereview.chromium.org/402066/diff/1089/71#newcode78
Line 78: view_.reset([[BookmarkBarView alloc] init]);
On 2009/11/19 06:30:46, rohitrao wrote:
> Also here.

Done.

http://codereview.chromium.org/402066/diff/1089/62
File chrome/browser/cocoa/bookmark_button_cell_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/62#newcode16
Line 16: NSButton* view_;
On 2009/11/19 06:30:46, rohitrao wrote:
> Same here, I don't see the point of putting this into the fixture if it's
never
> used outside of a test.

Done.

http://codereview.chromium.org/402066/diff/1089/67
File chrome/browser/cocoa/focus_tracker_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/67#newcode35
Line 35: ASSERT_TRUE([window makeFirstResponder:viewA_]);
On 2009/11/19 06:30:46, rohitrao wrote:
> Do these tests pass?  We don't need any of that pretend key code here, do we?

They pass and work fine.

http://codereview.chromium.org/402066/diff/1089/67#newcode62
Line 62: scoped_nsobject<NSView> view3([[NSView alloc]
initWithFrame:NSZeroRect]);
On 2009/11/19 06:30:46, rohitrao wrote:
> Hah, when I wrote this test, I made myself viewA, viewB, and view3.  Go me.

Done.

http://codereview.chromium.org/402066/diff/1089/66
File chrome/browser/cocoa/fullscreen_window_unittest.mm (left):

http://codereview.chromium.org/402066/diff/1089/66#oldcode24
Line 24: EXPECT_FALSE([window_ isReleasedWhenClosed]);
On 2009/11/19 06:30:46, rohitrao wrote:
> Why remove this?  I *think* it was an important feature of the fullscreen
> window.
 I readded it.

http://codereview.chromium.org/402066/diff/1089/53
File chrome/browser/cocoa/hyperlink_button_cell_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/53#newcode28
Line 28: void TestCellCustomization(HyperlinkButtonCell *cell) {
On 2009/11/19 06:30:46, rohitrao wrote:
> HyperlinkButtonCell* cell

Done.

[rohitrao (ping after 24h), 2009-11-19 21:10:22]

LGTM

http://codereview.chromium.org/402066/diff/1089/50
File chrome/browser/cocoa/page_info_window_controller_unittest.mm (left):

http://codereview.chromium.org/402066/diff/1089/50#oldcode12
Line 12: #include "testing/gtest/include/gtest/gtest.h"
We're not particularly consistent about whether or not to include gtest.h and
platform_test.h.  Is there a "correct" answer?

http://codereview.chromium.org/402066/diff/1089/92
File chrome/browser/cocoa/preferences_window_controller_unittest.mm (right):

http://codereview.chromium.org/402066/diff/1089/92#newcode75
Line 75: pref_controller_ = nil;
Ah, I see.  This line prevents us from doing a second close in TearDown().