Correct implementation of the ability to observe and modify resource redirects (issue #346).

git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@419 5089003a-bbd8-11dd-ad1f-f1f9622dbc98

libcef/browser_request_context.cc

@@ -271,6 +271,9 @@ void BrowserRequestContext::Init(
   }
 
   storage_.set_job_factory(job_factory);
+
+  url_request_interceptor_.reset(
+      BrowserResourceLoaderBridge::CreateRequestInterceptor());
 }
 
 BrowserRequestContext::~BrowserRequestContext() {

libcef/browser_request_context.h

@@ -8,6 +8,7 @@
 
 #include "net/http/http_cache.h"
 #include "net/http/url_security_manager.h"
+#include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_storage.h"
 
@@ -51,6 +52,7 @@ class BrowserRequestContext : public net::URLRequestContext {
   net::URLRequestContextStorage storage_;
   scoped_ptr<webkit_blob::BlobStorageController> blob_storage_controller_;
   scoped_ptr<net::URLSecurityManager> url_security_manager_;
+  scoped_ptr<net::URLRequest::Interceptor> url_request_interceptor_;
   FilePath cookie_store_path_;
   bool accept_all_cookies_;
 };

libcef/browser_resource_loader_bridge.cc

@@ -70,6 +70,8 @@
 #include "net/http/http_response_headers.h"
 #include "net/proxy/proxy_service.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_job_manager.h"
+#include "net/url_request/url_request_redirect_job.h"
 #include "webkit/appcache/appcache_interfaces.h"
 #include "webkit/blob/blob_storage_controller.h"
 #include "webkit/blob/deletable_file_reference.h"
@@ -92,6 +94,8 @@ using webkit_glue::ResourceResponseInfo;
 
 namespace {
 
+static const char kCefUserData[] = "cef_userdata";
+
 struct RequestParams {
   std::string method;
   GURL url;
@@ -133,6 +137,55 @@ private:
   bool allow_download_;
 };
 
+// Used to intercept redirect requests.
+class RequestInterceptor : public net::URLRequest::Interceptor
+{
+public:
+  RequestInterceptor() {
+    REQUIRE_IOT();
+    net::URLRequestJobManager::GetInstance()->RegisterRequestInterceptor(this);
+  }
+  ~RequestInterceptor() {
+    REQUIRE_IOT();
+    net::URLRequestJobManager::GetInstance()->UnregisterRequestInterceptor(this);
+  }
+
+  virtual net::URLRequestJob* MaybeIntercept(net::URLRequest* request)
+      OVERRIDE {
+    return NULL;
+  }
+
+  virtual net::URLRequestJob* MaybeInterceptRedirect(net::URLRequest* request,
+      const GURL& location) OVERRIDE {
+    REQUIRE_IOT();
+
+    ExtraRequestInfo* info =
+        static_cast<ExtraRequestInfo*>(request->GetUserData(kCefUserData));
+    if (!info)
+      return NULL;
+
+    CefRefPtr<CefBrowser> browser = info->browser();
+    CefRefPtr<CefClient> client = browser->GetClient();
+    CefRefPtr<CefRequestHandler> handler;
+    if (client.get())
+      handler = client->GetRequestHandler();
+    if (!handler.get())
+      return NULL;
+
+    CefString newUrlStr = location.spec();
+    handler->OnResourceRedirect(browser, request->url().spec(), newUrlStr);
+    if (newUrlStr != location.spec()) {
+      GURL new_url = GURL(std::string(newUrlStr));
+      if (!new_url.is_empty() && new_url.is_valid())
+        return new net::URLRequestRedirectJob(request, new_url);
+    }
+
+    return NULL;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(RequestInterceptor);
+};
+
 // The RequestProxy does most of its work on the IO thread.  The Start and
 // Cancel methods are proxied over to the IO thread, where an net::URLRequest
 // object is instantiated.
@@ -453,11 +506,10 @@ class RequestProxy : public net::URLRequest::Delegate,
           // redirect to the specified URL
           handled = true;
 
-          GURL new_url = GURL(std::string(redirectUrl));
+          params->url = GURL(std::string(redirectUrl));
           ResourceResponseInfo info;
           bool defer_redirect;
-          OnReceivedRedirect(params->url, new_url, info, &defer_redirect);
+          OnReceivedRedirect(params->url, info, &defer_redirect);
-          params->url = new_url;
         } else if (resourceStream.get()) {
           // load from the provided resource stream
           handled = true;
@@ -529,7 +581,7 @@ class RequestProxy : public net::URLRequest::Delegate,
       request_->set_load_flags(params->load_flags);
       request_->set_upload(params->upload.get());
       request_->set_context(_Context->request_context());
-      request_->SetUserData(NULL,
+      request_->SetUserData(kCefUserData,
           new ExtraRequestInfo(browser_.get(), params->request_type));
       BrowserAppCacheSystem::SetExtraRequestInfo(
           request_.get(), params->appcache_host_id, params->request_type);
@@ -631,30 +683,12 @@ class RequestProxy : public net::URLRequest::Delegate,
   // by the SyncRequestProxy subclass.
 
   virtual void OnReceivedRedirect(
-      const GURL& old_url,
       const GURL& new_url,
       const ResourceResponseInfo& info,
       bool* defer_redirect) {
     *defer_redirect = true;  // See AsyncFollowDeferredRedirect
-
-    GURL final_url = new_url;
-
-    if (browser_.get()) {
-      CefRefPtr<CefClient> client = browser_->GetClient();
-      CefRefPtr<CefRequestHandler> handler;
-      if (client.get())
-        handler = client->GetRequestHandler();
-
-      if(handler.get()) {
-        CefString newUrlStr = new_url.spec();
-        handler->OnResourceRedirect(browser_, old_url.spec(), newUrlStr);
-        if (newUrlStr != new_url.spec())
-          final_url = GURL(std::string(newUrlStr));
-      }
-    }
-
     owner_loop_->PostTask(FROM_HERE, base::Bind(
-        &RequestProxy::NotifyReceivedRedirect, this, final_url, info));
+        &RequestProxy::NotifyReceivedRedirect, this, new_url, info));
   }
 
   virtual void OnReceivedResponse(
@@ -666,7 +700,7 @@ class RequestProxy : public net::URLRequest::Delegate,
     if (request_.get()){
       url = request_->url();
       ExtraRequestInfo* info =
-          static_cast<ExtraRequestInfo*>(request_->GetUserData(NULL));
+          static_cast<ExtraRequestInfo*>(request_->GetUserData(kCefUserData));
       if (info)
         allow_download = info->allow_download();
     } else if (!simulated_url.is_empty() && simulated_url.is_valid()) {
@@ -710,7 +744,7 @@ class RequestProxy : public net::URLRequest::Delegate,
     DCHECK(request->status().is_success());
     ResourceResponseInfo info;
     PopulateResponseInfo(request, &info);
-    OnReceivedRedirect(request->url(), new_url, info, defer_redirect);
+    OnReceivedRedirect(new_url, info, defer_redirect);
   }
 
   virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE {
@@ -922,10 +956,9 @@ class SyncRequestProxy : public RequestProxy {
   // Event hooks that run on the IO thread:
 
   virtual void OnReceivedRedirect(
-      const GURL& old_url,
       const GURL& new_url,
       const ResourceResponseInfo& info,
-      bool* defer_redirect) OVERRIDE {
+      bool* defer_redirect) {
     // TODO(darin): It would be much better if this could live in WebCore, but
     // doing so requires API changes at all levels.  Similar code exists in
     // WebCore/platform/network/cf/ResourceHandleCFNet.cpp :-(
@@ -938,11 +971,11 @@ class SyncRequestProxy : public RequestProxy {
   }
 
   virtual void OnReceivedResponse(const ResourceResponseInfo& info,
-                                  const GURL&) OVERRIDE {
+                                  const GURL&) {
     *static_cast<ResourceResponseInfo*>(result_) = info;
   }
 
-  virtual void OnReceivedData(int bytes_read) OVERRIDE {
+  virtual void OnReceivedData(int bytes_read) {
     if (download_to_file_)
       file_stream_.Write(buf_->data(), bytes_read, net::CompletionCallback());
     else
@@ -952,7 +985,7 @@ class SyncRequestProxy : public RequestProxy {
 
   virtual void OnCompletedRequest(const net::URLRequestStatus& status,
                                   const std::string& security_info,
-                                  const base::Time& complete_time) OVERRIDE {
+                                  const base::Time& complete_time) {
     if (download_to_file_)
       file_stream_.Close();
     
@@ -961,7 +994,7 @@ class SyncRequestProxy : public RequestProxy {
   }
 
  protected:
-  virtual void InitializeParams(RequestParams* params) OVERRIDE {
+  virtual void InitializeParams(RequestParams* params) {
     // For synchronous requests ignore load limits to avoid a deadlock problem
     // in SyncRequestProxy (issue #192).
     params->load_flags |= net::LOAD_IGNORE_LIMITS;
@@ -1193,7 +1226,7 @@ CefRefPtr<CefBrowser> BrowserResourceLoaderBridge::GetBrowserForRequest(
     net::URLRequest* request) {
   REQUIRE_IOT();
   ExtraRequestInfo* extra_info =
-      static_cast<ExtraRequestInfo*>(request->GetUserData(NULL));
+      static_cast<ExtraRequestInfo*>(request->GetUserData(kCefUserData));
   if (extra_info)
     return extra_info->browser();
   return NULL;
@@ -1204,3 +1237,9 @@ scoped_refptr<base::MessageLoopProxy>
 BrowserResourceLoaderBridge::GetCacheThread() {
   return CefThread::GetMessageLoopProxyForThread(CefThread::FILE);
 }
+
+//static
+net::URLRequest::Interceptor*
+BrowserResourceLoaderBridge::CreateRequestInterceptor() {
+  return new RequestInterceptor();
+}

libcef/browser_resource_loader_bridge.h

@@ -8,14 +8,11 @@
 
 #include "include/cef.h"
 #include "base/message_loop_proxy.h"
+#include "net/url_request/url_request.h"
 #include <string>
 
 class GURL;
 
-namespace net {
-class URLRequest;
-};
-
 class BrowserResourceLoaderBridge {
  public:
   // May only be called after Init.
@@ -32,6 +29,10 @@ class BrowserResourceLoaderBridge {
   static CefRefPtr<CefBrowser> GetBrowserForRequest(net::URLRequest* request);
 
   static scoped_refptr<base::MessageLoopProxy> GetCacheThread();
+
+  // Used for intercepting URL redirects. Only one interceptor will be created
+  // and its lifespan is controlled by the BrowserRequestContext.
+  static net::URLRequest::Interceptor* CreateRequestInterceptor();
 };
 
 #endif  // _BROWSER_RESOURCE_LOADER_BRIDGE_H

libcef/scheme_impl.cc

@@ -220,6 +220,23 @@ public:
       location->Swap(&redirect_url_);
       return true;
     }
+
+    if (response_.get()) {
+      // Check for HTTP 302 or HTTP 303 redirect.
+      int status = response_->GetStatus();
+      if (status == 302 || status == 303) {
+        CefResponse::HeaderMap headerMap;
+        response_->GetHeaderMap(headerMap);
+        CefRequest::HeaderMap::iterator iter = headerMap.find("Location");
+        if(iter != headerMap.end()) {
+           GURL new_url = GURL(std::string(iter->second));
+           *http_status_code = status;
+           location->Swap(&new_url);
+           return true;
+        }
+      }
+    }
+
     return false;
   }
 

tests/unittests/navigation_unittest.cc

@@ -11,6 +11,7 @@ namespace {
 static const char* kNav1 = "http://tests/nav1.html";
 static const char* kNav2 = "http://tests/nav2.html";
 static const char* kNav3 = "http://tests/nav3.html";
+static const char* kNav4 = "http://tests/nav4.html";
 
 enum NavAction {
   NA_LOAD = 1,
@@ -340,6 +341,130 @@ TEST(NavigationTest, FrameNameIdent)
 
 namespace {
 
+bool g_got_nav1_request = false;
+bool g_got_nav3_request = false;
+bool g_got_nav4_request = false;
+bool g_got_invalid_request = false;
+
+class RedirectSchemeHandler : public CefSchemeHandler
+{
+public:
+  RedirectSchemeHandler() : offset_(0), status_(0) {}
+
+  virtual bool ProcessRequest(CefRefPtr<CefRequest> request,
+                              CefRefPtr<CefSchemeHandlerCallback> callback)
+                              OVERRIDE
+  {
+    EXPECT_TRUE(CefCurrentlyOn(TID_IO));
+
+    std::string url = request->GetURL();
+    if (url == kNav1) {
+      // Redirect using HTTP 302
+      g_got_nav1_request = true;
+      status_ = 302;
+      location_ = kNav2;
+      content_ = "<html><body>Redirected Nav1</body></html>";
+    } else if (url == kNav3) {
+      // Rdirect using redirectUrl
+      g_got_nav3_request = true;
+      status_ = -1;
+      location_ = kNav4;
+      content_ = "<html><body>Redirected Nav3</body></html>";
+    } else if (url == kNav4) {
+      g_got_nav4_request = true;
+      status_ = 200;
+      content_ = "<html><body>Nav4</body></html>";
+    }
+
+    if (status_ != 0) {
+      callback->HeadersAvailable();
+      return true;
+    } else {
+      g_got_invalid_request = true;
+      return false;
+    }
+  }
+
+  virtual void GetResponseHeaders(CefRefPtr<CefResponse> response,
+                                  int64& response_length,
+                                  CefString& redirectUrl) OVERRIDE
+  {
+    EXPECT_TRUE(CefCurrentlyOn(TID_IO));
+
+    EXPECT_TRUE(status_ != 0);
+
+    response->SetStatus(status_);
+    response->SetMimeType("text/html");
+    response_length = content_.size();
+
+    if (status_ == 302) {
+      // Redirect using HTTP 302
+      EXPECT_TRUE(location_.size() > 0);
+      response->SetStatusText("Found");
+      CefResponse::HeaderMap headers;
+      response->GetHeaderMap(headers);
+      headers.insert(std::make_pair("Location", location_));
+      response->SetHeaderMap(headers);
+    } else if (status_ == -1) {
+      // Rdirect using redirectUrl
+      EXPECT_TRUE(location_.size() > 0);
+      redirectUrl = location_;
+    }
+  }
+
+  virtual void Cancel() OVERRIDE
+  {
+    EXPECT_TRUE(CefCurrentlyOn(TID_IO));
+  }
+
+  virtual bool ReadResponse(void* data_out,
+                            int bytes_to_read,
+                            int& bytes_read,
+                            CefRefPtr<CefSchemeHandlerCallback> callback)
+                            OVERRIDE
+  {
+    EXPECT_TRUE(CefCurrentlyOn(TID_IO));
+
+    size_t size = content_.size();
+    if(offset_ < size) {
+      int transfer_size =
+          std::min(bytes_to_read, static_cast<int>(size - offset_));
+      memcpy(data_out, content_.c_str() + offset_, transfer_size);
+      offset_ += transfer_size;
+
+      bytes_read = transfer_size;
+      return true;
+    }
+
+    return false;
+  }
+
+protected:
+  std::string content_;
+  size_t offset_;
+  int status_;
+  std::string location_;
+
+  IMPLEMENT_REFCOUNTING(RedirectSchemeHandler);
+};
+
+class RedirectSchemeHandlerFactory : public CefSchemeHandlerFactory
+{
+public:
+  RedirectSchemeHandlerFactory() {}
+
+  virtual CefRefPtr<CefSchemeHandler> Create(CefRefPtr<CefBrowser> browser,
+                                             const CefString& scheme_name,
+                                             CefRefPtr<CefRequest> request)
+                                             OVERRIDE
+  {
+    EXPECT_TRUE(CefCurrentlyOn(TID_IO));
+    return new RedirectSchemeHandler();
+  }
+
+  IMPLEMENT_REFCOUNTING(RedirectSchemeHandlerFactory);
+};
+
 class RedirectTestHandler : public TestHandler
 {
 public:
@@ -357,18 +482,17 @@ public:
                               NavType navType,
                               bool isRedirect) OVERRIDE
   {
+    // Should be called for each URL that is actually loaded.
     std::string url = request->GetURL();
 
     if (url == kNav1) {
       got_nav1_before_browse_.yes();
-    } else if (url == kNav2) {
-      // should not happen
-      got_nav2_before_browse_.yes();
     } else if (url == kNav3) {
       got_nav3_before_browse_.yes();
-
+    } else if (url == kNav4) {
-      // End of test.
+      got_nav4_before_browse_.yes();
-      DestroyTest();
+    } else {
+      got_invalid_before_browse_.yes();
     }
 
     return false;
@@ -381,19 +505,13 @@ public:
                                     CefRefPtr<CefResponse> response,
                                     int loadFlags) OVERRIDE
   {
+    // Should only be called for the first URL.
     std::string url = request->GetURL();
 
     if (url == kNav1) {
       got_nav1_before_resource_load_.yes();
-      
+    } else {
-      // Redirect to the 2nd URL.
+      got_invalid_before_resource_load_.yes();
-      redirectUrl = kNav2;
-    } else if(url == kNav2) {
-      // Should not happen.
-      got_nav2_before_resource_load_.yes();
-    } else if(url == kNav3) {
-      // Should not happen.
-      got_nav3_before_resource_load_.yes();
     }
 
     return false;
@@ -403,21 +521,67 @@ public:
                                   const CefString& old_url,
                                   CefString& new_url) OVERRIDE
   {
+    // Should be called for each redirected URL.
+
     if (old_url == kNav1 && new_url == kNav2) {
+      // Called due to the nav1 redirect response.
       got_nav1_redirect_.yes();
 
       // Change the redirect to the 3rd URL.
       new_url = kNav3;
+    } else if (old_url == kNav1 && new_url == kNav3) {
+      // Called due to the redirect change above.
+      got_nav2_redirect_.yes();
+    } else if (old_url == kNav3 && new_url == kNav4) {
+      // Called due to the nav3 redirect response.
+      got_nav3_redirect_.yes();
+    } else {
+      got_invalid_redirect_.yes();
+    }
+  }
+
+  virtual void OnLoadStart(CefRefPtr<CefBrowser> browser,
+                           CefRefPtr<CefFrame> frame) OVERRIDE
+  {
+    // Should only be called for the final loaded URL.
+    std::string url = frame->GetURL();
+
+    if(url == kNav4) {
+      got_nav4_load_start_.yes();
+    } else {
+      got_invalid_load_start_.yes();
+    }
+  }
+
+  virtual void OnLoadEnd(CefRefPtr<CefBrowser> browser,
+                         CefRefPtr<CefFrame> frame,
+                         int httpStatusCode) OVERRIDE
+  {
+    // Should only be called for the final loaded URL.
+    std::string url = frame->GetURL();
+
+    if(url == kNav4) {
+      got_nav4_load_end_.yes();
+      DestroyTest();
+    } else {
+      got_invalid_load_end_.yes();
     }
   }
 
   TrackCallback got_nav1_before_browse_;
-  TrackCallback got_nav2_before_browse_;
   TrackCallback got_nav3_before_browse_;
+  TrackCallback got_nav4_before_browse_;
+  TrackCallback got_invalid_before_browse_;
   TrackCallback got_nav1_before_resource_load_;
-  TrackCallback got_nav2_before_resource_load_;
+  TrackCallback got_invalid_before_resource_load_;
-  TrackCallback got_nav3_before_resource_load_;
+  TrackCallback got_nav4_load_start_;
+  TrackCallback got_invalid_load_start_;
+  TrackCallback got_nav4_load_end_;
+  TrackCallback got_invalid_load_end_;
   TrackCallback got_nav1_redirect_;
+  TrackCallback got_nav2_redirect_;
+  TrackCallback got_nav3_redirect_;
+  TrackCallback got_invalid_redirect_;
 };
 
 } // namespace
@@ -425,15 +589,33 @@ public:
 // Verify frame names and identifiers.
 TEST(NavigationTest, Redirect)
 {
+  CefRegisterSchemeHandlerFactory("http", "tests",
+      new RedirectSchemeHandlerFactory());
+  WaitForIOThread();
+  
   CefRefPtr<RedirectTestHandler> handler =
       new RedirectTestHandler();
   handler->ExecuteTest();
 
+  CefClearSchemeHandlerFactories();
+  WaitForIOThread();
+
   ASSERT_TRUE(handler->got_nav1_before_browse_);
-  ASSERT_FALSE(handler->got_nav2_before_browse_);
   ASSERT_TRUE(handler->got_nav3_before_browse_);
+  ASSERT_TRUE(handler->got_nav4_before_browse_);
+  ASSERT_FALSE(handler->got_invalid_before_browse_);
   ASSERT_TRUE(handler->got_nav1_before_resource_load_);
-  ASSERT_FALSE(handler->got_nav2_before_resource_load_);
+  ASSERT_FALSE(handler->got_invalid_before_resource_load_);
-  ASSERT_FALSE(handler->got_nav3_before_resource_load_);
+  ASSERT_TRUE(handler->got_nav4_load_start_);
+  ASSERT_FALSE(handler->got_invalid_load_start_);
+  ASSERT_TRUE(handler->got_nav4_load_end_);
+  ASSERT_FALSE(handler->got_invalid_load_end_);
   ASSERT_TRUE(handler->got_nav1_redirect_);
+  ASSERT_TRUE(handler->got_nav2_redirect_);
+  ASSERT_TRUE(handler->got_nav3_redirect_);
+  ASSERT_FALSE(handler->got_invalid_redirect_);
+  ASSERT_TRUE(g_got_nav1_request);
+  ASSERT_TRUE(g_got_nav3_request);
+  ASSERT_TRUE(g_got_nav4_request);
+  ASSERT_FALSE(g_got_invalid_request);
 }

tests/unittests/scheme_handler_unittest.cc

@@ -340,23 +340,6 @@ public:
 // Global test results object.
 TestResults g_TestResults;
 
-void NotifyEvent(base::WaitableEvent* event)
-{
-  event->Signal();
-}
-
-// Post a task to the specified thread and wait for the task to execute as
-// indication that all previously pending tasks on that thread have completed.
-void WaitForThread(CefThreadId thread_id)
-{
-  base::WaitableEvent event(true, false);
-  CefPostTask(thread_id, NewCefRunnableFunction(&NotifyEvent, &event));
-  event.Wait();
-}
-
-#define WaitForIOThread() WaitForThread(TID_IO)
-#define WaitForUIThread() WaitForThread(TID_UI)
-
 // If |domain| is empty the scheme will be registered as non-standard.
 void RegisterTestScheme(const std::string& scheme, const std::string& domain)
 {

tests/unittests/test_handler.h

@@ -6,6 +6,7 @@
 #define _TEST_HANDLER_H
 
 #include "include/cef.h"
+#include "include/cef_runnable.h"
 #include "base/synchronization/waitable_event.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -186,4 +187,22 @@ private:
   IMPLEMENT_LOCKING(TestHandler);
 };
 
+
+static void NotifyEvent(base::WaitableEvent* event)
+{
+  event->Signal();
+}
+
+// Post a task to the specified thread and wait for the task to execute as
+// indication that all previously pending tasks on that thread have completed.
+static void WaitForThread(CefThreadId thread_id)
+{
+  base::WaitableEvent event(true, false);
+  CefPostTask(thread_id, NewCefRunnableFunction(&NotifyEvent, &event));
+  event.Wait();
+}
+
+#define WaitForIOThread() WaitForThread(TID_IO)
+#define WaitForUIThread() WaitForThread(TID_UI)
+
 #endif // _TEST_HANDLER_H