Add a NavigationThrottle to the public content/ interface

chrome/browser/apps/app_url_redirector.cc

Index: chrome/browser/apps/app_url_redirector.cc
diff --git a/chrome/browser/apps/app_url_redirector.cc b/chrome/browser/apps/app_url_redirector.cc
index cf151693b32f03671b7a2912b54c15b084059bd0..8db15dd7de81b698e0fc4cc5784bbdb1e567d141 100644
--- a/chrome/browser/apps/app_url_redirector.cc
+++ b/chrome/browser/apps/app_url_redirector.cc
@@ -9,22 +9,16 @@
 #include "base/logging.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
-#include "components/navigation_interception/intercept_navigation_resource_throttle.h"
+#include "components/navigation_interception/intercept_navigation_throttle.h"
 #include "components/navigation_interception/navigation_params.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/render_view_host.h"
-#include "content/public/browser/resource_request_info.h"
-#include "content/public/browser/resource_throttle.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
-#include "extensions/browser/info_map.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "net/url_request/url_request.h"
 
-using content::BrowserThread;
-using content::ResourceRequestInfo;
 using content::WebContents;
 using extensions::Extension;
 using extensions::UrlHandlers;
@@ -37,7 +31,6 @@ bool LaunchAppWithUrl(
     const std::string& handler_id,
     content::WebContents* source,
     const navigation_interception::NavigationParams& params) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Redirect top-level navigations only. This excludes iframes and webviews
   // in particular.
@@ -54,8 +47,7 @@ bool LaunchAppWithUrl(
     return true;
   }
 
-  // These are guaranteed by CreateThrottleFor below.
-  DCHECK(!params.is_post());
+  // This is guaranteed by CreateThrottleFor below.
   DCHECK(UrlHandlers::CanExtensionHandleUrl(app.get(), params.url()));
 
   Profile* profile =
@@ -73,57 +65,54 @@ bool LaunchAppWithUrl(
 }  // namespace
 
 // static
-content::ResourceThrottle*
-AppUrlRedirector::MaybeCreateThrottleFor(net::URLRequest* request,
-                                         ProfileIOData* profile_io_data) {
-  DVLOG(1) << "Considering URL for redirection: "
-           << request->method() << " " << request->url().spec();
+content::NavigationThrottle* AppUrlRedirector::MaybeCreateThrottleFor(

[nasko, 2015/09/04 00:01:52]

Why not keep the scoped_ptr? It is much easier to reason about ownership if we
explicitly see Pass vs just returns out as the former propagates through
multiple returns and the latter exists only on the initial method.

[clamy, 2015/09/04 12:59:14]

This is because I changed the ContentBrowserClient method to return an array of
NavigationThrottles instead of explicitly adding them on the NavigationHandle. I
thought it may be weird to return a scoped_vector instead of a more traditional
vector<NavigationThrottle*>, and so returns the latter. This meant the
constructors were changed to return raw pointers. Now if you think it's fine to
return a scoped_vector, then it makes sense for these constructors to return
scoped_ptrs.

[nasko, 2015/09/04 23:36:49]

Yes, using ScopedVector is probably going to be nicer.

[clamy, 2015/09/08 16:27:18]

Done.

+    content::NavigationHandle* handle,
+    content::WebContents* web_contents) {
+  DVLOG(1) << "Considering URL for redirection: " << handle->GetURL().spec();
+
+  content::BrowserContext* browser_context = web_contents->GetBrowserContext();
+  if (!browser_context)
+    return nullptr;
 
   // Support only GET for now.
-  if (request->method() != "GET") {
+  if (handle->IsPost()) {
     DVLOG(1) << "Skip redirection: method is not GET";
-    return NULL;
+    return nullptr;
   }
 
-  if (!request->url().SchemeIsHTTPOrHTTPS()) {
+  if (!handle->GetURL().SchemeIsHTTPOrHTTPS()) {
     DVLOG(1) << "Skip redirection: scheme is not HTTP or HTTPS";
-    return NULL;
-  }
-
-  // The user has indicated that a URL should be force downloaded. Turn off
-  // URL redirection in this case.
-  if (ResourceRequestInfo::ForRequest(request)->IsDownload()) {
-    DVLOG(1) << "Skip redirection: request is a forced download";
-    return NULL;
+    return nullptr;
   }
 
   // Never redirect URLs to apps in incognito. Technically, apps are not
   // supported in incognito, but that may change in future.
   // See crbug.com/240879, which tracks incognito support for v2 apps.
-  if (profile_io_data->IsOffTheRecord()) {
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  if (profile->GetProfileType() == Profile::INCOGNITO_PROFILE) {
     DVLOG(1) << "Skip redirection: unsupported in incognito";
-    return NULL;
+    return nullptr;
   }
 
-  const extensions::ExtensionSet& extensions =
-      profile_io_data->GetExtensionInfoMap()->extensions();
-  for (extensions::ExtensionSet::const_iterator iter = extensions.begin();
-       iter != extensions.end();
-       ++iter) {
+  const extensions::ExtensionSet& enabled_extensions =
+      extensions::ExtensionRegistry::Get(web_contents->GetBrowserContext())
+          ->enabled_extensions();
+  for (extensions::ExtensionSet::const_iterator iter =
+           enabled_extensions.begin();
+       iter != enabled_extensions.end(); ++iter) {
     const UrlHandlerInfo* handler =
-        UrlHandlers::FindMatchingUrlHandler(iter->get(), request->url());
+        UrlHandlers::FindMatchingUrlHandler(iter->get(), handle->GetURL());
     if (handler) {
       DVLOG(1) << "Found matching app handler for redirection: "
                << (*iter)->name() << "(" << (*iter)->id() << "):"
                << handler->id;
-      return new navigation_interception::InterceptNavigationResourceThrottle(
-          request,
-          base::Bind(&LaunchAppWithUrl,
-                     scoped_refptr<const Extension>(*iter),
+      return new navigation_interception::InterceptNavigationThrottle(
+          web_contents, handle,
+          base::Bind(&LaunchAppWithUrl, scoped_refptr<const Extension>(*iter),
                      handler->id));
     }
   }
 
   DVLOG(1) << "Skipping redirection: no matching app handler found";
-  return NULL;
+  return nullptr;
 }