[android_webview] Don't intercept resource and asset URLRequests.

android_webview/browser/net/android_stream_reader_url_request_job.cc

Index: android_webview/browser/net/android_stream_reader_url_request_job.cc
diff --git a/android_webview/browser/net/android_stream_reader_url_request_job.cc b/android_webview/browser/net/android_stream_reader_url_request_job.cc
index 3050094aecc419fb85cc366fa6194dc5d9be5a16..635746e59848a9db0a1b1c531e8765c57fcc74b4 100644
--- a/android_webview/browser/net/android_stream_reader_url_request_job.cc
+++ b/android_webview/browser/net/android_stream_reader_url_request_job.cc
@@ -68,6 +68,15 @@ class InputStreamReaderWrapper :
   DISALLOW_COPY_AND_ASSIGN(InputStreamReaderWrapper);
 };
 
+// In unittests the Job isn't created on the IO thread making it slightly
+// harder to post back to the job's thread. We use a helper data structure to
+// be able to use PostTaskAndReplyWithResult with OpenInputStreamOnWorkerThread
+// which will take care of posting back to the Job's thread.
+struct AndroidStreamReaderURLRequestJob::OpenInputStreamResult {

[mnaganov (inactive), 2013/03/06 09:26:47]

Is this about C++ or Java unittests? If C++, perhaps it would be better to make
the test to have an IO thread? See for example WebContentsAudioInputStreamTest.

[mkosiba (inactive), 2013/03/06 19:05:27]

C++ unittests. So it looks all of the remaining job implementations are
thread-agnostic, so I'd prefer to keep it like that. I'll pass in a
MessageLoopProxy instead of using the struct.

+  scoped_ptr<InputStream> input_stream;
+  scoped_ptr<AndroidStreamReaderURLRequestJob::Delegate> delegate;
+};
+
 AndroidStreamReaderURLRequestJob::AndroidStreamReaderURLRequestJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
@@ -81,19 +90,47 @@ AndroidStreamReaderURLRequestJob::AndroidStreamReaderURLRequestJob(
 AndroidStreamReaderURLRequestJob::~AndroidStreamReaderURLRequestJob() {
 }
 
+// static
+scoped_ptr<AndroidStreamReaderURLRequestJob::OpenInputStreamResult>
+AndroidStreamReaderURLRequestJob::OpenInputStreamOnWorkerThread(
+    scoped_ptr<AndroidStreamReaderURLRequestJob::Delegate> delegate,
+    const GURL& url) {
+  JNIEnv* env = AttachCurrentThread();
+  DCHECK(env);
+
+  scoped_ptr<AndroidStreamReaderURLRequestJob::OpenInputStreamResult> result(
+      new AndroidStreamReaderURLRequestJob::OpenInputStreamResult());
+  result->input_stream = delegate->OpenInputStream(env, url);
+  result->delegate = delegate.Pass();
+  return result.Pass();
+}
+
 void AndroidStreamReaderURLRequestJob::Start() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Start reading asynchronously so that all error reporting and data
   // callbacks happen as they would for network requests.
   SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING,
                                   net::ERR_IO_PENDING));
-  MessageLoop::current()->PostTask(
+
+  // This could be done in the InputStreamReader but would force more
+  // complex synchronization in the delegate.
+  PostTaskAndReplyWithResult(
+      GetWorkerThreadRunner(),
       FROM_HERE,
       base::Bind(
-          &AndroidStreamReaderURLRequestJob::StartAsync,
-          weak_factory_.GetWeakPtr()));
+          &OpenInputStreamOnWorkerThread,
+          // This is intentional - the job could be deleted while the callback
+          // is executing on the background thread.
+          // The delegate will be "returned" to the job once the InputStream
+          // open attempt is completed.
+          base::Passed(&delegate_),

[boliu, 2013/03/06 02:30:18]

How bad is it to make delegate RefCountedThreadSafe..? This passing ownership
around feels a bit too weird

[mkosiba (inactive), 2013/03/06 19:05:27]

but the delegate _isn't_ thread safe. I actually like passing the ownership as
it makes it impossible for the Job to access the delegate at the wrong time
without having to rely on any mutexes or anything.
I'll expand the comment to include this additional bit of information.

+          request()->url()),
+      base::Bind(&AndroidStreamReaderURLRequestJob::OnInputStreamOpened,
+                 weak_factory_.GetWeakPtr()));
 }
 
 void AndroidStreamReaderURLRequestJob::Kill() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   weak_factory_.InvalidateWeakPtrs();
   URLRequestJob::Kill();
 }
@@ -103,28 +140,32 @@ AndroidStreamReaderURLRequestJob::CreateStreamReader(InputStream* stream) {
   return make_scoped_ptr(new InputStreamReader(stream));
 }
 
-void AndroidStreamReaderURLRequestJob::StartAsync() {
-  JNIEnv* env = AttachCurrentThread();
-  DCHECK(env);
-
-  // This could be done in the InputStreamReader but would force more
-  // complex synchronization in the delegate.
-  scoped_ptr<android_webview::InputStream> stream(
-      delegate_->OpenInputStream(env, request()));
-
-  if (!stream) {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                     net::ERR_FAILED));
+void AndroidStreamReaderURLRequestJob::OnInputStreamOpened(
+    scoped_ptr<OpenInputStreamResult> result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(result->delegate);
+  delegate_ = result->delegate.Pass();
+  scoped_ptr<InputStream> input_stream = result->input_stream.Pass();
+
+  if (!input_stream) {
+    bool restart_required = false;
+    delegate_->OnInputStreamOpenFailed(request(), &restart_required);
+    if (restart_required) {
+      NotifyRestartRequired();
+    } else {
+      NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                       net::ERR_FAILED));
+    }
     return;
   }
 
   scoped_ptr<InputStreamReader> input_stream_reader(
-      CreateStreamReader(stream.get()));
+      CreateStreamReader(input_stream.get()));
   DCHECK(input_stream_reader);
 
   DCHECK(!input_stream_reader_wrapper_);
-  input_stream_reader_wrapper_ =
-      new InputStreamReaderWrapper(stream.Pass(), input_stream_reader.Pass());
+  input_stream_reader_wrapper_ = new InputStreamReaderWrapper(
+      input_stream.Pass(), input_stream_reader.Pass());
 
   PostTaskAndReplyWithResult(
       GetWorkerThreadRunner(),
@@ -137,6 +178,7 @@ void AndroidStreamReaderURLRequestJob::StartAsync() {
 }
 
 void AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted(int result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Clear the IO_PENDING status set in Start().
   SetStatus(net::URLRequestStatus());
   if (result >= 0) {
@@ -148,6 +190,7 @@ void AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted(int result) {
 }
 
 void AndroidStreamReaderURLRequestJob::OnReaderReadCompleted(int result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // The URLRequest API contract requires that:
   // * NotifyDone be called once, to set the status code, indicate the job is
   //   finished (there will be no further IO),
@@ -174,6 +217,7 @@ base::TaskRunner* AndroidStreamReaderURLRequestJob::GetWorkerThreadRunner() {
 bool AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
                                                    int dest_size,
                                                    int* bytes_read) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(input_stream_reader_wrapper_);
 
   PostTaskAndReplyWithResult(
@@ -193,6 +237,7 @@ bool AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
 
 bool AndroidStreamReaderURLRequestJob::GetMimeType(
     std::string* mime_type) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   JNIEnv* env = AttachCurrentThread();
   DCHECK(env);
 
@@ -208,6 +253,7 @@ bool AndroidStreamReaderURLRequestJob::GetMimeType(
 }
 
 bool AndroidStreamReaderURLRequestJob::GetCharset(std::string* charset) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   JNIEnv* env = AttachCurrentThread();
   DCHECK(env);