Fix Pepper URL Loader callbacks.

(Previously committed r71334, reverted r71400. Added null check pointed out by
jam.)

Add some tests (for aborting calls).

BUG=none
TEST=ppapi_tests

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71463 0039d316-1c4b-4281-b951-d872f2087c98
parent 8d717973
...@@ -484,5 +484,10 @@ std::string TestFileIO::TestAbortCalls() { ...@@ -484,5 +484,10 @@ std::string TestFileIO::TestAbortCalls() {
} }
} }
// TODO(viettrungluu): Also test that Close() aborts callbacks.
// crbug.com/69457
PASS(); PASS();
} }
// TODO(viettrungluu): Test Close(). crbug.com/69457
// Copyright (c) 2010 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 // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "ppapi/tests/test_url_loader.h" #include "ppapi/tests/test_url_loader.h"
#include <stdio.h> #include <stdio.h>
#include <string.h>
#include <string> #include <string>
#include "ppapi/c/dev/ppb_file_io_dev.h" #include "ppapi/c/dev/ppb_file_io_dev.h"
...@@ -50,6 +51,7 @@ void TestURLLoader::RunTest() { ...@@ -50,6 +51,7 @@ void TestURLLoader::RunTest() {
RUN_TEST(SameOriginRestriction); RUN_TEST(SameOriginRestriction);
RUN_TEST(StreamToFile); RUN_TEST(StreamToFile);
RUN_TEST(AuditURLRedirect); RUN_TEST(AuditURLRedirect);
RUN_TEST(AbortCalls);
} }
std::string TestURLLoader::ReadEntireFile(pp::FileIO_Dev* file_io, std::string TestURLLoader::ReadEntireFile(pp::FileIO_Dev* file_io,
...@@ -70,13 +72,13 @@ std::string TestURLLoader::ReadEntireFile(pp::FileIO_Dev* file_io, ...@@ -70,13 +72,13 @@ std::string TestURLLoader::ReadEntireFile(pp::FileIO_Dev* file_io,
data->append(buf, rv); data->append(buf, rv);
} }
return ""; PASS();
} }
std::string TestURLLoader::ReadEntireResponseBody(pp::URLLoader* loader, std::string TestURLLoader::ReadEntireResponseBody(pp::URLLoader* loader,
std::string* body) { std::string* body) {
TestCompletionCallback callback; TestCompletionCallback callback;
char buf[256]; char buf[2]; // Small so that multiple reads are needed.
for (;;) { for (;;) {
int32_t rv = loader->ReadResponseBody(buf, sizeof(buf), callback); int32_t rv = loader->ReadResponseBody(buf, sizeof(buf), callback);
...@@ -89,7 +91,7 @@ std::string TestURLLoader::ReadEntireResponseBody(pp::URLLoader* loader, ...@@ -89,7 +91,7 @@ std::string TestURLLoader::ReadEntireResponseBody(pp::URLLoader* loader,
body->append(buf, rv); body->append(buf, rv);
} }
return ""; PASS();
} }
std::string TestURLLoader::LoadAndCompareBody( std::string TestURLLoader::LoadAndCompareBody(
...@@ -121,7 +123,7 @@ std::string TestURLLoader::LoadAndCompareBody( ...@@ -121,7 +123,7 @@ std::string TestURLLoader::LoadAndCompareBody(
if (body != expected_body) if (body != expected_body)
return "URLLoader::ReadResponseBody returned unexpected content"; return "URLLoader::ReadResponseBody returned unexpected content";
return ""; PASS();
} }
std::string TestURLLoader::TestBasicGET() { std::string TestURLLoader::TestBasicGET() {
...@@ -296,7 +298,66 @@ std::string TestURLLoader::TestAuditURLRedirect() { ...@@ -296,7 +298,66 @@ std::string TestURLLoader::TestAuditURLRedirect() {
if (response_info.GetRedirectURL().AsString() != "www.google.com") if (response_info.GetRedirectURL().AsString() != "www.google.com")
return "Redirect URL should be www.google.com"; return "Redirect URL should be www.google.com";
return ""; PASS();
}
std::string TestURLLoader::TestAbortCalls() {
pp::URLRequestInfo request;
request.SetURL("test_url_loader_data/hello.txt");
TestCompletionCallback callback;
int32_t rv;
// Abort |Open()|.
{
callback.reset_run_count();
rv = pp::URLLoader(*instance_).Open(request, callback);
if (callback.run_count() > 0)
return "URLLoader::Open ran callback synchronously.";
if (rv == PP_ERROR_WOULDBLOCK) {
rv = callback.WaitForResult();
if (rv != PP_ERROR_ABORTED)
return "URLLoader::Open not aborted.";
} else if (rv != PP_OK) {
return ReportError("URLLoader::Open", rv);
}
}
// Abort |ReadResponseBody()|.
{
char buf[2] = { 0 };
{
pp::URLLoader loader(*instance_);
rv = loader.Open(request, callback);
if (rv == PP_ERROR_WOULDBLOCK)
rv = callback.WaitForResult();
if (rv != PP_OK)
return ReportError("URLLoader::Open", rv);
callback.reset_run_count();
rv = loader.ReadResponseBody(buf, sizeof(buf), callback);
} // Destroy |loader|.
if (rv == PP_ERROR_WOULDBLOCK) {
// Save a copy and make sure |buf| doesn't get written to.
char buf_copy[2];
memcpy(&buf_copy, &buf, sizeof(buf));
rv = callback.WaitForResult();
if (rv != PP_ERROR_ABORTED)
return "URLLoader::ReadResponseBody not aborted.";
if (memcmp(&buf_copy, &buf, sizeof(buf)) != 0)
return "URLLoader::ReadResponseBody wrote data after resource "
"destruction.";
} else if (rv != PP_OK) {
return ReportError("URLLoader::ReadResponseBody", rv);
}
}
// TODO(viettrungluu): More abort tests (but add basic tests first).
// Also test that Close() aborts properly. crbug.com/69457
PASS();
} }
// TODO(viettrungluu): Add tests for FollowRedirect,
// Get{Upload,Download}Progress, Close (including abort tests if applicable).
// TODO(darin): Add a test for GrantUniversalAccess. // TODO(darin): Add a test for GrantUniversalAccess.
// Copyright (c) 2010 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 // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -42,6 +42,7 @@ class TestURLLoader : public TestCase { ...@@ -42,6 +42,7 @@ class TestURLLoader : public TestCase {
std::string TestStreamToFile(); std::string TestStreamToFile();
std::string TestSameOriginRestriction(); std::string TestSameOriginRestriction();
std::string TestAuditURLRedirect(); std::string TestAuditURLRedirect();
std::string TestAbortCalls();
const PPB_FileIOTrusted_Dev* file_io_trusted_interface_; const PPB_FileIOTrusted_Dev* file_io_trusted_interface_;
}; };
......
// Copyright (c) 2010 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 // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "third_party/WebKit/WebKit/chromium/public/WebURLResponse.h" #include "third_party/WebKit/WebKit/chromium/public/WebURLResponse.h"
#include "webkit/appcache/web_application_cache_host_impl.h" #include "webkit/appcache/web_application_cache_host_impl.h"
#include "webkit/plugins/ppapi/common.h" #include "webkit/plugins/ppapi/common.h"
#include "webkit/plugins/ppapi/plugin_module.h"
#include "webkit/plugins/ppapi/ppapi_plugin_instance.h" #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
#include "webkit/plugins/ppapi/ppb_url_request_info_impl.h" #include "webkit/plugins/ppapi/ppb_url_request_info_impl.h"
#include "webkit/plugins/ppapi/ppb_url_response_info_impl.h" #include "webkit/plugins/ppapi/ppb_url_response_info_impl.h"
...@@ -232,19 +233,19 @@ PPB_URLLoader_Impl* PPB_URLLoader_Impl::AsPPB_URLLoader_Impl() { ...@@ -232,19 +233,19 @@ PPB_URLLoader_Impl* PPB_URLLoader_Impl::AsPPB_URLLoader_Impl() {
int32_t PPB_URLLoader_Impl::Open(PPB_URLRequestInfo_Impl* request, int32_t PPB_URLLoader_Impl::Open(PPB_URLRequestInfo_Impl* request,
PP_CompletionCallback callback) { PP_CompletionCallback callback) {
int32_t rv = ValidateCallback(callback);
if (rv != PP_OK)
return rv;
if (loader_.get()) if (loader_.get())
return PP_ERROR_INPROGRESS; return PP_ERROR_INPROGRESS;
// We only support non-blocking calls.
if (!callback.func)
return PP_ERROR_BADARGUMENT;
WebFrame* frame = GetFrame(instance_); WebFrame* frame = GetFrame(instance_);
if (!frame) if (!frame)
return PP_ERROR_FAILED; return PP_ERROR_FAILED;
WebURLRequest web_request(request->ToWebURLRequest(frame)); WebURLRequest web_request(request->ToWebURLRequest(frame));
int32_t rv = CanRequest(frame, web_request.url()); rv = CanRequest(frame, web_request.url());
if (rv != PP_OK) if (rv != PP_OK)
return rv; return rv;
...@@ -263,28 +264,25 @@ int32_t PPB_URLLoader_Impl::Open(PPB_URLRequestInfo_Impl* request, ...@@ -263,28 +264,25 @@ int32_t PPB_URLLoader_Impl::Open(PPB_URLRequestInfo_Impl* request,
loader_->loadAsynchronously(web_request, this); loader_->loadAsynchronously(web_request, this);
request_info_ = scoped_refptr<PPB_URLRequestInfo_Impl>(request); request_info_ = scoped_refptr<PPB_URLRequestInfo_Impl>(request);
pending_callback_ = callback;
// Notify completion when we receive a redirect or response headers. // Notify completion when we receive a redirect or response headers.
RegisterCallback(callback);
return PP_ERROR_WOULDBLOCK; return PP_ERROR_WOULDBLOCK;
} }
int32_t PPB_URLLoader_Impl::FollowRedirect(PP_CompletionCallback callback) { int32_t PPB_URLLoader_Impl::FollowRedirect(PP_CompletionCallback callback) {
if (pending_callback_.func) int32_t rv = ValidateCallback(callback);
return PP_ERROR_INPROGRESS; if (rv != PP_OK)
return rv;
// We only support non-blocking calls.
if (!callback.func)
return PP_ERROR_BADARGUMENT;
WebURL redirect_url = GURL(response_info_->redirect_url()); WebURL redirect_url = GURL(response_info_->redirect_url());
int32_t rv = CanRequest(GetFrame(instance_), redirect_url); rv = CanRequest(GetFrame(instance_), redirect_url);
if (rv != PP_OK) if (rv != PP_OK)
return rv; return rv;
pending_callback_ = callback;
loader_->setDefersLoading(false); // Allow the redirect to continue. loader_->setDefersLoading(false); // Allow the redirect to continue.
RegisterCallback(callback);
return PP_ERROR_WOULDBLOCK; return PP_ERROR_WOULDBLOCK;
} }
...@@ -316,16 +314,13 @@ bool PPB_URLLoader_Impl::GetDownloadProgress( ...@@ -316,16 +314,13 @@ bool PPB_URLLoader_Impl::GetDownloadProgress(
int32_t PPB_URLLoader_Impl::ReadResponseBody(char* buffer, int32_t PPB_URLLoader_Impl::ReadResponseBody(char* buffer,
int32_t bytes_to_read, int32_t bytes_to_read,
PP_CompletionCallback callback) { PP_CompletionCallback callback) {
int32_t rv = ValidateCallback(callback);
if (rv != PP_OK)
return rv;
if (!response_info_ || response_info_->body()) if (!response_info_ || response_info_->body())
return PP_ERROR_FAILED; return PP_ERROR_FAILED;
if (bytes_to_read <= 0 || !buffer) if (bytes_to_read <= 0 || !buffer)
return PP_ERROR_BADARGUMENT; return PP_ERROR_BADARGUMENT;
if (pending_callback_.func)
return PP_ERROR_INPROGRESS;
// We only support non-blocking calls.
if (!callback.func)
return PP_ERROR_BADARGUMENT;
user_buffer_ = buffer; user_buffer_ = buffer;
user_buffer_size_ = bytes_to_read; user_buffer_size_ = bytes_to_read;
...@@ -340,23 +335,24 @@ int32_t PPB_URLLoader_Impl::ReadResponseBody(char* buffer, ...@@ -340,23 +335,24 @@ int32_t PPB_URLLoader_Impl::ReadResponseBody(char* buffer,
return done_status_; return done_status_;
} }
pending_callback_ = callback; RegisterCallback(callback);
return PP_ERROR_WOULDBLOCK; return PP_ERROR_WOULDBLOCK;
} }
int32_t PPB_URLLoader_Impl::FinishStreamingToFile( int32_t PPB_URLLoader_Impl::FinishStreamingToFile(
PP_CompletionCallback callback) { PP_CompletionCallback callback) {
int32_t rv = ValidateCallback(callback);
if (rv != PP_OK)
return rv;
if (!response_info_ || !response_info_->body()) if (!response_info_ || !response_info_->body())
return PP_ERROR_FAILED; return PP_ERROR_FAILED;
if (pending_callback_.func)
return PP_ERROR_INPROGRESS;
// We may have already reached EOF. // We may have already reached EOF.
if (done_status_ != PP_ERROR_WOULDBLOCK) if (done_status_ != PP_ERROR_WOULDBLOCK)
return done_status_; return done_status_;
// Wait for didFinishLoading / didFail. // Wait for didFinishLoading / didFail.
pending_callback_ = callback; RegisterCallback(callback);
return PP_ERROR_WOULDBLOCK; return PP_ERROR_WOULDBLOCK;
} }
...@@ -367,6 +363,8 @@ void PPB_URLLoader_Impl::Close() { ...@@ -367,6 +363,8 @@ void PPB_URLLoader_Impl::Close() {
WebFrame* frame = instance_->container()->element().document().frame(); WebFrame* frame = instance_->container()->element().document().frame();
frame->stopLoading(); frame->stopLoading();
} }
// TODO(viettrungluu): Check what happens to the callback (probably the
// wrong thing). May need to post abort here. crbug.com/69457
} }
void PPB_URLLoader_Impl::GrantUniversalAccess() { void PPB_URLLoader_Impl::GrantUniversalAccess() {
...@@ -431,7 +429,7 @@ void PPB_URLLoader_Impl::didReceiveData(WebURLLoader* loader, ...@@ -431,7 +429,7 @@ void PPB_URLLoader_Impl::didReceiveData(WebURLLoader* loader,
if (user_buffer_) { if (user_buffer_) {
RunCallback(FillUserBuffer()); RunCallback(FillUserBuffer());
} else { } else {
DCHECK(!pending_callback_.func); DCHECK(!pending_callback_.get() || pending_callback_->completed());
} }
} }
...@@ -482,13 +480,37 @@ void PPB_URLLoader_Impl::InstanceDestroyed(PluginInstance* instance) { ...@@ -482,13 +480,37 @@ void PPB_URLLoader_Impl::InstanceDestroyed(PluginInstance* instance) {
// goes out of scope. // goes out of scope.
} }
int32_t PPB_URLLoader_Impl::ValidateCallback(PP_CompletionCallback callback) {
// We only support non-blocking calls.
if (!callback.func)
return PP_ERROR_BADARGUMENT;
if (pending_callback_.get() && !pending_callback_->completed())
return PP_ERROR_INPROGRESS;
return PP_OK;
}
void PPB_URLLoader_Impl::RegisterCallback(PP_CompletionCallback callback) {
DCHECK(callback.func);
DCHECK(!pending_callback_.get() || pending_callback_->completed());
PP_Resource resource_id = GetReferenceNoAddRef();
CHECK(resource_id);
pending_callback_ = new TrackedCompletionCallback(
module()->GetCallbackTracker(), resource_id, callback);
}
void PPB_URLLoader_Impl::RunCallback(int32_t result) { void PPB_URLLoader_Impl::RunCallback(int32_t result) {
if (!pending_callback_.func) // This may be null only when this is a main document loader.
if (!pending_callback_.get()) {
CHECK(main_document_loader_);
return; return;
}
PP_CompletionCallback callback = {0}; scoped_refptr<TrackedCompletionCallback> callback;
std::swap(callback, pending_callback_); callback.swap(pending_callback_);
PP_RunCompletionCallback(&callback, result); callback->Run(result); // Will complete abortively if necessary.
} }
size_t PPB_URLLoader_Impl::FillUserBuffer() { size_t PPB_URLLoader_Impl::FillUserBuffer() {
......
// Copyright (c) 2010 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 // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -7,10 +7,12 @@ ...@@ -7,10 +7,12 @@
#include <deque> #include <deque>
#include "base/ref_counted.h"
#include "base/scoped_ptr.h" #include "base/scoped_ptr.h"
#include "ppapi/c/pp_completion_callback.h" #include "ppapi/c/pp_completion_callback.h"
#include "ppapi/c/trusted/ppb_url_loader_trusted.h" #include "ppapi/c/trusted/ppb_url_loader_trusted.h"
#include "third_party/WebKit/WebKit/chromium/public/WebURLLoaderClient.h" #include "third_party/WebKit/WebKit/chromium/public/WebURLLoaderClient.h"
#include "webkit/plugins/ppapi/callbacks.h"
#include "webkit/plugins/ppapi/ppapi_plugin_instance.h" #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
#include "webkit/plugins/ppapi/resource.h" #include "webkit/plugins/ppapi/resource.h"
...@@ -89,7 +91,17 @@ class PPB_URLLoader_Impl : public Resource, ...@@ -89,7 +91,17 @@ class PPB_URLLoader_Impl : public Resource,
PPB_URLResponseInfo_Impl* response_info() const { return response_info_; } PPB_URLResponseInfo_Impl* response_info() const { return response_info_; }
private: private:
// Check that |callback| is valid (only non-blocking operation is supported)
// and that no callback is already pending. Returns |PP_OK| if okay, else
// |PP_ERROR_...| to be returned to the plugin.
int32_t ValidateCallback(PP_CompletionCallback callback);
// Sets up |callback| as the pending callback. This should only be called once
// it is certain that |PP_ERROR_WOULDBLOCK| will be returned.
void RegisterCallback(PP_CompletionCallback callback);
void RunCallback(int32_t result); void RunCallback(int32_t result);
size_t FillUserBuffer(); size_t FillUserBuffer();
// Converts a WebURLResponse to a URLResponseInfo and saves it. // Converts a WebURLResponse to a URLResponseInfo and saves it.
...@@ -110,10 +122,11 @@ class PPB_URLLoader_Impl : public Resource, ...@@ -110,10 +122,11 @@ class PPB_URLLoader_Impl : public Resource,
bool RecordDownloadProgress() const; bool RecordDownloadProgress() const;
bool RecordUploadProgress() const; bool RecordUploadProgress() const;
// This will be NULL if the instance has been deleted but this PPB_URLLoader_Impl was // This will be NULL if the instance has been deleted but this
// somehow leaked. In general, you should not need to check this for NULL. // PPB_URLLoader_Impl was somehow leaked. In general, you should not need to
// However, if you see a NULL pointer crash, that means somebody is holding // check this for NULL. However, if you see a NULL pointer crash, that means
// a reference to this object longer than the PluginInstance's lifetime. // somebody is holding a reference to this object longer than the
// PluginInstance's lifetime.
PluginInstance* instance_; PluginInstance* instance_;
// If true, then the plugin instance is a full-frame plugin and we're just // If true, then the plugin instance is a full-frame plugin and we're just
...@@ -122,7 +135,7 @@ class PPB_URLLoader_Impl : public Resource, ...@@ -122,7 +135,7 @@ class PPB_URLLoader_Impl : public Resource,
scoped_ptr<WebKit::WebURLLoader> loader_; scoped_ptr<WebKit::WebURLLoader> loader_;
scoped_refptr<PPB_URLRequestInfo_Impl> request_info_; scoped_refptr<PPB_URLRequestInfo_Impl> request_info_;
scoped_refptr<PPB_URLResponseInfo_Impl> response_info_; scoped_refptr<PPB_URLResponseInfo_Impl> response_info_;
PP_CompletionCallback pending_callback_; scoped_refptr<TrackedCompletionCallback> pending_callback_;
std::deque<char> buffer_; std::deque<char> buffer_;
int64_t bytes_sent_; int64_t bytes_sent_;
int64_t total_bytes_to_be_sent_; int64_t total_bytes_to_be_sent_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment