Handling default service in the web intents picker controller.

chrome/browser/ui/intents/web_intent_picker_controller_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/bind.h"
 #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/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"
 #include "chrome/browser/ui/intents/web_intent_picker_controller.h"
 #include "chrome/browser/ui/intents/web_intent_picker_model.h"
 #include "chrome/browser/ui/intents/web_intent_picker_model_observer.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
@@ skipping 84 matching lines @@
   }
 
   // WebIntentPicker implementation.
   virtual void Close() OVERRIDE {}
   virtual void SetActionString(const string16& action) OVERRIDE {}
   virtual void OnExtensionInstallSuccess(const std::string& id) OVERRIDE {
     num_extensions_installed_++;
   }
   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();
   }
 
   // WebIntentPickerModelObserver implementation.
   virtual void OnModelChanged(WebIntentPickerModel* model) OVERRIDE {
     num_installed_services_ =
         static_cast<int>(model->GetInstalledServiceCount());
   }
   virtual void OnFaviconChanged(
       WebIntentPickerModel* model, size_t index) OVERRIDE {
@@ skipping 148 matching lines @@
                         extension_id.c_str(),
                         UTF16ToUTF8(action).c_str(), UTF16ToUTF8(type).c_str(),
                         icon_url.c_str());
     fake_url_fetcher_factory_->SetFakeResponse(cws_query_url.spec(), response,
                                                true);
 
     fake_url_fetcher_factory_->SetFakeResponse(icon_url, icon_response_,
                                                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);
   }
 
   void OnServiceChosen(const GURL& url, Disposition disposition) {
     controller_->OnServiceChosen(url, disposition);
   }
 
   void OnCancelled() {
@@ skipping 256 matching lines @@
   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_);
 }
+
+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_);
+}