Fixed double-calling of OnCancelled for intents with window disposition

BUG=113277
TEST=none


Review URL: http://codereview.chromium.org/9371012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@121725 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 27 insertions, 3 deletions

diff --git a/chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm
index 4016671..7cb1bcc 100644
--- a/chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm
@@ -51,6 +51,7 @@ class WebIntentBubbleControllerTest : public CocoaTest {
   void CreatePicker() {
     picker_.reset(new WebIntentPickerCocoa());
     picker_->delegate_ = &delegate_;
+    picker_->model_ = &model_;
     window_ = nil;
     controller_ = nil;
   }
@@ -120,6 +121,7 @@ class WebIntentBubbleControllerTest : public CocoaTest {
   scoped_ptr<WebIntentPickerCocoa> picker_;
   MockIntentPickerDelegate delegate_;
   MessageLoopForUI message_loop_;
+  WebIntentPickerModel model_;  // The model used by the picker
 };
 
 TEST_F(WebIntentBubbleControllerTest, EmptyBubble) {
@@ -159,3 +161,18 @@ TEST_F(WebIntentBubbleControllerTest, CloseWillClose) {
 
   ignore_result(picker_.release());  // Closing |picker_| will self-destruct it.
 }
+
+TEST_F(WebIntentBubbleControllerTest, DontCancelAfterServiceInvokation) {
+  CreateBubble();
+  model_.AddItem(string16(), GURL(), WebIntentPickerModel::DISPOSITION_WINDOW);
+
+  EXPECT_CALL(delegate_, OnServiceChosen(
+      0, WebIntentPickerModel::DISPOSITION_WINDOW));
+  EXPECT_CALL(delegate_, OnCancelled()).Times(0);
+  EXPECT_CALL(delegate_, OnClosing());
+
+  picker_->OnServiceChosen(0);
+  picker_->Close();
+
+  ignore_result(picker_.release());  // Closing |picker_| will self-destruct it.
+}
diff --git a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.h b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.h
index 4fbf584..71bfeaf 100644
--- a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.h
+++ b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.h
@@ -65,6 +65,9 @@ class WebIntentPickerCocoa : public WebIntentPicker,
   // Delegate for inline disposition tab contents.
   scoped_ptr<WebIntentInlineDispositionDelegate> inline_disposition_delegate_;
 
+  // Indicate that we invoked a service, instead of just closing/cancelling.
+  bool service_invoked;
+
   // Post a delayed task to do layout, if it isn't already pending.
   void PerformDelayedLayout();
 
diff --git a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
index b025780..2b02ce0 100644
--- a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
+++ b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
@@ -37,7 +37,8 @@ WebIntentPickerCocoa::WebIntentPickerCocoa()
       model_(NULL),
       browser_(NULL),
       controller_(nil),
-      weak_ptr_factory_(this) {
+      weak_ptr_factory_(this),
+      service_invoked(false) {
 }
 
 
@@ -49,7 +50,8 @@ WebIntentPickerCocoa::WebIntentPickerCocoa(Browser* browser,
      model_(model),
      browser_(browser),
      controller_(nil),
-     ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+     weak_ptr_factory_(this),
+     service_invoked(false) {
   model_->set_observer(this);
 
   DCHECK(browser);
@@ -145,7 +147,8 @@ void WebIntentPickerCocoa::OnInlineDisposition(WebIntentPickerModel* model) {
 
 void WebIntentPickerCocoa::OnCancelled() {
   DCHECK(delegate_);
-  delegate_->OnCancelled();
+  if (!service_invoked)
+    delegate_->OnCancelled();
   delegate_->OnClosing();
   MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
@@ -153,6 +156,7 @@ void WebIntentPickerCocoa::OnCancelled() {
 void WebIntentPickerCocoa::OnServiceChosen(size_t index) {
   DCHECK(delegate_);
   const WebIntentPickerModel::Item& item = model_->GetItemAt(index);
+  service_invoked = true;
   delegate_->OnServiceChosen(index, item.disposition);
 }