[AiS] changing mac omnibox suggestions form NSMatrix to NSTableView

chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell_unittest.mm

Index: chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell_unittest.mm
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell_unittest.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell_unittest.mm
index 0367a8494848f23f95d9900e1bfeaaead40bc29a..668d57fc80458edc0c33248adbff53d37b45622f 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell_unittest.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell_unittest.mm
@@ -11,6 +11,12 @@
 #include "components/omnibox/suggestion_answer.h"
 #import "testing/gtest_mac.h"
 
+@interface OmniboxPopupCellData ()
+- (void)setContents:(NSAttributedString*)contents;
+- (NSAttributedString*)description;

[groby-ooo-7-16, 2015/06/11 01:22:17]

That's already defined in an anonymous category in the main file, so probably
should be a public method.

[Scott Hess - ex-Googler, 2015/06/11 21:52:02]

Any concerns about this overriding NSObject protocol's -description method?  I
don't think it would cause problems, I just initially read this differently than
I think you wrote it, and that made me wonder :-).

[dschuyler, 2015/06/11 22:34:22]

Done.

[dschuyler, 2015/06/11 22:34:22]

While I'm ok with it being description, I can also change it - if you'd like it
changed, a suggestion on what to change it to would be great :)

+- (void)setImage:(NSImage*)image;

[groby-ooo-7-16, 2015/06/11 01:22:17]

Instead of having setters on what's otherwise an immutable object, I'd suggest 

- (instanceType)initWithDescription:(NSAttributedString*)string
image:(NSImage*)image

And just build and set the celldata as you need it. 

[Scott Hess - ex-Googler, 2015/06/11 21:52:02]

I agree, for a data object you should either aim for fully-mutable or
fully-immutable, and in this case it really doesn't need to be mutable except
for a setup-time wart.

[dschuyler, 2015/06/11 22:34:22]

I used the existing initWithMatch, with the hope that's ok.

Done.

[dschuyler, 2015/06/11 22:34:22]

I think we're going in the fully-immutable direction (though there is still a
test function that cheats and another that deals with the prefix that I'm trying
not to change too much in this CL).

+@end
+
 namespace {
 
 class OmniboxPopupCellTest : public CocoaTest {
@@ -20,32 +26,35 @@ class OmniboxPopupCellTest : public CocoaTest {
 
   void SetUp() override {
     CocoaTest::SetUp();
-    cell_.reset([[OmniboxPopupCell alloc] initTextCell:@""]);
-    button_.reset([[NSButton alloc] initWithFrame:NSMakeRect(0, 0, 200, 20)]);
-    [button_ setCell:cell_];
-    [[test_window() contentView] addSubview:button_];
+    cellData_.reset([[OmniboxPopupCellData alloc] init]);
+    cell_.reset([[OmniboxPopupCell alloc] init]);
+    [cell_ setObjectValue:cellData_];
+    control_.reset([[NSControl alloc] initWithFrame:NSMakeRect(0, 0, 200, 20)]);
+    [control_ setCell:cell_];
+    [[test_window() contentView] addSubview:control_];
   };
 
  protected:
+  base::scoped_nsobject<OmniboxPopupCellData> cellData_;
   base::scoped_nsobject<OmniboxPopupCell> cell_;
-  base::scoped_nsobject<NSButton> button_;
+  base::scoped_nsobject<NSControl> control_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(OmniboxPopupCellTest);
 };
 
-TEST_VIEW(OmniboxPopupCellTest, button_);
+TEST_VIEW(OmniboxPopupCellTest, control_);
 
 TEST_F(OmniboxPopupCellTest, Image) {
-  [cell_ setImage:[NSImage imageNamed:NSImageNameInfo]];
-  [button_ display];
+  [cellData_ setImage:[NSImage imageNamed:NSImageNameInfo]];

[groby-ooo-7-16, 2015/06/11 01:22:17]

This doesn't what you expect it to do - you already did a setObjectValue: on
cellData, which created a copy.

[dschuyler, 2015/06/11 22:34:22]

Done.

+  [control_ display];
 }
 
 TEST_F(OmniboxPopupCellTest, Title) {
   base::scoped_nsobject<NSAttributedString> text([[NSAttributedString alloc]
       initWithString:@"The quick brown fox jumps over the lazy dog."]);
-  [cell_ setAttributedTitle:text];
-  [button_ display];
+  [cellData_ setContents:text];
+  [control_ display];
 }
 
 TEST_F(OmniboxPopupCellTest, AnswerStyle) {
@@ -66,17 +75,17 @@ TEST_F(OmniboxPopupCellTest, AnswerStyle) {
   AutocompleteMatch match;
   match.answer = SuggestionAnswer::ParseAnswer(dictionary);
   EXPECT_TRUE(match.answer);
-  [cell_ setMatch:match];
-  EXPECT_NSEQ([[cell_ description] string], finalString);
-  size_t length = [[cell_ description] length];
+  cellData_.reset([[OmniboxPopupCellData alloc] initWithMatch:match image:nil]);
+  EXPECT_NSEQ([[cellData_ description] string], finalString);
+  size_t length = [[[cellData_ description] string] length];
   const NSRange checkValues[] = {{0, 2}, {2, 2}, {4, 4}};
   EXPECT_EQ(length, 8UL);
   NSDictionary* lastAttributes = nil;
   for (const NSRange& value : checkValues) {
     NSRange range;
     NSDictionary* currentAttributes =
-        [[cell_ description] attributesAtIndex:value.location
-                                effectiveRange:&range];
+        [[cellData_ description] attributesAtIndex:value.location
+                                    effectiveRange:&range];
     EXPECT_TRUE(NSEqualRanges(value, range));
     EXPECT_FALSE([currentAttributes isEqualToDictionary:lastAttributes]);
     lastAttributes = currentAttributes;