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 46a2be71542419c701431cad84525faf8edfbb21..98ffdc484236dcd23d3fe9de7a05fac6cc52c9db 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc
@@ -11,9 +11,8 @@
 #include "base/stringprintf.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/intents/default_web_intent_service.h"
 #include "chrome/browser/favicon/favicon_service.h"
-#include "chrome/browser/intents/web_intents_registry.h"
-#include "chrome/browser/intents/web_intents_registry_factory.h"

[groby-ooo-7-16, 2012/05/01 15:28:33]

Nice catch :)

 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/intents/web_intent_picker.h"
@@ -118,6 +117,7 @@ class WebIntentPickerMock : public WebIntentPicker,
   }
   virtual void OnExtensionInstallFailure(const std::string& id) OVERRIDE {}
   virtual void OnPendingAsyncCompleted() OVERRIDE {
+    LOG(INFO) << "OnPendingAsyncCompleted()";

[groby-ooo-7-16, 2012/05/01 15:28:33]

Do we really need this? As a LOG, no less? At least DLOG or DVLOG, maybe?

[Greg Billock, 2012/05/01 19:17:54]

removed

On 2012/05/01 15:28:33, groby wrote:

     StopWaiting();
   }
 
@@ -286,6 +286,16 @@ class WebIntentPickerControllerBrowserTest : public InProcessBrowserTest {
                                                true);
   }
 
+  void SetDefaultService(const string16& action,
+                         const std::string& url) {
+    DefaultWebIntentService default_service;
+    default_service.action = action;
+    default_service.type = kType1;
+    default_service.user_date = 1000000;
+    default_service.service_url = url;
+    web_data_service_->AddDefaultWebIntentService(default_service);
+  }
+
   void OnSendReturnMessage(
     webkit_glue::WebIntentReplyType reply_type) {
     controller_->OnSendReturnMessage(reply_type);
@@ -562,3 +572,32 @@ IN_PROC_BROWSER_TEST_F(WebIntentPickerControllerBrowserTest,
   EXPECT_EQ(0, picker_.num_installed_services_);
   EXPECT_TRUE(dispatcher.replied_);
 }
+
+IN_PROC_BROWSER_TEST_F(WebIntentPickerControllerBrowserTest,
+                       DefaultsTest) {
+  AddWebIntentService(kAction1, kServiceURL1);
+  AddWebIntentService(kAction1, kServiceURL2);
+
+  SetDefaultService(kAction1, kServiceURL1.spec());
+
+  webkit_glue::WebIntentData intent;
+  intent.action = ASCIIToUTF16("a");
+  intent.type = ASCIIToUTF16("b");

[groby-ooo-7-16, 2012/05/01 15:28:33]

I'm confused - why would that find any matches? Shouldn't this be kType1?

[Greg Billock, 2012/05/01 19:17:54]

Yeah, this is confused. These action/types aren't actually used (it's the
ShowDialog args), but that's confusing and will undoubtedly be fixed.

+  IntentsDispatcherMock dispatcher(intent);
+  controller_->SetIntentsDispatcher(&dispatcher);
+
+  ui_test_utils::WindowedTabAddedNotificationObserver new_tab_observer((
+      content::Source<content::WebContentsDelegate>(browser())));
+  controller_->ShowDialog(browser(), kAction1, kType1);
+  new_tab_observer.Wait();
+
+  EXPECT_EQ(2, picker_.num_installed_services_);
+  EXPECT_EQ(0, picker_.num_icons_changed_);

[groby-ooo-7-16, 2012/05/01 15:28:33]

How is this significant to defaults? Do they never pull icons?

[Greg Billock, 2012/05/01 19:17:54]

Basically the picker isn't activated, so they don't. It isn't really essential,
though. I can remove it -- the big thing is there's no OnServiceChosen call
here.

+
+  // The tab is shown immediately without needing to call OnServiceChosen.
+  ASSERT_EQ(2, browser()->tab_count());
+  EXPECT_EQ(GURL(kServiceURL1),
+            browser()->GetSelectedWebContents()->GetURL());
+
+  EXPECT_TRUE(dispatcher.dispatched_);
+}