Fire WEB_INTENT_SERVICE_CONTENT_CLOSED when the user clicks 'use another service' instead of when the picker closes.

BUG=146945

R=gbillock@chromium.org

+erg for GTK change.

+sky for Views change.

+asvitkine for Cocoa change.

Review URL: https://chromiumcodereview.appspot.com/10915269

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

8 files changed, 39 insertions, 20 deletions

diff --git a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
index cd4d1cc..cf1c540 100644
--- a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
+++ b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
@@ -188,7 +188,7 @@ void WebIntentPickerCocoa::OnInlineDisposition(const string16& title,
 void WebIntentPickerCocoa::OnCancelled() {
   DCHECK(delegate_);
   if (!service_invoked)
-    delegate_->OnPickerClosed();
+    delegate_->OnUserCancelledPickerDialog();
   delegate_->OnClosing();
   MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
diff --git a/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm b/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm
index 12038d9..6a80cd0 100644
--- a/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm
+++ b/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm
@@ -29,7 +29,7 @@ class MockIntentPickerDelegate : public WebIntentPickerDelegate {
   MOCK_METHOD1(OnExtensionInstallRequested, void(const std::string& id));
   MOCK_METHOD1(OnSuggestionsLinkClicked,
       void(WindowOpenDisposition disposition));
-  MOCK_METHOD0(OnPickerClosed, void());
+  MOCK_METHOD0(OnUserCancelledPickerDialog, void());
   MOCK_METHOD0(OnChooseAnotherService, void());
   MOCK_METHOD0(OnClosing, void());
 };
@@ -67,7 +67,7 @@ void WebIntentSheetControllerBrowserTest::CreatePicker() {
 IN_PROC_BROWSER_TEST_F(WebIntentSheetControllerBrowserTest, CloseWillClose) {
   CreateBubble(chrome::GetActiveTabContents(browser()));
 
-  EXPECT_CALL(delegate_, OnPickerClosed()).Times(0);
+  EXPECT_CALL(delegate_, OnUserCancelledPickerDialog()).Times(0);
   EXPECT_CALL(delegate_, OnClosing());
   picker_->Close();
 
@@ -84,7 +84,7 @@ IN_PROC_BROWSER_TEST_F(WebIntentSheetControllerBrowserTest,
 
   EXPECT_CALL(delegate_, OnServiceChosen(
       url, webkit_glue::WebIntentServiceData::DISPOSITION_WINDOW));
-  EXPECT_CALL(delegate_, OnPickerClosed()).Times(0);
+  EXPECT_CALL(delegate_, OnUserCancelledPickerDialog()).Times(0);
   EXPECT_CALL(delegate_, OnClosing());
 
   picker_->OnServiceChosen(0);
@@ -96,7 +96,7 @@ IN_PROC_BROWSER_TEST_F(WebIntentSheetControllerBrowserTest,
 IN_PROC_BROWSER_TEST_F(WebIntentSheetControllerBrowserTest,
     OnCancelledWillSignalClose) {
   CreatePicker();
-  EXPECT_CALL(delegate_, OnPickerClosed());
+  EXPECT_CALL(delegate_, OnUserCancelledPickerDialog());
   EXPECT_CALL(delegate_, OnClosing());
   picker_->OnCancelled();
 
diff --git a/chrome/browser/ui/gtk/web_intent_picker_gtk.cc b/chrome/browser/ui/gtk/web_intent_picker_gtk.cc
index 3bf1602..c639f46 100644
--- a/chrome/browser/ui/gtk/web_intent_picker_gtk.cc
+++ b/chrome/browser/ui/gtk/web_intent_picker_gtk.cc
@@ -459,7 +459,7 @@ void WebIntentPickerGtk::OnDestroy(GtkWidget* button) {
 }
 
 void WebIntentPickerGtk::OnCloseButtonClick(GtkWidget* button) {
-  delegate_->OnPickerClosed();
+  delegate_->OnUserCancelledPickerDialog();
 }
 
 void WebIntentPickerGtk::OnExtensionLinkClick(GtkWidget* link) {
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.cc b/chrome/browser/ui/intents/web_intent_picker_controller.cc
index 4adb9c8..4c1a995 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.cc
@@ -206,6 +206,9 @@ WebIntentPickerController::WebIntentPickerController(
                  content::Source<content::NavigationController>(controller));
   registrar_.Add(this, chrome::NOTIFICATION_TAB_CLOSING,
                  content::Source<content::NavigationController>(controller));
+#if defined(TOOLKIT_VIEWS)
+  cancelled_ = true;
+#endif
 }
 
 WebIntentPickerController::~WebIntentPickerController() {
@@ -239,6 +242,10 @@ void WebIntentPickerController::ShowDialog(bool suppress_defaults) {
 
   DCHECK(intents_dispatcher_);
 
+#if defined(TOOLKIT_VIEWS)
+  cancelled_ = true;
+#endif
+
   // Only show a picker once.
   // TODO(gbillock): There's a hole potentially admitting multiple
   // in-flight dispatches since we don't create the picker
@@ -337,6 +344,10 @@ void WebIntentPickerController::OnServiceChosen(
   ExtensionService* service = tab_contents_->profile()->GetExtensionService();
   DCHECK(service);
 
+#if defined(TOOLKIT_VIEWS)
+  cancelled_ = false;
+#endif
+
   // Set the default here. Activating the intent resets the picker model.
   // TODO(gbillock): we should perhaps couple the model to the dispatcher so
   // we can re-activate the model on use-another-service.
@@ -492,18 +503,13 @@ void WebIntentPickerController::OnSuggestionsLinkClicked(
   chrome::Navigate(&params);
 }
 
-void WebIntentPickerController::OnPickerClosed() {
+void WebIntentPickerController::OnUserCancelledPickerDialog() {
   if (!intents_dispatcher_)
     return;
 
-  if (service_tab_) {
-    intents_dispatcher_->SendReplyMessage(
-        webkit_glue::WEB_INTENT_SERVICE_CONTENTS_CLOSED, string16());
-  } else {
-    intents_dispatcher_->SendReplyMessage(
-        webkit_glue::WEB_INTENT_PICKER_CANCELLED, string16());
-    web_intents::RecordPickerCancel(uma_bucket_);
-  }
+  intents_dispatcher_->SendReplyMessage(
+      webkit_glue::WEB_INTENT_PICKER_CANCELLED, string16());
+  web_intents::RecordPickerCancel(uma_bucket_);
 
   ClosePicker();
 }
@@ -517,6 +523,10 @@ void WebIntentPickerController::OnChooseAnotherService() {
 void WebIntentPickerController::OnClosing() {
   SetDialogState(kPickerHidden);
   picker_ = NULL;
+#if defined(TOOLKIT_VIEWS)
+  if (cancelled_)
+    OnUserCancelledPickerDialog();
+#endif
 }
 
 void WebIntentPickerController::OnExtensionInstallSuccess(
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.h b/chrome/browser/ui/intents/web_intent_picker_controller.h
index f5b2ae8..d5b5d61 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.h
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.h
@@ -107,7 +107,7 @@ class WebIntentPickerController
       WindowOpenDisposition disposition) OVERRIDE;
   virtual void OnSuggestionsLinkClicked(
       WindowOpenDisposition disposition) OVERRIDE;
-  virtual void OnPickerClosed() OVERRIDE;
+  virtual void OnUserCancelledPickerDialog() OVERRIDE;
   virtual void OnChooseAnotherService() OVERRIDE;
   virtual void OnClosing() OVERRIDE;
 
@@ -276,6 +276,15 @@ class WebIntentPickerController
   // case, a picker may be non-NULL before it is shown.
   bool picker_shown_;
 
+#if defined(TOOLKIT_VIEWS)
+  // Set to true if user cancelled the picker dialog. Set to false if the picker
+  // dialog is closing for any other reason.
+  // TODO(rouslan): We need to fix DialogDelegate in Views to notify us when the
+  // user closes the picker dialog. This boolean is a mediocre workaround for
+  // lack of that information.
+  bool cancelled_;
+#endif
+
   // Weak pointer to the source WebContents for the intent if the TabContents
   // with which this controller is associated is hosting a web intents window
   // disposition service.
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc b/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
index 74ae47b..8a8aa4e 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
@@ -320,7 +320,7 @@ class WebIntentPickerControllerBrowserTest : public InProcessBrowserTest {
   }
 
   void OnCancelled() {
-    controller_->OnPickerClosed();
+    controller_->OnUserCancelledPickerDialog();
   }
 
   void OnExtensionInstallRequested(const std::string& extension_id) {
diff --git a/chrome/browser/ui/intents/web_intent_picker_delegate.h b/chrome/browser/ui/intents/web_intent_picker_delegate.h
index 3c0bbb6..6dfd71b 100644
--- a/chrome/browser/ui/intents/web_intent_picker_delegate.h
+++ b/chrome/browser/ui/intents/web_intent_picker_delegate.h
@@ -47,7 +47,7 @@ class WebIntentPickerDelegate {
   virtual void OnSuggestionsLinkClicked(WindowOpenDisposition disposition) = 0;
 
   // Called when the user cancels out of the dialog.
-  virtual void OnPickerClosed() = 0;
+  virtual void OnUserCancelledPickerDialog() = 0;
 
   // Called when the user wants to pick another service from within inline
   // disposition.
diff --git a/chrome/browser/ui/views/web_intent_picker_views.cc b/chrome/browser/ui/views/web_intent_picker_views.cc
index 9d280c8..603039a 100644
--- a/chrome/browser/ui/views/web_intent_picker_views.cc
+++ b/chrome/browser/ui/views/web_intent_picker_views.cc
@@ -756,6 +756,7 @@ class WebIntentPickerViews : public views::ButtonListener,
   virtual ~WebIntentPickerViews();
 
   // views::ButtonListener implementation.
+  // This method is called when the user cancels the picker dialog.
   virtual void ButtonPressed(views::Button* sender,
                              const ui::Event& event) OVERRIDE;
 
@@ -917,12 +918,11 @@ WebIntentPickerViews::~WebIntentPickerViews() {
 
 void WebIntentPickerViews::ButtonPressed(views::Button* sender,
                                          const ui::Event& event) {
-  delegate_->OnPickerClosed();
+  delegate_->OnUserCancelledPickerDialog();
 }
 
 void WebIntentPickerViews::WindowClosing() {
   delegate_->OnClosing();
-  delegate_->OnPickerClosed();
 }
 
 void WebIntentPickerViews::DeleteDelegate() {