Remove ProtocolFactory/Interceptor uses in GViewRequestInterceptor.

Gets rid of more use of net/ globals, replacing with URLRequestJobFactory uses. Helps make net/ more thread-compatible.

BUG=81979
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86802

Review URL: http://codereview.chromium.org/7019030

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86804 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/gview_request_interceptor.cc

@@ -5,14 +5,12 @@
 #include "chrome/browser/chromeos/gview_request_interceptor.h"
 
 #include "base/file_path.h"
-#include "base/memory/singleton.h"
 #include "base/path_service.h"
 #include "chrome/common/chrome_paths.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request.h"
-#include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_redirect_job.h"
 #include "webkit/glue/plugins/plugin_list.h"
 
@@ -21,37 +19,41 @@ namespace chromeos {
 // The PDF mime type is treated special if the browser has a built-in
 // PDF viewer plug-in installed - we want to intercept only if we're
 // told to.
-static const char* const kPdfMimeType = "application/pdf";
+static const char kPdfMimeType[] = "application/pdf";
 
 // This is the list of mime types currently supported by the Google
 // Document Viewer.
-static const char* const supported_mime_type_list[] = {
+static const char* const kSupportedMimeTypeList[] = {
   kPdfMimeType,
   "application/vnd.ms-powerpoint"
 };
 
-static const char* const kGViewUrlPrefix = "http://docs.google.com/gview?url=";
+static const char kGViewUrlPrefix[] = "http://docs.google.com/gview?url=";
 
 GViewRequestInterceptor::GViewRequestInterceptor() {
-  net::URLRequest::RegisterRequestInterceptor(this);
-  for (size_t i = 0; i < arraysize(supported_mime_type_list); ++i) {
-    supported_mime_types_.insert(supported_mime_type_list[i]);
+  for (size_t i = 0; i < arraysize(kSupportedMimeTypeList); ++i) {
+    supported_mime_types_.insert(kSupportedMimeTypeList[i]);
   }
 }
 
 GViewRequestInterceptor::~GViewRequestInterceptor() {
-  net::URLRequest::UnregisterRequestInterceptor(this);
 }
 
 net::URLRequestJob* GViewRequestInterceptor::MaybeIntercept(
-    net::URLRequest* request) {
+    net::URLRequest* request) const {
   // Don't attempt to intercept here as we want to wait until the mime
   // type is fully determined.
   return NULL;
 }
 
+net::URLRequestJob* GViewRequestInterceptor::MaybeInterceptRedirect(
+    const GURL& location,
+    net::URLRequest* request) const {
+  return NULL;
+}
+
 net::URLRequestJob* GViewRequestInterceptor::MaybeInterceptResponse(
-    net::URLRequest* request) {
+    net::URLRequest* request) const {
   // Do not intercept this request if it is a download.
   if (request->load_flags() & net::LOAD_IS_DOWNLOAD) {
     return NULL;
@@ -81,8 +83,4 @@ net::URLRequestJob* GViewRequestInterceptor::MaybeInterceptResponse(
   return NULL;
 }
 
-GViewRequestInterceptor* GViewRequestInterceptor::GetInstance() {
-  return Singleton<GViewRequestInterceptor>::get();
-}
-
 }  // namespace chromeos

chrome/browser/chromeos/gview_request_interceptor.h

-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -8,9 +8,7 @@
 
 #include <string>
 #include "base/hash_tables.h"
-#include "net/url_request/url_request.h"
-
-template <typename T> struct DefaultSingletonTraits;
+#include "net/url_request/url_request_job_factory.h"
 
 namespace chromeos {
 
@@ -20,26 +18,27 @@ namespace chromeos {
 // document types (such as PDF) and redirect the request to the Google
 // Document Viewer, including the document's original URL as a
 // parameter.
-class GViewRequestInterceptor : public net::URLRequest::Interceptor {
+class GViewRequestInterceptor : public net::URLRequestJobFactory::Interceptor {
  public:
+  GViewRequestInterceptor();
+  virtual ~GViewRequestInterceptor();
+
   // Always returns NULL because we don't want to attempt a redirect
   // before seeing the detected mime type of the request.
-  virtual net::URLRequestJob* MaybeIntercept(net::URLRequest* request);
+  virtual net::URLRequestJob* MaybeIntercept(net::URLRequest* request) const;
+
+  // Always returns NULL.
+  virtual net::URLRequestJob* MaybeInterceptRedirect(
+      const GURL& location,
+      net::URLRequest* request) const;
 
   // Determines if the requested document can be viewed by the Google
   // Document Viewer.  If it can, returns a net::URLRequestJob that
   // redirects the browser to the view URL.
-  virtual net::URLRequestJob* MaybeInterceptResponse(net::URLRequest* request);
-
-  // Singleton accessor.
-  static GViewRequestInterceptor* GetInstance();
+  virtual net::URLRequestJob* MaybeInterceptResponse(
+      net::URLRequest* request) const;
 
  private:
-  friend struct DefaultSingletonTraits<GViewRequestInterceptor>;
-
-  GViewRequestInterceptor();
-  virtual ~GViewRequestInterceptor();
-
   // The list of supported mime types.
   base::hash_set<std::string> supported_mime_types_;
 };

chrome/browser/chromeos/gview_request_interceptor_unittest.cc

@@ -11,6 +11,7 @@
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job.h"
+#include "net/url_request/url_request_job_factory.h"
 #include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -18,6 +19,8 @@
 
 namespace chromeos {
 
+namespace {
+
 class GViewURLRequestTestJob : public net::URLRequestTestJob {
  public:
   explicit GViewURLRequestTestJob(net::URLRequest* request)
@@ -47,23 +50,25 @@ class GViewURLRequestTestJob : public net::URLRequestTestJob {
   ~GViewURLRequestTestJob() {}
 };
 
-class GViewRequestInterceptorTest : public testing::Test {
+class GViewRequestProtocolFactory
+    : public net::URLRequestJobFactory::ProtocolHandler {
  public:
-  virtual void SetUp() {
-    net::URLRequest::RegisterProtocolFactory("http",
-                                        &GViewRequestInterceptorTest::Factory);
-    interceptor_ = GViewRequestInterceptor::GetInstance();
-    ASSERT_TRUE(PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path_));
-  }
+  GViewRequestProtocolFactory() {}
+  virtual ~GViewRequestProtocolFactory() {}
 
-  virtual void TearDown() {
-    net::URLRequest::RegisterProtocolFactory("http", NULL);
-    message_loop_.RunAllPending();
+  virtual net::URLRequestJob* MaybeCreateJob(net::URLRequest* request) const {
+    return new GViewURLRequestTestJob(request);
   }
+};
 
-  static net::URLRequestJob* Factory(net::URLRequest* request,
-                                     const std::string& scheme) {
-    return new GViewURLRequestTestJob(request);
+class GViewRequestInterceptorTest : public testing::Test {
+ public:
+  virtual void SetUp() {
+    job_factory_.SetProtocolHandler("http", new GViewRequestProtocolFactory);
+    job_factory_.AddInterceptor(new GViewRequestInterceptor);
+    request_context_ = new TestURLRequestContext;
+    request_context_->set_job_factory(&job_factory_);
+    ASSERT_TRUE(PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path_));
   }
 
   void RegisterPDFPlugin() {
@@ -102,13 +107,15 @@ class GViewRequestInterceptorTest : public testing::Test {
 
  protected:
   MessageLoopForIO message_loop_;
+  net::URLRequestJobFactory job_factory_;
+  scoped_refptr<TestURLRequestContext> request_context_;
   TestDelegate test_delegate_;
-  net::URLRequest::Interceptor* interceptor_;
   FilePath pdf_path_;
 };
 
 TEST_F(GViewRequestInterceptorTest, DoNotInterceptHtml) {
   net::URLRequest request(GURL("http://foo.com/index.html"), &test_delegate_);
+  request.set_context(request_context_);
   request.Start();
   MessageLoop::current()->Run();
   EXPECT_EQ(0, test_delegate_.received_redirect_count());
@@ -117,6 +124,7 @@ TEST_F(GViewRequestInterceptorTest, DoNotInterceptHtml) {
 
 TEST_F(GViewRequestInterceptorTest, DoNotInterceptDownload) {
   net::URLRequest request(GURL("http://foo.com/file.pdf"), &test_delegate_);
+  request.set_context(request_context_);
   request.set_load_flags(net::LOAD_IS_DOWNLOAD);
   request.Start();
   MessageLoop::current()->Run();
@@ -135,6 +143,7 @@ TEST_F(GViewRequestInterceptorTest, DoNotInterceptPdfWhenEnabled) {
   }
 
   net::URLRequest request(GURL("http://foo.com/file.pdf"), &test_delegate_);
+  request.set_context(request_context_);
   request.Start();
   MessageLoop::current()->Run();
   EXPECT_EQ(0, test_delegate_.received_redirect_count());
@@ -152,6 +161,7 @@ TEST_F(GViewRequestInterceptorTest, InterceptPdfWhenDisabled) {
   }
 
   net::URLRequest request(GURL("http://foo.com/file.pdf"), &test_delegate_);
+  request.set_context(request_context_);
   request.Start();
   MessageLoop::current()->Run();
   EXPECT_EQ(1, test_delegate_.received_redirect_count());
@@ -165,6 +175,7 @@ TEST_F(GViewRequestInterceptorTest, InterceptPdfWithNoPlugin) {
   SetPDFPluginLoadedState(false, &enabled);
 
   net::URLRequest request(GURL("http://foo.com/file.pdf"), &test_delegate_);
+  request.set_context(request_context_);
   request.Start();
   MessageLoop::current()->Run();
   EXPECT_EQ(1, test_delegate_.received_redirect_count());
@@ -174,6 +185,7 @@ TEST_F(GViewRequestInterceptorTest, InterceptPdfWithNoPlugin) {
 
 TEST_F(GViewRequestInterceptorTest, InterceptPowerpoint) {
   net::URLRequest request(GURL("http://foo.com/file.ppt"), &test_delegate_);
+  request.set_context(request_context_);
   request.Start();
   MessageLoop::current()->Run();
   EXPECT_EQ(1, test_delegate_.received_redirect_count());
@@ -181,4 +193,6 @@ TEST_F(GViewRequestInterceptorTest, InterceptPowerpoint) {
                  request.url());
 }
 
+}  // namespace
+
 }  // namespace chromeos

chrome/browser/profiles/profile_io_data.cc

@@ -49,6 +49,10 @@
 #include "webkit/database/database_tracker.h"
 #include "webkit/quota/quota_manager.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/gview_request_interceptor.h"
+#endif  // defined(OS_CHROMEOS)
+
 namespace {
 
 // ----------------------------------------------------------------------------
@@ -457,6 +461,13 @@ void ProfileIOData::LazyInitialize() const {
           profile_params_->file_system_context,
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)));
   DCHECK(set_protocol);
+#if defined(OS_CHROMEOS)
+  // Install the GView request interceptor that will redirect requests
+  // of compatible documents (PDF, etc) to the GView document viewer.
+  const CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
+  if (parsed_command_line.HasSwitch(switches::kEnableGView))
+    job_factory_->AddInterceptor(new chromeos::GViewRequestInterceptor);
+#endif  // defined(OS_CHROMEOS)
 
   // Take ownership over these parameters.
   database_tracker_ = profile_params_->database_tracker;

chrome/browser/ui/browser_init.cc

@@ -555,12 +555,6 @@ bool BrowserInit::LaunchBrowser(const CommandLine& command_line,
   // of what window has focus.
   chromeos::WmMessageListener::GetInstance();
 
-  // Install the GView request interceptor that will redirect requests
-  // of compatible documents (PDF, etc) to the GView document viewer.
-  const CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
-  if (parsed_command_line.HasSwitch(switches::kEnableGView)) {
-    chromeos::GViewRequestInterceptor::GetInstance();
-  }
   if (process_startup) {
     // This observer is a singleton. It is never deleted but the pointer is kept
     // in a static so that it isn't reported as a leak.

net/url_request/url_request_job_factory.cc

@@ -76,6 +76,39 @@ URLRequestJob* URLRequestJobFactory::MaybeCreateJobWithProtocolHandler(
   return it->second->MaybeCreateJob(request);
 }
 
+URLRequestJob* URLRequestJobFactory::MaybeInterceptRedirect(
+    const GURL& location,
+    URLRequest* request) const {
+  DCHECK(CalledOnValidThread());
+  URLRequestJob* job = NULL;
+
+  if (!(request->load_flags() & LOAD_DISABLE_INTERCEPT)) {
+    InterceptorList::const_iterator i;
+    for (i = interceptors_.begin(); i != interceptors_.end(); ++i) {
+      job = (*i)->MaybeInterceptRedirect(location, request);
+      if (job)
+        return job;
+    }
+  }
+  return NULL;
+}
+
+URLRequestJob* URLRequestJobFactory::MaybeInterceptResponse(
+    URLRequest* request) const {
+  DCHECK(CalledOnValidThread());
+  URLRequestJob* job = NULL;
+
+  if (!(request->load_flags() & LOAD_DISABLE_INTERCEPT)) {
+    InterceptorList::const_iterator i;
+    for (i = interceptors_.begin(); i != interceptors_.end(); ++i) {
+      job = (*i)->MaybeInterceptResponse(request);
+      if (job)
+        return job;
+    }
+  }
+  return NULL;
+}
+
 bool URLRequestJobFactory::IsHandledProtocol(const std::string& scheme) const {
   DCHECK(CalledOnValidThread());
   InterceptorList::const_iterator i;

net/url_request/url_request_job_factory.h

@@ -86,6 +86,11 @@ class NET_TEST URLRequestJobFactory
   URLRequestJob* MaybeCreateJobWithProtocolHandler(const std::string& scheme,
                                                    URLRequest* request) const;
 
+  URLRequestJob* MaybeInterceptRedirect(const GURL& location,
+                                        URLRequest* request) const;
+
+  URLRequestJob* MaybeInterceptResponse(URLRequest* request) const;
+
   bool IsHandledProtocol(const std::string& scheme) const;
 
   bool IsHandledURL(const GURL& url) const;

net/url_request/url_request_job_manager.cc

@@ -49,9 +49,7 @@ URLRequestJobManager* URLRequestJobManager::GetInstance() {
 
 URLRequestJob* URLRequestJobManager::CreateJob(
     URLRequest* request) const {
-#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-#endif
 
   // If we are given an invalid URL, then don't even try to inspect the scheme.
   if (!request->url().is_valid())
@@ -132,18 +130,35 @@ URLRequestJob* URLRequestJobManager::CreateJob(
 URLRequestJob* URLRequestJobManager::MaybeInterceptRedirect(
     URLRequest* request,
     const GURL& location) const {
-#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-#endif
-  if ((request->load_flags() & LOAD_DISABLE_INTERCEPT) ||
-      (request->status().status() == URLRequestStatus::CANCELED) ||
-      !request->url().is_valid() ||
-      !SupportsScheme(request->url().scheme()))
+  if (!request->url().is_valid() ||
+      request->load_flags() & LOAD_DISABLE_INTERCEPT ||
+      request->status().status() == URLRequestStatus::CANCELED) {
+    return NULL;
+  }
+
+  const URLRequestJobFactory* job_factory = NULL;
+  if (request->context())
+    job_factory = request->context()->job_factory();
+
+  const std::string& scheme = request->url().scheme();  // already lowercase
+  if (job_factory) {
+    if (!job_factory->IsHandledProtocol(scheme)) {
+      return NULL;
+    }
+  } else if (!SupportsScheme(scheme)) {
     return NULL;
+  }
+
+  URLRequestJob* job = NULL;
+  if (job_factory)
+    job = job_factory->MaybeInterceptRedirect(location, request);
+  if (job)
+    return job;
 
   InterceptorList::const_iterator i;
   for (i = interceptors_.begin(); i != interceptors_.end(); ++i) {
-    URLRequestJob* job = (*i)->MaybeInterceptRedirect(request, location);
+    job = (*i)->MaybeInterceptRedirect(request, location);
     if (job)
       return job;
   }
@@ -152,18 +167,35 @@ URLRequestJob* URLRequestJobManager::MaybeInterceptRedirect(
 
 URLRequestJob* URLRequestJobManager::MaybeInterceptResponse(
     URLRequest* request) const {
-#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-#endif
-  if ((request->load_flags() & LOAD_DISABLE_INTERCEPT) ||
-      (request->status().status() == URLRequestStatus::CANCELED) ||
-      !request->url().is_valid() ||
-      !SupportsScheme(request->url().scheme()))
+  if (!request->url().is_valid() ||
+      request->load_flags() & LOAD_DISABLE_INTERCEPT ||
+      request->status().status() == URLRequestStatus::CANCELED) {
     return NULL;
+  }
+
+  const URLRequestJobFactory* job_factory = NULL;
+  if (request->context())
+    job_factory = request->context()->job_factory();
+
+  const std::string& scheme = request->url().scheme();  // already lowercase
+  if (job_factory) {
+    if (!job_factory->IsHandledProtocol(scheme)) {
+      return NULL;
+    }
+  } else if (!SupportsScheme(scheme)) {
+    return NULL;
+  }
+
+  URLRequestJob* job = NULL;
+  if (job_factory)
+    job = job_factory->MaybeInterceptResponse(request);
+  if (job)
+    return job;
 
   InterceptorList::const_iterator i;
   for (i = interceptors_.begin(); i != interceptors_.end(); ++i) {
-    URLRequestJob* job = (*i)->MaybeInterceptResponse(request);
+    job = (*i)->MaybeInterceptResponse(request);
     if (job)
       return job;
   }
@@ -188,9 +220,7 @@ bool URLRequestJobManager::SupportsScheme(const std::string& scheme) const {
 URLRequest::ProtocolFactory* URLRequestJobManager::RegisterProtocolFactory(
     const std::string& scheme,
     URLRequest::ProtocolFactory* factory) {
-#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-#endif
 
   base::AutoLock locked(lock_);
 
@@ -211,9 +241,7 @@ URLRequest::ProtocolFactory* URLRequestJobManager::RegisterProtocolFactory(
 
 void URLRequestJobManager::RegisterRequestInterceptor(
     URLRequest::Interceptor* interceptor) {
-#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-#endif
 
   base::AutoLock locked(lock_);
 
@@ -224,9 +252,7 @@ void URLRequestJobManager::RegisterRequestInterceptor(
 
 void URLRequestJobManager::UnregisterRequestInterceptor(
     URLRequest::Interceptor* interceptor) {
-#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-#endif
 
   base::AutoLock locked(lock_);
 

net/url_request/url_request_job_manager.h

@@ -76,7 +76,6 @@ class URLRequestJobManager {
   URLRequestJobManager();
   ~URLRequestJobManager();
 
-#ifndef NDEBUG
   // The first guy to call this function sets the allowed thread.  This way we
   // avoid needing to define that thread externally.  Since we expect all
   // callers to be on the same thread, we don't worry about threads racing to
@@ -98,7 +97,6 @@ class URLRequestJobManager {
     // bug http://b/issue?id=1338969 has been filed to fix things and turn the
     // check back on.
     return true;
-#endif
   }
 
   // We use this to assert that CreateJob and the registration functions all