Patch to fix problems with PPB_URLLoader_Impl and PPAPITests.URLLoader. The cross-origin test doesn't properly check for an error, and the custom-referrer change broke cross-origin requests as a result. Also, there was confusion with some errors being reported as PP_ERROR_FAILED and others as PP_ERROR_NOACCESS. After conversations with WebKit folks, it seems unlikely that a consistent system of error codes can be added, so instead, have PPB_URLLoader_Impl::didFail report PP_ERROR_NOACCESS for unknown error domains (WebKit) and switch on net::kErrorDomain errors from our lower level WebURLLoader.

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

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

4 files changed, 49 insertions, 15 deletions

diff --git a/ppapi/tests/test_case.html.mock-http-headers b/ppapi/tests/test_case.html.mock-http-headers
new file mode 100644
index 0000000..6b253aa
--- /dev/null
+++ b/ppapi/tests/test_case.html.mock-http-headers
@@ -0,0 +1,3 @@
+HTTP/1.1 200 OK
+Content-Type: text/html
+Access-Control-Allow-Origin: *
diff --git a/ppapi/tests/test_url_loader.cc b/ppapi/tests/test_url_loader.cc
index f559507..8fda286 100644
--- a/ppapi/tests/test_url_loader.cc
+++ b/ppapi/tests/test_url_loader.cc
@@ -11,11 +11,13 @@
 #include "ppapi/c/dev/ppb_file_io_dev.h"
 #include "ppapi/c/dev/ppb_file_io_trusted_dev.h"
 #include "ppapi/c/dev/ppb_testing_dev.h"
+#include "ppapi/c/dev/ppb_url_util_dev.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_url_loader.h"
 #include "ppapi/cpp/dev/file_io_dev.h"
 #include "ppapi/cpp/dev/file_ref_dev.h"
 #include "ppapi/cpp/dev/file_system_dev.h"
+#include "ppapi/cpp/dev/url_util_dev.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/url_loader.h"
@@ -271,9 +273,21 @@ std::string TestURLLoader::TestSameOriginRestriction() {
 }
 
 std::string TestURLLoader::TestCrossOriginRequest() {
+  // Get the document URL and use it to construct a URL that will be
+  // considered cross-origin by the WebKit access control code, and yet be
+  // reachable by the test server.
+  PP_URLComponents_Dev components;
+  pp::Var pp_document_url = pp::URLUtil_Dev::Get()->GetDocumentURL(
+      *instance_, &components);
+  std::string document_url = pp_document_url.AsString();
+  // Replace "127.0.0.1" with "localhost".
+  if (document_url.find("127.0.0.1") == std::string::npos)
+    return "Can't construct a cross-origin URL";
+  std::string cross_origin_url = document_url.replace(
+      components.host.begin, components.host.len, "localhost");
+
   pp::URLRequestInfo request(instance_);
-  // Create a URL that will be considered to be a different origin.
-  request.SetURL("http://127.0.0.1/test_url_loader_data/hello.txt");
+  request.SetURL(cross_origin_url);
   request.SetAllowCrossOriginRequests(true);
 
   TestCompletionCallback callback(instance_->pp_instance());
@@ -284,7 +298,7 @@ std::string TestURLLoader::TestCrossOriginRequest() {
     rv = callback.WaitForResult();
 
   // We expect success since we allowed a cross-origin request.
-  if (rv == PP_ERROR_NOACCESS)
+  if (rv != PP_OK)
     return ReportError("URLLoader::Open()", rv);
 
   PASS();
diff --git a/webkit/plugins/ppapi/ppb_url_loader_impl.cc b/webkit/plugins/ppapi/ppb_url_loader_impl.cc
index d0d4f06..193b461 100644
--- a/webkit/plugins/ppapi/ppb_url_loader_impl.cc
+++ b/webkit/plugins/ppapi/ppb_url_loader_impl.cc
@@ -5,6 +5,7 @@
 #include "webkit/plugins/ppapi/ppb_url_loader_impl.h"
 
 #include "base/logging.h"
+#include "net/base/net_errors.h"
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_url_loader.h"
@@ -16,6 +17,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebKitClient.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPluginContainer.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebURLError.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebURLLoader.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebURLLoaderOptions.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebURLRequest.h"
@@ -247,7 +249,7 @@ int32_t PPB_URLLoader_Impl::Open(PPB_URLRequestInfo_Impl* request,
     return rv;
 
   if (request->RequiresUniversalAccess() && !has_universal_access_)
-    return PP_ERROR_BADARGUMENT;
+    return PP_ERROR_NOACCESS;
 
   if (loader_.get())
     return PP_ERROR_INPROGRESS;
@@ -276,13 +278,11 @@ int32_t PPB_URLLoader_Impl::Open(PPB_URLRequestInfo_Impl* request,
     return PP_ERROR_FAILED;
 
   loader_->loadAsynchronously(web_request, this);
-  // Check for immediate failure; The AssociatedURLLoader will call our
-  // didFail method synchronously for certain kinds of access violations
-  // so we must return an error to the caller.
-  // TODO(bbudge) Modify the underlying AssociatedURLLoader to only call
-  // back asynchronously.
-  if (done_status_ == PP_ERROR_FAILED)
-    return PP_ERROR_NOACCESS;
+  // TODO(bbudge) Remove this code when AssociatedURLLoader is changed to
+  // return errors asynchronously.
+  if (done_status_ == PP_ERROR_FAILED ||
+      done_status_ == PP_ERROR_NOACCESS)
+    return done_status_;
 
   request_info_ = scoped_refptr<PPB_URLRequestInfo_Impl>(request);
 
@@ -475,8 +475,23 @@ void PPB_URLLoader_Impl::didFinishLoading(WebURLLoader* loader,
 
 void PPB_URLLoader_Impl::didFail(WebURLLoader* loader,
                                  const WebURLError& error) {
-  // TODO(darin): Provide more detailed error information.
-  done_status_ = PP_ERROR_FAILED;
+  if (error.domain.equals(WebString::fromUTF8(net::kErrorDomain))) {
+    // TODO(bbudge): Extend pp_errors.h to cover interesting network errors
+    // from the net error domain.
+    switch (error.reason) {
+      case net::ERR_ACCESS_DENIED:
+      case net::ERR_NETWORK_ACCESS_DENIED:
+        done_status_ = PP_ERROR_NOACCESS;
+        break;
+      default:
+        done_status_ = PP_ERROR_FAILED;
+        break;
+    }
+  } else {
+    // It's a WebKit error.
+    done_status_ = PP_ERROR_NOACCESS;
+  }
+
   RunCallback(done_status_);
 }
 
diff --git a/webkit/plugins/ppapi/ppb_url_request_info_impl.cc b/webkit/plugins/ppapi/ppb_url_request_info_impl.cc
index 5d8adfa..185ab2a 100644
--- a/webkit/plugins/ppapi/ppb_url_request_info_impl.cc
+++ b/webkit/plugins/ppapi/ppb_url_request_info_impl.cc
@@ -382,8 +382,10 @@ WebURLRequest PPB_URLRequestInfo_Impl::ToWebURLRequest(WebFrame* frame) const {
   if (has_custom_referrer_url_) {
     if (!custom_referrer_url_.empty())
       frame->setReferrerForRequest(web_request, GURL(custom_referrer_url_));
-  } else {
-    frame->setReferrerForRequest(web_request, WebURL());  // Use default.
+  } else if (!allow_cross_origin_requests_) {
+    // Use default, except for cross-origin requests, since 'referer' is not
+    // whitelisted and will cause the request to fail.
+    frame->setReferrerForRequest(web_request, WebURL());
   }
 
   if (has_custom_content_transfer_encoding_) {