diff options
author | Kristian Monsen <kristianm@google.com> | 2010-08-23 15:09:26 +0100 |
---|---|---|
committer | Kristian Monsen <kristianm@google.com> | 2010-08-27 16:11:57 +0100 |
commit | 595f6a2d4cfce1617d12cafdd0a6b750ce23690f (patch) | |
tree | 10e2b7e83571cf174d0ae08c827d7ba49b0ddd95 /WebKit | |
parent | ae9569ae6f9fab34553804e7ca28168a63c327ea (diff) | |
download | external_webkit-595f6a2d4cfce1617d12cafdd0a6b750ce23690f.zip external_webkit-595f6a2d4cfce1617d12cafdd0a6b750ce23690f.tar.gz external_webkit-595f6a2d4cfce1617d12cafdd0a6b750ce23690f.tar.bz2 |
Checking that all load operations are done in order.
Adding a m_loadState variable and checking that everything is happening when they are supposed to. There were some problems were race conditions between the threads created some issues.
Change-Id: I7fca570aa917eaaa741c3745f6b8417a656d18c4
Diffstat (limited to 'WebKit')
-rw-r--r-- | WebKit/android/WebCoreSupport/WebRequest.cpp | 55 | ||||
-rw-r--r-- | WebKit/android/WebCoreSupport/WebRequest.h | 11 |
2 files changed, 63 insertions, 3 deletions
diff --git a/WebKit/android/WebCoreSupport/WebRequest.cpp b/WebKit/android/WebCoreSupport/WebRequest.cpp index 5e760ae..a712fb0 100644 --- a/WebKit/android/WebCoreSupport/WebRequest.cpp +++ b/WebKit/android/WebCoreSupport/WebRequest.cpp @@ -32,6 +32,7 @@ #include "WebResourceRequest.h" #include "jni.h" +#include <cutils/log.h> #include <net/base/data_url.h> #include <net/base/io_buffer.h> #include <net/http/http_response_headers.h> @@ -45,6 +46,16 @@ extern android::AssetManager* globalAssetManager(); // - Finish the file upload. Testcase is mobile buzz // - Add network throttle needed by Android plugins +// TODO: Turn off asserts crashing before release +// http://b/issue?id=2951985 +#undef ASSERT +#define ASSERT(assertion, ...) do \ + if (!(assertion)) { \ + android_printLog(ANDROID_LOG_ERROR, __FILE__, __VA_ARGS__); \ + CRASH(); \ + } \ +while (0) + namespace android { namespace { @@ -55,6 +66,7 @@ WebRequest::WebRequest(WebUrlLoaderClient* loader, WebResourceRequest webResourc : m_urlLoader(loader) , m_inputStream(0) , m_androidUrl(false) + , m_loadState(Created) { m_url = webResourceRequest.url(); GURL gurl(webResourceRequest.url()); @@ -70,8 +82,8 @@ WebRequest::WebRequest(WebUrlLoaderClient* loader, WebResourceRequest webResourc // for data and send to WebCore WebRequest::WebRequest(WebUrlLoaderClient* loader, WebResourceRequest webResourceRequest, int inputStream) : m_urlLoader(loader) - , m_request(0) , m_androidUrl(true) + , m_loadState(Created) { JNIEnv* env = JSC::Bindings::getJNIEnv(); m_inputStream = (int)env->NewGlobalRef((_jobject*)inputStream); @@ -80,6 +92,9 @@ WebRequest::WebRequest(WebUrlLoaderClient* loader, WebResourceRequest webResourc WebRequest::~WebRequest() { + ASSERT(m_loadState == Finished, "dtor called on a WebRequest in a different state than finished (%d)", m_loadState); + + m_loadState = Deleted; JNIEnv* env = JSC::Bindings::getJNIEnv(); if (m_inputStream) env->DeleteGlobalRef((_jobject*)m_inputStream); @@ -87,6 +102,9 @@ WebRequest::~WebRequest() void WebRequest::finish(bool success) { + ASSERT(m_loadState < Finished, "called finish on an already finished WebRequest (%d)", m_loadState); + + m_loadState = Finished; if (success) { LoaderData* loaderData = new LoaderData(m_urlLoader); m_urlLoader->maybeCallOnMainThread(WebUrlLoaderClient::didFinishLoading, loaderData); @@ -101,12 +119,17 @@ void WebRequest::finish(bool success) void WebRequest::AppendBytesToUpload(const char* bytes, int bytesLen) { - // This should always be called after start and before finish. + // AppendBytesToUpload is only valid before calling start + ASSERT(m_loadState == Created, "Start called on a WebRequest not in CREATED state: (%s)", m_url.c_str()); m_request->AppendBytesToUpload(bytes, bytesLen); } void WebRequest::start(bool isPrivateBrowsing) { + ASSERT(m_loadState == Created, "Start called on a WebRequest not in CREATED state: (%s)", m_url.c_str()); + + m_loadState = Started; + if (m_androidUrl) return handleAndroidURL(); @@ -127,11 +150,18 @@ void WebRequest::start(bool isPrivateBrowsing) void WebRequest::cancel() { + ASSERT(m_loadState >= Started, "Cancel called on a not started WebRequest: (%s)", m_url.c_str()); + ASSERT(m_loadState != Cancelled, "Cancel called on an already cancelled WebRequest: (%s)", m_url.c_str()); + // There is a possible race condition between the IO thread finishing the request and // the WebCore thread cancelling it. If the request has already finished, do // nothing to avoid sending duplicate finish messages to WebCore. - if (!m_request) + if (m_loadState > Cancelled) { return; + } + ASSERT(m_request, "Request set to 0 before it is finished"); + + m_loadState = Cancelled; m_request->Cancel(); finish(true); @@ -141,12 +171,14 @@ void WebRequest::handleAndroidURL() { JNIEnv* env = JSC::Bindings::getJNIEnv(); if (m_inputStream == 0) { + m_loadState = Finished; WebResponse webResponse(m_url, "", 0, "", 0); LoaderData* loaderData = new LoaderData(m_urlLoader, webResponse); m_urlLoader->maybeCallOnMainThread(WebUrlLoaderClient::didFail, loaderData); return; } + m_loadState = Response; WebResponse webResponse(m_url, "", 0, "", 200); LoaderData* loaderResponse = new LoaderData(m_urlLoader, webResponse); m_urlLoader->maybeCallOnMainThread(WebUrlLoaderClient::didReceiveResponse, loaderResponse); @@ -167,6 +199,7 @@ void WebRequest::handleAndroidURL() data->reserve(size); env->GetByteArrayRegion(jb, 0, size, (jbyte*)&data->front()); + m_loadState = GotData; // Passes ownership of data LoaderData* loaderData = new LoaderData(m_urlLoader, data, size); m_urlLoader->maybeCallOnMainThread(WebUrlLoaderClient::didReceiveAndroidFileData, loaderData); @@ -187,11 +220,13 @@ void WebRequest::handleDataURL(GURL url) if (net::DataURL::Parse(url, &mimeType, &charset, data.get())) { // PopulateURLResponse from chrome implementation // weburlloader_impl.cc + m_loadState = Response; WebResponse webResponse(url.spec(), mimeType, data->size(), charset, 200); LoaderData* loaderResponse = new LoaderData(m_urlLoader, webResponse); m_urlLoader->maybeCallOnMainThread(WebUrlLoaderClient::didReceiveResponse, loaderResponse); if (!data->empty()) { + m_loadState = GotData; LoaderData* loaderData = new LoaderData(m_urlLoader, data.leakPtr()); m_urlLoader->maybeCallOnMainThread(WebUrlLoaderClient::didReceiveDataUrl, loaderData); } @@ -235,6 +270,8 @@ void WebRequest::handleBrowserURL(GURL url) // deferring redirect. void WebRequest::OnReceivedRedirect(URLRequest* newRequest, const GURL& newUrl, bool* deferRedirect) { + ASSERT(m_loadState < Response, "Redirect after receiving response"); + if (newRequest && newRequest->status().is_success()) { WebResponse webResponse(newRequest); webResponse.setUrl(newUrl.spec()); @@ -270,6 +307,9 @@ void WebRequest::OnAuthRequired(URLRequest* request, net::AuthChallengeInfo* aut // response headers if this is a request for a HTTP resource. void WebRequest::OnResponseStarted(URLRequest* request) { + ASSERT(m_loadState == Started, "Got response after receiving response"); + + m_loadState = Response; if (request && request->status().is_success()) { WebResponse webResponse(request); LoaderData* loaderData = new LoaderData(m_urlLoader, webResponse); @@ -284,6 +324,8 @@ void WebRequest::OnResponseStarted(URLRequest* request) void WebRequest::startReading() { + ASSERT(m_loadState == Response || m_loadState == GotData, "StartReading in state other than RESPONSE and GOTDATA"); + int bytesRead = 0; // chrome only does one read, and schedules the next on the same thread @@ -295,6 +337,7 @@ void WebRequest::startReading() break; } + m_loadState = GotData; // Read ok, forward buffer to webcore m_networkBuffer->AddRef(); LoaderData* loaderData = new LoaderData(m_urlLoader, m_networkBuffer.get(), bytesRead); @@ -314,6 +357,9 @@ void WebRequest::startReading() bool WebRequest::read(int* bytesRead) { + ASSERT(m_loadState == Response || m_loadState == GotData, "read in state other than RESPONSE and GOTDATA"); + ASSERT(m_networkBuffer == 0, "Read called with a nonzero buffer"); + // TODO: when asserts work, check that the buffer is 0 here m_networkBuffer = new net::IOBuffer(kInitialReadBufSize); return m_request->Read(m_networkBuffer, kInitialReadBufSize, bytesRead); @@ -330,7 +376,10 @@ bool WebRequest::read(int* bytesRead) // and bytes read will be -1. void WebRequest::OnReadCompleted(URLRequest* request, int bytesRead) { + ASSERT(m_loadState == Response || m_loadState == GotData, "OnReadCompleted in state other than RESPONSE and GOTDATA"); + if (request->status().is_success()) { + m_loadState = GotData; m_networkBuffer->AddRef(); LoaderData* loaderData = new LoaderData(m_urlLoader, m_networkBuffer.get(), bytesRead); // m_networkBuffer->Release() on main thread diff --git a/WebKit/android/WebCoreSupport/WebRequest.h b/WebKit/android/WebCoreSupport/WebRequest.h index b6b3ae2..26cfcbb 100644 --- a/WebKit/android/WebCoreSupport/WebRequest.h +++ b/WebKit/android/WebCoreSupport/WebRequest.h @@ -35,6 +35,16 @@ class MessageLoop; namespace android { +enum LoadState { + Created, + Started, + Response, + GotData, + Cancelled, + Finished, + Deleted +}; + class WebResourceRequest; // All methods in this class must be called on the io thread @@ -80,6 +90,7 @@ private: int m_inputStream; bool m_androidUrl; std::string m_url; + LoadState m_loadState; }; } // namespace android |