Handling default service in the web intents picker controller.

chrome/browser/ui/intents/web_intent_picker_controller.cc

Index: chrome/browser/ui/intents/web_intent_picker_controller.cc
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.cc b/chrome/browser/ui/intents/web_intent_picker_controller.cc
index 868609ff94fe3b8e10ec69bfd2199d79f18d8192..32221f98f811c909bae80ce3014ffd1d9fde8c6e 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.cc
@@ -26,6 +26,7 @@
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_source.h"
@@ -164,32 +165,66 @@ void WebIntentPickerController::ShowDialog(Browser* browser,
                                            const string16& action,
                                            const string16& type) {
   // Only show a picker once.
-  if (picker_shown_)
-    return;
-
+  // ??? Is this good enough? There's a hole if we don't create the picker
+  // in this method, but only after disk retrieval.
   // TODO(binji): Figure out what to do when intents are invoked from incognito
   // mode.
-  if (wrapper_->profile()->IsOffTheRecord())
+  if (picker_shown_ || wrapper_->profile()->IsOffTheRecord()) {
+    if (intents_dispatcher_) {
+      intents_dispatcher_->SendReplyMessage(
+          webkit_glue::WEB_INTENT_REPLY_FAILURE, ASCIIToUTF16(""));
+    }
     return;
+  }
 
+  browser_ = browser;
+  action_ = action;
   picker_model_->Clear();
   picker_model_->set_action(action);
   picker_model_->set_mimetype(type);
 
-  // If picker is non-NULL, it was set by a test.
-  if (picker_ == NULL) {
-    picker_ = WebIntentPicker::Create(browser, wrapper_, this,
-                                      picker_model_.get());
+  // If the intent is explicit, skip showing the picker.
+  if (intents_dispatcher_) {
+    const GURL& service = intents_dispatcher_->GetIntent().service;
+    // TODO(gbillock): When we can parse pages for the intent tag,
+    // take out this requirement that explicit intents dispatch to
+    // extension urls.
+    if (!service.is_valid() || !service.SchemeIs(chrome::kExtensionScheme)) {
+      intents_dispatcher_->SendReplyMessage(
+          webkit_glue::WEB_INTENT_REPLY_FAILURE, ASCIIToUTF16(
+              "Only extension urls are supported for explicit invocation"));
+      return;
+    }
+
+    // We get services from the registry because that must retrieve the
+    // registered extension page for this action/type if it is permitted to be
+    // dispatched.
+    pending_async_count_++;
+    GetWebIntentsRegistry(wrapper_)->GetIntentServices(
+        action, type, base::Bind(
+            &WebIntentPickerController::WebIntentServicesForExplicitIntent,
+            weak_ptr_factory_.GetWeakPtr()));
+    return;
   }
 
-  picker_->SetActionString(GetIntentActionString(UTF16ToUTF8(action)));
+  pending_async_count_ += 2;

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

Why exactly do we keep a pending count? (Nobody implements
OnPendingAsyncCompleted except for a stub, so we can probably kill this
machinery)

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

I thought it was used to guide the operation of the picker dialog. If not, yes,
we can scrap it. Let's do that separately though.

+  pending_registry_calls_count_ += 1;

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

Why add 1 here? We already do that when we call the registry? (Also, why at all?
We will always call the Callback as soon as we complete, and the count will
always be 1)

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

There's now two registry callbacks we need to wait for.
The right solution is a barrier closure, but we don't have one. :-/

On 2012/04/24 21:36:51, groby wrote:

 
-  picker_shown_ = true;
-  pending_async_count_+= 2;
   GetWebIntentsRegistry(wrapper_)->GetIntentServices(
       action, type,
           base::Bind(&WebIntentPickerController::OnWebIntentServicesAvailable,
               weak_ptr_factory_.GetWeakPtr()));
+
+  GURL invoking_url = wrapper_->web_contents()->GetURL();
+  if (invoking_url.is_valid()) {
+    pending_async_count_++;
+    pending_registry_calls_count_++;
+    GetWebIntentsRegistry(wrapper_)->GetDefaultIntentService(
+        action, type, invoking_url,
+        base::Bind(&WebIntentPickerController::OnWebIntentDefaultsAvailable,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
   GetCWSIntentsRegistry(wrapper_)->GetIntentServices(
       action, type,
       base::Bind(&WebIntentPickerController::OnCWSIntentServicesAvailable,
@@ -216,12 +251,8 @@ void WebIntentPickerController::OnServiceChosen(const GURL& url,
       break;
 
     case WebIntentPickerModel::DISPOSITION_WINDOW: {
-      // TODO(gbillock): This really only handles the 'window' disposition in a
-      // quite prototype way. We need to flesh out what happens to the picker
-      // during the lifetime of the service url context, and that may mean we
-      // need to pass more information into the injector to find the picker
-      // again and close it.
       int index = TabStripModel::kNoTab;
+      // TODO(gbillock): use browser_?

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

I believe there has been a CL (or there is one in flight) that does do that
already - the switch to WebContents?

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

got rid of this. I'll add a todo to scrap the ShowDialog arg, as we don't need
it any more.

On 2012/04/24 21:36:51, groby wrote:

       Browser* browser = Browser::GetBrowserForController(
           &wrapper_->web_contents()->GetController(), &index);
       TabContentsWrapper* contents = Browser::TabContentsFactory(
@@ -277,6 +308,7 @@ void WebIntentPickerController::OnExtensionInstallRequested(
 
 void WebIntentPickerController::OnExtensionLinkClicked(const std::string& id) {
   // Navigate from source tab.
+  // TODO(gbillock): use browser_?

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

See above

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

Done.

   Browser* browser =
       BrowserList::FindBrowserWithWebContents(wrapper_->web_contents());
   GURL extension_url(extension_urls::GetWebstoreItemDetailURLPrefix() + id);
@@ -345,6 +377,7 @@ void WebIntentPickerController::OnSendReturnMessage(
   if (service_tab_ &&
       reply_type != webkit_glue::WEB_INTENT_SERVICE_CONTENTS_CLOSED) {
     int index = TabStripModel::kNoTab;
+    // TODO(gbillock): use browser_?

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

See above

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

Done.

     Browser* browser = Browser::GetBrowserForController(
         &service_tab_->GetController(), &index);
     if (browser) {
@@ -386,9 +419,72 @@ void WebIntentPickerController::OnWebIntentServicesAvailable(
     favicon_consumer_.SetClientData(favicon_service, handle, i);
   }
 
+  RegistryCallsCompleted();

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

Why do we flag registry calls completed here, not when we get the default
service?

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

Bad name. This is the method we want to happen when both are complete.

On 2012/04/24 21:36:51, groby wrote:

+  AsyncOperationFinished();
+}
+
+void WebIntentPickerController::WebIntentServicesForExplicitIntent(
+    const std::vector<webkit_glue::WebIntentServiceData>& services) {
+  DCHECK(intents_dispatcher_);
+  DCHECK(intents_dispatcher_->GetIntent().service.is_valid());
+  for (size_t i = 0; i < services.size(); ++i) {
+    if (services[i].service_url != intents_dispatcher_->GetIntent().service)
+      continue;
+
+    if (services[i].disposition ==
+        webkit_glue::WebIntentServiceData::DISPOSITION_INLINE)
+      CreatePicker();

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

This is more of a TBD, but I really think we need to (at some point) separate
the idea of the picker and the inline disposition contents into two separate
objects.

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

Makes sense.

+    OnServiceChosen(services[i].service_url,
+                    ConvertDisposition(services[i].disposition));
+    return;
+  }
+
+  // We did not find an acceptable extension. The intent cannot
+  // be dispatched.
+  intents_dispatcher_->SendReplyMessage(
+      webkit_glue::WEB_INTENT_REPLY_FAILURE,  ASCIIToUTF16(
+          "Explicit extension URL is not available."));
+
+  // Finished.
+  AsyncOperationFinished();
+}
+
+void WebIntentPickerController::OnWebIntentDefaultsAvailable(
+    const DefaultWebIntentService& default_service) {
+  if (!default_service.service_url.empty()) {
+    DCHECK(default_service.suppression == 0);
+    default_service_url_ = default_service.service_url;
+  }
+
+  RegistryCallsCompleted();
   AsyncOperationFinished();
 }
 
+void WebIntentPickerController::RegistryCallsCompleted() {
+  if (--pending_registry_calls_count_ != 0) return;
+
+  if (!default_service_url_.empty()) {
+    // If there's a default service, dispatch to it immediately
+    // without showing the picker.
+    const WebIntentPickerModel::InstalledService* default_service =
+        picker_model_->GetInstalledServiceWithURL(GURL(default_service_url_));
+
+    if (default_service != NULL) {
+      if (default_service->disposition ==
+          WebIntentPickerModel::DISPOSITION_INLINE)
+        CreatePicker();
+      OnServiceChosen(default_service->url, default_service->disposition);
+      return;
+    }
+  }
+
+  CreatePicker();
+  picker_->SetActionString(GetIntentActionString(UTF16ToUTF8(action_)));
+
+  // TODO(gbillock): handle the case of having only one service -- set
+  // the make-default checkbox to "true" upon display.
+}
+
 void WebIntentPickerController::OnFaviconDataAvailable(
     FaviconService::Handle handle, history::FaviconData favicon_data) {
   size_t index = favicon_consumer_.GetClientDataForCurrentRequest();
@@ -549,6 +645,18 @@ void WebIntentPickerController::AsyncOperationFinished() {
   }
 }
 
+void WebIntentPickerController::CreatePicker() {
+  // If picker is non-NULL, it was set by a test.
+  // Use browser_?
+  Browser* browser =
+      BrowserList::FindBrowserWithWebContents(wrapper_->web_contents());
+  if (picker_ == NULL) {
+    picker_ = WebIntentPicker::Create(browser, wrapper_, this,
+                                      picker_model_.get());
+  }
+  picker_shown_ = true;
+}
+
 void WebIntentPickerController::ClosePicker() {
   if (picker_)
     picker_->Close();