Refactor WebIntentsRegistry internals to use closures as WebDataService results handlers.

chrome/browser/intents/web_intents_registry.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 "chrome/browser/intents/web_intents_registry.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 22 matching lines @@
 
 const char* kQuickOfficeViewerMimeType[] = {
   "application/msword",
   "application/vnd.ms-powerpoint",
   "application/vnd.ms-excel",
   "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
   "application/vnd.openxmlformats-officedocument.presentationml.presentation",
   "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
 };
 
+typedef base::Callback<void(const WDTypedResult* result)> ResultsHandler;
 typedef WebIntentsRegistry::IntentServiceList IntentServiceList;
 
 // Compares two mime types for equality. Supports wild cards in both
 // |type1| and |type2|. Wild cards are of the form '<type>/*' or '*'.
 bool MimeTypesAreEqual(const string16& type1, const string16& type2) {
   // We don't have a MIME matcher that allows patterns on both sides
   // Instead, we do two comparisons, treating each type in turn as a
   // pattern. If either one matches, we consider this a MIME match.
   if (net::MatchesMimeType(UTF16ToUTF8(type1), UTF16ToUTF8(type2)))
     return true;
@@ skipping 42 matching lines @@
       ++iter;
     else
       iter = matching_services->erase(iter);
   }
 }
 
 // Callback for existence checks. Converts a callback for a list of services
 // into a callback that returns true if the list contains a specific service.
 void ExistenceCallback(const webkit_glue::WebIntentServiceData& service,
                        const base::Callback<void(bool)>& callback,
-                       const WebIntentsRegistry::IntentServiceList& list) {
+                       const WDTypedResult* result) {
+
+  WebIntentsRegistry::IntentServiceList list = static_cast<
+      const WDResult<IntentServiceList>*>(result)->GetValue();
+
   for (WebIntentsRegistry::IntentServiceList::const_iterator i = list.begin();
        i != list.end(); ++i) {
     if (*i == service) {
       callback.Run(true);
       return;
     }
   }
 
   callback.Run(false);
 }
@@ skipping 27 matching lines @@
   return !((lhs.service_url != rhs.service_url) ||
       (lhs.action != rhs.action) ||
       (lhs.title != rhs.title) ||
       (lhs.disposition != rhs.disposition));
 }
 
 }  // namespace
 
 using webkit_glue::WebIntentServiceData;
 
-// Internal object representing all data associated with a single query.
-struct WebIntentsRegistry::IntentsQuery : public WebDataServiceConsumer {
-
-  // Handle so we can call back into the WebIntentsRegistry when
-  // processing query results. The registry is guaranteed to be
-  // valid for the life of this object. We do not own this object.
-  WebIntentsRegistry* registry_;
-
-  // Underlying data query.
-  WebDataService::Handle query_handle_;
-
-  // The callback for this particular query.
-  QueryCallback callback_;
-
-  // Callback for a query for defaults.
-  DefaultQueryCallback default_callback_;
+// Internal object containing arguments to used in post processing

[Greg Billock, 2012/07/31 19:10:09]

to be used

[Steve McKay, 2012/07/31 19:14:35]

Done.

+// WDS results.
+struct WebIntentsRegistry::QueryParams {
 
   // The particular action to filter for while searching through extensions.
   // If |action_| is empty, return all extension-provided services.
   string16 action_;
 
   // The MIME type that was requested for this service query.
   // Suppports wild cards.
   string16 type_;
 
   // The url of the invoking page.
   GURL url_;
 
-  // Create a new IntentsQuery for services with the specified action/type.
-  IntentsQuery(WebIntentsRegistry* registry,
-               const QueryCallback& callback,
-               const string16& action, const string16& type)
-      : registry_(registry), callback_(callback), action_(action),
-        type_(type) {}
+  // Create a new QueryParams for all intent services or for existence checks.
+  QueryParams() : type_(ASCIIToUTF16("*")) {}
 
-  // Create a new IntentsQuery for all intent services or for existence checks.
-  IntentsQuery(WebIntentsRegistry* registry,
-               const QueryCallback callback)
-      : registry_(registry), callback_(callback), type_(ASCIIToUTF16("*")) {}
+  QueryParams(const string16& action, const string16& type)
+      : action_(action), type_(type) {}
 
-  // Create a new IntentsQuery for default services.
-  IntentsQuery(WebIntentsRegistry* registry,
-               const DefaultQueryCallback& callback,
-               const string16& action, const string16& type, const GURL& url)
-      : registry_(registry), default_callback_(callback), action_(action),
-        type_(type), url_(url) {}
+  // Create a new QueryParams for default services.
+  QueryParams(const string16& action, const string16& type, const GURL& url)
+      : action_(action), type_(type), url_(url) {}
+};
+
+// Internal object adapter the WDS consumer interface to an easier

[Greg Billock, 2012/07/31 19:10:09]

for the

[Steve McKay, 2012/07/31 19:14:35]

Reworded.

+// to use closure/callback way of doing business.
+class WebIntentsRegistry::QueryAdapter : public WebDataServiceConsumer {
+
+ public:
+  // Underlying data query.
+  WebDataService::Handle query_handle_;
+
+  // Create a new QueryAdapter that delegates results to |handler|.
+  QueryAdapter(WebIntentsRegistry* registry, const ResultsHandler& handler)
+      : registry_(registry), handler_(handler) {
+    registry_->TrackQuery(this);
+  }
 
   void OnWebDataServiceRequestDone(
       WebDataService::Handle h,
       const WDTypedResult* result) OVERRIDE {
 
-    // dispatch the request
-    if (result->GetType() == WEB_INTENTS_RESULT) {
-      registry_->OnWebIntentsResultReceived(this, result);
-    } else if (result->GetType() == WEB_INTENTS_DEFAULTS_RESULT) {
-      registry_->OnWebIntentsDefaultsResultReceived(this, result);
-    } else {
-      NOTREACHED();
-    }
+    handler_.Run(result);
+    registry_->ReleaseQuery(this);
   }
+
+ private:
+  // Handle so we can call back into the WebIntentsRegistry when
+  // processing query results. The registry is guaranteed to be
+  // valid for the life of this object. We do not own this object.
+  WebIntentsRegistry* registry_;
+
+  // The callback for this particular query.
+  ResultsHandler handler_;
 };
 
 WebIntentsRegistry::WebIntentsRegistry() {}
 
 WebIntentsRegistry::~WebIntentsRegistry() {
 
   // Cancel all pending queries, since we can't handle them any more.
   for (QueryVector::iterator it = pending_queries_.begin();
       it != pending_queries_.end(); ++it) {
-    IntentsQuery* query = *it;
+    QueryAdapter* query = *it;
     wds_->CancelRequest(query->query_handle_);
     delete query;
   }
 }
 
 void WebIntentsRegistry::Initialize(
     scoped_refptr<WebDataService> wds,
     ExtensionServiceInterface* extension_service) {
   wds_ = wds;
   extension_service_ = extension_service;
 }
 
 void WebIntentsRegistry::OnWebIntentsResultReceived(
-    IntentsQuery* query,
+    const QueryParams& params,
+    const QueryCallback& callback,
     const WDTypedResult* result) {
-  DCHECK(query);
   DCHECK(result);
   DCHECK(result->GetType() == WEB_INTENTS_RESULT);
 
-  ReleaseQuery(query);
-
   IntentServiceList matching_services = static_cast<
       const WDResult<IntentServiceList>*>(result)->GetValue();
 
   // Loop over all services in all extensions, collect ones
   // matching the query.
   if (extension_service_) {
     const ExtensionSet* extensions = extension_service_->extensions();
     if (extensions) {
       for (ExtensionSet::const_iterator i(extensions->begin());
            i != extensions->end(); ++i) {
-        AddMatchingServicesForExtension(**i, query->action_,
+        AddMatchingServicesForExtension(**i, params.action_,
                                         &matching_services);
       }
     }
   }
 
   // Filter out all services not matching the query type.
-  FilterServicesByType(query->type_, &matching_services);
+  FilterServicesByType(params.type_, &matching_services);
 
   // Collapse intents that are equivalent for all but |type|.
   CollapseIntents(&matching_services);
 
-  query->callback_.Run(matching_services);
-  delete query;
+  callback.Run(matching_services);
 }
 
 void WebIntentsRegistry::OnWebIntentsDefaultsResultReceived(
-    IntentsQuery* query,
+    const QueryParams& params,
+    const DefaultQueryCallback& callback,
     const WDTypedResult* result) {
-  DCHECK(query);
   DCHECK(result);
   DCHECK(result->GetType() == WEB_INTENTS_DEFAULTS_RESULT);
 
-  ReleaseQuery(query);
-
   std::vector<DefaultWebIntentService> services = static_cast<
       const WDResult<std::vector<DefaultWebIntentService> >*>(result)->
           GetValue();
 
   DefaultWebIntentService default_service;
   std::vector<DefaultWebIntentService>::iterator iter(services.begin());
   for (; iter != services.end(); ++iter) {
-    if (!WebIntentsTypesMatch(iter->type, query->type_)) {
+    if (!WebIntentsTypesMatch(iter->type, params.type_)) {
       continue;
     }
-    if (!iter->url_pattern.MatchesURL(query->url_)) {
+    if (!iter->url_pattern.MatchesURL(params.url_)) {
       continue;
     }
     const Extension* extension = ExtensionForURL(iter->service_url);
     if (extension != NULL &&
         !extension_service_->IsExtensionEnabled(extension->id())) {
       continue;
     }
 
     // Found a match. If it is better than default_service, use it.
     // Currently the metric is that if the new value is user-set,
@@ skipping 13 matching lines @@
   // TODO(hshi): Temporary workaround for http://crbug.com/134197.
   // If no user-set default service is found, use built-in QuickOffice Viewer as
   // default for MS office files. Remove this once full defaults is in place.
   if (default_service.user_date <= 0) {
     for (size_t i = 0; i < sizeof(kQuickOfficeViewerMimeType) / sizeof(char*);
          ++i) {
       DefaultWebIntentService qoviewer_service;
       qoviewer_service.action = ASCIIToUTF16(kViewActionURL);
       qoviewer_service.type = ASCIIToUTF16(kQuickOfficeViewerMimeType[i]);
       qoviewer_service.service_url = kQuickOfficeViewerServiceURL;
-      if (WebIntentsTypesMatch(qoviewer_service.type, query->type_)) {
+      if (WebIntentsTypesMatch(qoviewer_service.type, params.type_)) {
         default_service = qoviewer_service;
         break;
       }
     }
   }
 
-  query->default_callback_.Run(default_service);
-  delete query;
+  callback.Run(default_service);
 }
 
 void WebIntentsRegistry::GetIntentServices(
     const string16& action, const string16& type,
     const QueryCallback& callback) {
   DCHECK(wds_.get());
   DCHECK(!callback.is_null());
 
-  IntentsQuery* query = new IntentsQuery(this, callback, action, type);
+  const QueryParams params(action, type);
+  const ResultsHandler handler = base::Bind(
+      &WebIntentsRegistry::OnWebIntentsResultReceived,
+      base::Unretained(this),
+      params,
+      callback);
+
+  QueryAdapter* query = new QueryAdapter(this, handler);
   query->query_handle_ = wds_->GetWebIntentServices(action, query);
-  TrackQuery(query);
 }
 
 void WebIntentsRegistry::GetAllIntentServices(
     const QueryCallback& callback) {
   DCHECK(wds_.get());
   DCHECK(!callback.is_null());
 
-  IntentsQuery* query = new IntentsQuery(this, callback);
+  const QueryParams params;
+  const ResultsHandler handler = base::Bind(
+      &WebIntentsRegistry::OnWebIntentsResultReceived,
+      base::Unretained(this),
+      params,
+      callback);
+
+  QueryAdapter* query = new QueryAdapter(this, handler);
   query->query_handle_ = wds_->GetAllWebIntentServices(query);
-  TrackQuery(query);
 }
 
 void WebIntentsRegistry::IntentServiceExists(
     const WebIntentServiceData& service,
     const base::Callback<void(bool)>& callback) {
   DCHECK(!callback.is_null());
 
-  IntentsQuery* query = new IntentsQuery(
-      this, base::Bind(&ExistenceCallback, service, callback));
+  ResultsHandler handler = base::Bind(
+      &ExistenceCallback,
+      service,
+      callback);
+
+  QueryAdapter* query = new QueryAdapter(this, handler);
   query->query_handle_ = wds_->GetWebIntentServicesForURL(
       UTF8ToUTF16(service.service_url.spec()), query);
-  TrackQuery(query);
 }
 
 void WebIntentsRegistry::GetIntentServicesForExtensionFilter(
         const string16& action,
         const string16& type,
         const std::string& extension_id,
         const QueryCallback& callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  // This isn't a WDS query, so we don't track it,
-  // or claim the query later.
-  scoped_ptr<IntentsQuery> query(
-      new IntentsQuery(this, callback, action, type));
+  const QueryParams params(action, type);
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&WebIntentsRegistry::DoGetIntentServicesForExtensionFilter,
-                 base::Unretained(this),
-                 base::Passed(&query), extension_id));
+      base::Bind(
+          &WebIntentsRegistry::DoGetIntentServicesForExtensionFilter,
+          base::Unretained(this),
+          params,
+          extension_id,
+          callback));
 }
 
 void WebIntentsRegistry::DoGetIntentServicesForExtensionFilter(
-    scoped_ptr<IntentsQuery> query,
-    const std::string& extension_id) {
+    const QueryParams& params,
+    const std::string& extension_id,
+    const QueryCallback& callback) {
   IntentServiceList matching_services;
 
   if (extension_service_) {
     const Extension* extension =
         extension_service_->GetExtensionById(extension_id, false);
     AddMatchingServicesForExtension(*extension,
-                                    query->action_,
+                                    params.action_,
                                     &matching_services);
-    FilterServicesByType(query->type_, &matching_services);
+    FilterServicesByType(params.type_, &matching_services);
   }
 
-  query->callback_.Run(matching_services);
+  callback.Run(matching_services);
 }
 
 void WebIntentsRegistry::RegisterDefaultIntentService(
     const DefaultWebIntentService& default_service) {
   DCHECK(wds_.get());
   wds_->AddDefaultWebIntentService(default_service);
 }
 
 void WebIntentsRegistry::UnregisterDefaultIntentService(
     const DefaultWebIntentService& default_service) {
   DCHECK(wds_.get());
   wds_->RemoveDefaultWebIntentService(default_service);
 }
 
 void WebIntentsRegistry::GetDefaultIntentService(
     const string16& action,
     const string16& type,
     const GURL& invoking_url,
     const DefaultQueryCallback& callback) {
   DCHECK(!callback.is_null());
 
-  IntentsQuery* query =
-      new IntentsQuery(this, callback, action, type, invoking_url);
+  const QueryParams params(action, type);
+
+  ResultsHandler handler = base::Bind(
+      &WebIntentsRegistry::OnWebIntentsDefaultsResultReceived,
+      base::Unretained(this),
+      params,
+      callback);
+
+  QueryAdapter* query = new QueryAdapter(this, handler);
   query->query_handle_ =
       wds_->GetDefaultWebIntentServicesForAction(action, query);
-  TrackQuery(query);
 }
 
 void WebIntentsRegistry::RegisterIntentService(
     const WebIntentServiceData& service) {
   DCHECK(wds_.get());
   wds_->AddWebIntentService(service);
 }
 
 void WebIntentsRegistry::UnregisterIntentService(
     const WebIntentServiceData& service) {
@@ skipping 37 matching lines @@
   if (!extensions)
     return NULL;
 
   // Use the unsafe ExtensionURLInfo constructor: we don't care if the extension
   // is running or not.
   GURL gurl(url);
   ExtensionURLInfo info(gurl);
   return extensions->GetExtensionOrAppByURL(info);
 }
 
-void WebIntentsRegistry::TrackQuery(IntentsQuery* query) {
+void WebIntentsRegistry::TrackQuery(QueryAdapter* query) {
   DCHECK(query);
   pending_queries_.push_back(query);
 }
 
-void WebIntentsRegistry::ReleaseQuery(IntentsQuery* query) {
+void WebIntentsRegistry::ReleaseQuery(QueryAdapter* query) {
   QueryVector::iterator it = std::find(
       pending_queries_.begin(), pending_queries_.end(), query);
-  if (it != pending_queries_.end())
+  if (it != pending_queries_.end()) {
     pending_queries_.erase(it);
-  else
+    delete query;
+  } else {
     NOTREACHED();
+  }
 }