Revert 86802 - 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

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

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/7075005

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

chrome/browser/chromeos/gview_request_interceptor.cc

@@ -5,12 +5,14 @@
 #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"
 
@@ -19,41 +21,37 @@ 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 kPdfMimeType[] = "application/pdf";
+static const char* const kPdfMimeType = "application/pdf";
 
 // This is the list of mime types currently supported by the Google
 // Document Viewer.
-static const char* const kSupportedMimeTypeList[] = {
+static const char* const supported_mime_type_list[] = {
   kPdfMimeType,
   "application/vnd.ms-powerpoint"
 };
 
-static const char kGViewUrlPrefix[] = "http://docs.google.com/gview?url=";
+static const char* const kGViewUrlPrefix = "http://docs.google.com/gview?url=";
 
 GViewRequestInterceptor::GViewRequestInterceptor() {
-  for (size_t i = 0; i < arraysize(kSupportedMimeTypeList); ++i) {
-    supported_mime_types_.insert(kSupportedMimeTypeList[i]);
+  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]);
   }
 }
 
 GViewRequestInterceptor::~GViewRequestInterceptor() {
+  net::URLRequest::UnregisterRequestInterceptor(this);
 }
 
 net::URLRequestJob* GViewRequestInterceptor::MaybeIntercept(
-    net::URLRequest* request) const {
+    net::URLRequest* request) {
   // 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) const {
+    net::URLRequest* request) {
   // Do not intercept this request if it is a download.
   if (request->load_flags() & net::LOAD_IS_DOWNLOAD) {
     return NULL;
@@ -83,4 +81,8 @@ net::URLRequestJob* GViewRequestInterceptor::MaybeInterceptResponse(
   return NULL;
 }
 
+GViewRequestInterceptor* GViewRequestInterceptor::GetInstance() {
+  return Singleton<GViewRequestInterceptor>::get();
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/gview_request_interceptor.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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,7 +8,9 @@
 
 #include <string>
 #include "base/hash_tables.h"
-#include "net/url_request/url_request_job_factory.h"
+#include "net/url_request/url_request.h"
+
+template <typename T> struct DefaultSingletonTraits;
 
 namespace chromeos {
 
@@ -18,27 +20,26 @@ 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::URLRequestJobFactory::Interceptor {
+class GViewRequestInterceptor : public net::URLRequest::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) const;
-
-  // Always returns NULL.
-  virtual net::URLRequestJob* MaybeInterceptRedirect(
-      const GURL& location,
-      net::URLRequest* request) const;
+  virtual net::URLRequestJob* MaybeIntercept(net::URLRequest* request);
 
   // 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) const;
+  virtual net::URLRequestJob* MaybeInterceptResponse(net::URLRequest* request);
+
+  // Singleton accessor.
+  static GViewRequestInterceptor* GetInstance();
 
  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,7 +11,6 @@
 #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"
@@ -19,8 +18,6 @@
 
 namespace chromeos {
 
-namespace {
-
 class GViewURLRequestTestJob : public net::URLRequestTestJob {
  public:
   explicit GViewURLRequestTestJob(net::URLRequest* request)
@@ -50,27 +47,25 @@ class GViewURLRequestTestJob : public net::URLRequestTestJob {
   ~GViewURLRequestTestJob() {}
 };
 
-class GViewRequestProtocolFactory
-    : public net::URLRequestJobFactory::ProtocolHandler {
- public:
-  GViewRequestProtocolFactory() {}
-  virtual ~GViewRequestProtocolFactory() {}
-
-  virtual net::URLRequestJob* MaybeCreateJob(net::URLRequest* request) const {
-    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_);
+    net::URLRequest::RegisterProtocolFactory("http",
+                                        &GViewRequestInterceptorTest::Factory);
+    interceptor_ = GViewRequestInterceptor::GetInstance();
     ASSERT_TRUE(PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path_));
   }
 
+  virtual void TearDown() {
+    net::URLRequest::RegisterProtocolFactory("http", NULL);
+    message_loop_.RunAllPending();
+  }
+
+  static net::URLRequestJob* Factory(net::URLRequest* request,
+                                     const std::string& scheme) {
+    return new GViewURLRequestTestJob(request);
+  }
+
   void RegisterPDFPlugin() {
     webkit::npapi::WebPluginInfo info;
     info.path = pdf_path_;
@@ -107,15 +102,13 @@ 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());
@@ -124,7 +117,6 @@ 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();
@@ -143,7 +135,6 @@ 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());
@@ -161,7 +152,6 @@ 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());
@@ -175,7 +165,6 @@ 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());
@@ -185,7 +174,6 @@ 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());
@@ -193,6 +181,4 @@ TEST_F(GViewRequestInterceptorTest, InterceptPowerpoint) {
                  request.url());
 }
 
-}  // namespace
-
 }  // namespace chromeos

chrome/browser/profiles/profile_io_data.cc

@@ -49,10 +49,6 @@
 #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 {
 
 // ----------------------------------------------------------------------------
@@ -461,13 +457,6 @@ 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,6 +555,12 @@ 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,39 +76,6 @@ 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,11 +86,6 @@ 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,7 +49,9 @@ 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())
@@ -130,35 +132,18 @@ URLRequestJob* URLRequestJobManager::CreateJob(
 URLRequestJob* URLRequestJobManager::MaybeInterceptRedirect(
     URLRequest* request,
     const GURL& location) const {
+#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-  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)) {
+#endif
+  if ((request->load_flags() & LOAD_DISABLE_INTERCEPT) ||
+      (request->status().status() == URLRequestStatus::CANCELED) ||
+      !request->url().is_valid() ||
+      !SupportsScheme(request->url().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) {
-    job = (*i)->MaybeInterceptRedirect(request, location);
+    URLRequestJob* job = (*i)->MaybeInterceptRedirect(request, location);
     if (job)
       return job;
   }
@@ -167,35 +152,18 @@ URLRequestJob* URLRequestJobManager::MaybeInterceptRedirect(
 
 URLRequestJob* URLRequestJobManager::MaybeInterceptResponse(
     URLRequest* request) const {
+#ifndef NDEBUG
   DCHECK(IsAllowedThread());
-  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)) {
+#endif
+  if ((request->load_flags() & LOAD_DISABLE_INTERCEPT) ||
+      (request->status().status() == URLRequestStatus::CANCELED) ||
+      !request->url().is_valid() ||
+      !SupportsScheme(request->url().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) {
-    job = (*i)->MaybeInterceptResponse(request);
+    URLRequestJob* job = (*i)->MaybeInterceptResponse(request);
     if (job)
       return job;
   }