Handling default service in the web intents picker controller.

chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc

Index: chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
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 8aa53cac9bcb3eb69fde4264a387683c34a6b220..ecfaa4bf5529c17b051c2b594f084e00d0e18262 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
@@ -9,6 +9,7 @@
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stringprintf.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/intents/web_intents_registry.h"
@@ -94,13 +95,23 @@ class DummyURLFetcherFactory : public content::URLFetcherFactory {
 class WebIntentPickerMock : public WebIntentPicker,
                             public WebIntentPickerModelObserver {
  public:
-  WebIntentPickerMock()
-      : num_installed_services_(0),
-        num_icons_changed_(0),
-        num_extension_icons_changed_(0),
-        num_extensions_installed_(0),
-        message_loop_started_(false),
-        pending_async_completed_(false) {
+  WebIntentPickerMock() {
+    reset();
+  }
+
+  void reset() {

[groby-ooo-7-16, 2012/04/24 21:36:51]

Why separate this out? Nobody calls reset?

[Greg Billock, 2012/04/25 16:04:32]

I was using this but found a better way and forgot to put it back.

+    num_installed_services_ = 0;
+    num_icons_changed_ = 0;
+    num_extension_icons_changed_ = 0;
+    num_extensions_installed_ = 0;
+    message_loop_started_ = false;
+    pending_async_completed_ = false;
+    num_inline_disposition_ = 0;
+    delegate_ = NULL;
+  }
+
+  void MockClose() {
+    delegate_->OnClosing();
   }
 
   // WebIntentPicker implementation.
@@ -128,7 +139,9 @@ class WebIntentPickerMock : public WebIntentPicker,
     num_extension_icons_changed_++;
   }
   virtual void OnInlineDisposition(
-      WebIntentPickerModel* model, const GURL& url) OVERRIDE {}
+      WebIntentPickerModel* model, const GURL& url) OVERRIDE {
+    num_inline_disposition_++;
+  }
 
   void Wait() {
     if (!pending_async_completed_) {
@@ -150,13 +163,16 @@ class WebIntentPickerMock : public WebIntentPicker,
   int num_extensions_installed_;
   bool message_loop_started_;
   bool pending_async_completed_;
+  int num_inline_disposition_;
+  WebIntentPickerDelegate* delegate_;
 };
 
 class IntentsDispatcherMock : public content::WebIntentsDispatcher {
  public:
   explicit IntentsDispatcherMock(const webkit_glue::WebIntentData& intent)
       : intent_(intent),
-        dispatched_(false) {}
+        dispatched_(false),
+        replied_(false) {}
 
   virtual const webkit_glue::WebIntentData& GetIntent() OVERRIDE {
     return intent_;
@@ -168,6 +184,8 @@ class IntentsDispatcherMock : public content::WebIntentsDispatcher {
 
   virtual void SendReplyMessage(webkit_glue::WebIntentReplyType reply_type,
                                 const string16& data) OVERRIDE {
+    replied_ = true;
+    LOG(INFO) << "Intent Reply: " << UTF16ToASCII(data);
   }
 
   virtual void RegisterReplyNotification(
@@ -176,6 +194,7 @@ class IntentsDispatcherMock : public content::WebIntentsDispatcher {
 
   webkit_glue::WebIntentData intent_;
   bool dispatched_;
+  bool replied_;
 };
 
 class WebIntentPickerControllerBrowserTest : public InProcessBrowserTest {
@@ -220,12 +239,17 @@ class WebIntentPickerControllerBrowserTest : public InProcessBrowserTest {
     controller_ = GetBrowser()->
         GetSelectedTabContentsWrapper()->web_intent_picker_controller();
 
-    controller_->set_picker(&picker_);
+    SetupMockPicker();
     controller_->set_model_observer(&picker_);
+    picker_.delegate_ = controller_;
 
     CreateFakeIcon();
   }
 
+  virtual void SetupMockPicker() {
+    controller_->set_picker(&picker_);
+  }
+
   virtual Browser* GetBrowser() { return browser(); }
 
   void AddWebIntentService(const string16& action, const GURL& service_url) {
@@ -438,3 +462,103 @@ IN_PROC_BROWSER_TEST_F(WebIntentPickerControllerBrowserTest,
   // window disposition, it will create a new tab.
   ASSERT_EQ(2, browser()->tab_count());
 }
+
+// Test that an explicit intent does not trigger loading intents from the
+// registry (skips the picker), and creates the intent service handler
+// immediately.
+IN_PROC_BROWSER_TEST_F(WebIntentPickerControllerBrowserTest,
+                       ExplicitIntentTest) {
+  // Install a target service for the explicit intent.
+  const char extension_id[] = "ooodacpbmglpoagccnepcbfhfhpdgddn";
+  AddCWSExtensionServiceWithResult(extension_id, kAction1, kType2);
+  controller_->ShowDialog(browser(), kAction1, kType2);
+  picker_.Wait();
+
+  webkit_glue::WebIntentData intent;
+  intent.action = kAction1;
+  intent.type = kType2;
+  IntentsDispatcherMock dispatcher(intent);
+  controller_->SetIntentsDispatcher(&dispatcher);

[groby-ooo-7-16, 2012/04/24 21:36:51]

Why do we set up a dispatcher when we install a target service?

[Greg Billock, 2012/04/25 16:04:32]

Needed for smooth operation of the controller. I looked into going deeper and
installing it directly, but this just seemed a lot easier.

+
+  OnExtensionInstallRequested(extension_id);
+  picker_.Wait();
+  ASSERT_EQ(1, picker_.num_extensions_installed_);
+  // The intent launches a new tab.
+  ASSERT_EQ(2, browser()->tab_count());
+
+  // Make the controller think nothing is being shown.
+  picker_.MockClose();
+  SetupMockPicker();
+
+  // Now call the explicit intent.
+  webkit_glue::WebIntentData explicitIntent;
+  explicitIntent.action = kAction1;
+  explicitIntent.type = kType2;
+  explicitIntent.service = GURL(StringPrintf("%s://%s/%s",
+                                             chrome::kExtensionScheme,
+                                             extension_id,
+                                             "share.html"));
+  LOG(INFO) << "Calling " << explicitIntent.service.spec();
+  IntentsDispatcherMock dispatcher2(explicitIntent);
+  controller_->SetIntentsDispatcher(&dispatcher2);
+  controller_->ShowDialog(browser(), kAction1, kType2);
+
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_EQ(0, picker_.num_inline_disposition_);
+  EXPECT_FALSE(dispatcher2.replied_);
+
+  // num_installed_services_ would be 2 if the intent wasn't explicit.
+  EXPECT_EQ(0, picker_.num_installed_services_);
+}
+
+// Test that an explicit intent for non-installed extension won't
+// complete.
+IN_PROC_BROWSER_TEST_F(WebIntentPickerControllerBrowserTest,
+                       ExplicitIntentNoExtensionTest) {
+  AddWebIntentService(kAction1, kServiceURL1);
+  AddWebIntentService(kAction1, kServiceURL2);
+  AddCWSExtensionServiceWithResult(kDummyExtensionId, kAction1, kType1);
+
+  webkit_glue::WebIntentData intent;
+  intent.action = kAction1;
+  intent.type = kType1;
+  intent.service = GURL(StringPrintf("%s://%s/%s",
+                                     chrome::kExtensionScheme,
+                                     kDummyExtensionId,
+                                     UTF16ToASCII(kAction1).c_str()));
+  LOG(INFO) << "Calling " << intent.service.spec();
+  IntentsDispatcherMock dispatcher(intent);
+  controller_->SetIntentsDispatcher(&dispatcher);
+  controller_->ShowDialog(browser(), kAction1, kType1);
+  picker_.Wait();
+
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_EQ(0, picker_.num_inline_disposition_);
+  EXPECT_TRUE(dispatcher.replied_);
+
+  // num_installed_services_ would be 2 if the intent wasn't explicit.
+  EXPECT_EQ(0, picker_.num_installed_services_);
+}
+
+// Test that explicit intents won't load non-extensions.
+IN_PROC_BROWSER_TEST_F(WebIntentPickerControllerBrowserTest,
+                       ExplicitIntentNonExtensionTest) {
+  AddWebIntentService(kAction1, kServiceURL1);
+  AddWebIntentService(kAction1, kServiceURL2);
+  AddCWSExtensionServiceWithResult(kDummyExtensionId, kAction1, kType1);
+
+  webkit_glue::WebIntentData intent;
+  intent.action = kAction1;
+  intent.type = kType1;
+  intent.service = GURL("http://www.google.com/");
+  IntentsDispatcherMock dispatcher(intent);
+  controller_->SetIntentsDispatcher(&dispatcher);
+  controller_->ShowDialog(browser(), kAction1, kType1);
+
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_EQ(0, picker_.num_inline_disposition_);
+
+  // num_installed_services_ would be 2 if the intent wasn't explicit.
+  EXPECT_EQ(0, picker_.num_installed_services_);
+  EXPECT_TRUE(dispatcher.replied_);
+}