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

chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm

Index: chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
index dc0c944553aff9f5ae512a7637e3928446d123d0..edc64d7dc26c0aea88e60ae322990e953624328c 100644
--- a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
+++ b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
@@ -6,6 +6,8 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/bind.h"
+#include "base/message_loop.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -52,7 +54,8 @@ WebIntentPickerCocoa::WebIntentPickerCocoa()
     : delegate_(NULL),
       model_(NULL),
       browser_(NULL),
-      controller_(NULL) {
+      controller_(nil),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {

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

this macro is only needed for msvc i think

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

Done.

 }
 
 
@@ -63,7 +66,8 @@ WebIntentPickerCocoa::WebIntentPickerCocoa(Browser* browser,
    : delegate_(delegate),
      model_(model),
      browser_(browser),
-     controller_(NULL) {
+     controller_(nil),
+     ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   model_->set_observer(this);
 
   DCHECK(browser);
@@ -91,28 +95,43 @@ WebIntentPickerCocoa::~WebIntentPickerCocoa() {
 void WebIntentPickerCocoa::Close() {
 }
 
-void WebIntentPickerCocoa::OnModelChanged(WebIntentPickerModel* model) {
-  DCHECK(controller_);
-  scoped_nsobject<NSMutableArray> urlArray(
-      [[NSMutableArray alloc] initWithCapacity:model->GetItemCount()]);
-
-  for (size_t i = 0; i < model->GetItemCount(); ++i) {
-    const WebIntentPickerModel::Item& item = model->GetItemAt(i);
+void WebIntentPickerCocoa::PerformDelayedLayout() {
+    // Check to see if a layout has already been scheduled.

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

indent only 2

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

Done.

+    if (weak_ptr_factory_.HasWeakPtrs())
+      return;
+
+    // Delay performing layout by a second so that all the animations from
+    // InfoBubbleWindow and origin updates from BaseBubbleController finish, so
+    // that we don't all race trying to change the frame's origin.
+    //
+    // Using MessageLoop is superior here to |-performSelector:| because it will
+    // not retain its target; if the child outlives its parent, zombies get left
+    // behind (http://crbug.com/59619). This will cancel the scheduled task if
+    // the controller get destroyed before the message
+    // can be delivered.

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

This looks familiar. Can this code be factored out somehow instead of copying it
over?

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

It _is_ familiar. The PageInfoBubble does _almost_ the same thing. 

Except for a different delay, a different selector that's ultimately invoked,
and the fact that one is part of the bubble class and the other isn't.

They're just different enough that I couldn't find a good way to pull this out
without making it overkill. 

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

[Nico, 2012/02/06 22:39:38]

:-/

+    MessageLoop::current()->PostDelayedTask(FROM_HERE,
+        base::Bind(&WebIntentPickerCocoa::PerformLayout,
+                   weak_ptr_factory_.GetWeakPtr()),
+        100 /* milliseconds */);
+}
 
-    [urlArray addObject:
-        [NSString stringWithUTF8String:item.url.spec().c_str()]];
-    [controller_ replaceImageAtIndex:i withImage:item.favicon.ToNSImage()];
-  }
+void WebIntentPickerCocoa::PerformLayout() {
+  DCHECK(controller_);
+  // If the window is animating closed when this is called, the
+  // animation could be holding the last reference to |controller_|
+  // (and thus |this|).  Pin it until the task is completed.
+  scoped_nsobject<WebIntentBubbleController> keep_alive([controller_ retain]);
+  [controller_ performLayoutWithModel:model_];
+}
 
-  [controller_ setServiceURLs:urlArray];
+void WebIntentPickerCocoa::OnModelChanged(WebIntentPickerModel* model) {
+  PerformDelayedLayout();
 }
 
 void WebIntentPickerCocoa::OnFaviconChanged(WebIntentPickerModel* model,
                                             size_t index) {
-  DCHECK(controller_);
-
-  const WebIntentPickerModel::Item& item = model->GetItemAt(index);
-  [controller_ replaceImageAtIndex:index withImage:item.favicon.ToNSImage()];
+  // We don't handle individual icon changes - just redo the whole model.
+  PerformDelayedLayout();
 }
 
 void WebIntentPickerCocoa::OnInlineDisposition(WebIntentPickerModel* model) {
@@ -133,6 +152,7 @@ void WebIntentPickerCocoa::OnInlineDisposition(WebIntentPickerModel* model) {
 
   [controller_ setInlineDispositionTabContents:
       inline_disposition_tab_contents_.get()];
+  PerformDelayedLayout();
 
   delegate_->OnInlineDispositionWebContentsCreated(web_contents);
 }