Switch to using WebIntentPickerModel, and bring picker closer to mocks.

chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/cocoa_test_helper.h"
 #import "chrome/browser/ui/cocoa/hyperlink_button_cell.h"
 #import "chrome/browser/ui/cocoa/info_bubble_window.h"
 #import "chrome/browser/ui/cocoa/web_intent_bubble_controller.h"
 #include "chrome/browser/ui/cocoa/web_intent_picker_cocoa.h"
 #include "chrome/browser/ui/intents/web_intent_picker_delegate.h"
@@ skipping 32 matching lines @@
     controller_ =
         [[WebIntentBubbleController alloc] initWithPicker:picker_.get()
                                              parentWindow:test_window()
                                                anchoredAt:anchor];
     window_ = static_cast<InfoBubbleWindow*>([controller_ window]);
     [controller_ showWindow:nil];
   }
 
   // Checks the controller's window for the requisite subviews and icons.
   void CheckWindow(size_t icon_count) {
-    NSArray* views = [[window_ contentView] subviews];
+    NSArray* flip_views = [[window_ contentView] subviews];
 
-    // 4 subviews - Icon, Header text, NSMatrix, CWS link.
-    EXPECT_EQ(4U, [views count]);
-    EXPECT_TRUE([[views objectAtIndex:0] isKindOfClass:[NSButton class]]);
-    EXPECT_TRUE([[views objectAtIndex:1] isKindOfClass:[NSMatrix class]]);
-    EXPECT_TRUE([[views objectAtIndex:2] isKindOfClass:[NSTextField class]]);
-    EXPECT_TRUE([[views objectAtIndex:3] isKindOfClass:[NSImageView class]]);
+    // Expect 1 subview - the flip view

[Nico, 2012/02/03 23:27:07]

.

[groby-ooo-7-16, 2012/02/06 22:30:19]

Done.

+    ASSERT_EQ(1U, [flip_views count]);
+
+    NSArray* views = [[flip_views objectAtIndex:0] subviews];
+
+    // 3 + |icon_count| subviews - Icon, Header text, |icon_count| buttons,
+    // and a CWS link.
+    ASSERT_EQ(3U + icon_count, [views count]);
+
+    NSUInteger cws_index = 2 + icon_count;

[Nico, 2012/02/03 23:27:07]

You don't really use this. Maybe remove, and below the "// Verify" comment, do:

  NSButton* button = static_cast<NSButton*>([views lastObject]);
  EXPECT_TRUE([button isKindOfClass:[NSButton class]]);
  // ...rest as before.

[groby-ooo-7-16, 2012/02/06 22:30:19]

Done.

+
+    EXPECT_TRUE([[views objectAtIndex:0] isKindOfClass:[NSTextField class]]);
+    EXPECT_TRUE([[views objectAtIndex:1] isKindOfClass:[NSImageView class]]);
+    for(NSUInteger i = 0; i < icon_count; ++i) {
+      EXPECT_TRUE([[views objectAtIndex:2 + i] isKindOfClass:[NSButton class]]);
+    }
+    EXPECT_TRUE([[views objectAtIndex:cws_index]
+                    isKindOfClass:[NSButton class]]);
 
     // Verify the Chrome Web Store button.
-    NSButton* button = static_cast<NSButton*>([views objectAtIndex:0]);
+    NSButton* button = static_cast<NSButton*>([views objectAtIndex:cws_index]);
     EXPECT_TRUE([[button cell] isKindOfClass:[HyperlinkButtonCell class]]);
     CheckButton(button, @selector(showChromeWebStore:));
 
-    // Verify icons/buttons pointing to services.
-    NSMatrix* icon_matrix = static_cast<NSMatrix*>([views objectAtIndex:1]);
-    NSArray* cells = [icon_matrix cells];
-    size_t cell_count = 0;
-    for (NSButtonCell* cell in cells) {
-      // Skip not populated cells
-      if ([cell isKindOfClass:[NSImageCell class]])
-        continue;
-
-      ++cell_count;
-      CheckButton(cell, @selector(invokeService:));
+    // Verify buttons pointing to services.
+    for(NSUInteger i = 0; i < icon_count; ++i) {
+      NSButton* button = [views objectAtIndex:2 + i];
+      CheckServiceButton(button, i);

[Nico, 2012/02/03 23:27:07]

Can't you fold this loop into the previous loop?

[groby-ooo-7-16, 2012/02/06 22:30:19]

I didn't want to fold it up since technically the test blows up completely if
the classes don't match. Changed the class check to ASSERT_TRUE to make that
explicit.

On 2012/02/03 23:27:07, Nico wrote:

     }
-    // TODO(binji): This check needs to be disabled until the bubble controller
-    // is fixed to use the WebIntentPickerModel directly.
-    // EXPECT_EQ(icon_count, cell_count);
 
     EXPECT_EQ([window_ delegate], controller_);
   }
 
+  // Checks that a service button is hooked up correctly.
+  void CheckServiceButton(NSButton* button, NSUInteger service_index) {
+    CheckButton(button, @selector(invokeService:));
+    EXPECT_EQ(NSInteger(service_index), [button tag]);
+  }
   // Checks that a button is hooked up correctly.
   void CheckButton(id button, SEL action) {
     EXPECT_TRUE([button isKindOfClass:[NSButton class]] ||
       [button isKindOfClass:[NSButtonCell class]]);
     EXPECT_EQ(action, [button action]);
     EXPECT_EQ(controller_, [button target]);
     EXPECT_TRUE([button stringValue]);
   }
 
   WebIntentBubbleController* controller_;  // Weak, owns self.
   InfoBubbleWindow* window_;  // Weak, owned by controller.
   scoped_ptr<WebIntentPickerCocoa> picker_;
   FakeIntentPickerDelegate delegate_;
 };
 
 TEST_F(WebIntentBubbleControllerTest, EmptyBubble) {
   CreateBubble();
 
   CheckWindow(/*icon_count=*/0);
 }
 
 TEST_F(WebIntentBubbleControllerTest, PopulatedBubble) {
   CreateBubble();
-  [controller_ replaceImageAtIndex:2 withImage:nil];
 
-  CheckWindow(/*icon_count=*/3);
+  WebIntentPickerModel model;
+  model.AddItem(string16(),GURL(),WebIntentPickerModel::DISPOSITION_WINDOW);
+  model.AddItem(string16(),GURL(),WebIntentPickerModel::DISPOSITION_WINDOW);
+
+  [controller_ performLayoutWithModel:&model];
+
+  CheckWindow(/*icon_count=*/2);
 }