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

android_webview/browser/net/android_stream_reader_url_request_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "android_webview/browser/net/android_stream_reader_url_request_job.h"
 
 #include "android_webview/browser/input_stream.h"
 #include "android_webview/browser/net/input_stream_reader.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 50 matching lines @@
  private:
   friend class base::RefCountedThreadSafe<InputStreamReaderWrapper>;
   ~InputStreamReaderWrapper() {}
 
   scoped_ptr<android_webview::InputStream> input_stream_;
   scoped_ptr<android_webview::InputStreamReader> input_stream_reader_;
 
   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,
     scoped_ptr<Delegate> delegate)
     : URLRequestJob(request, network_delegate),
       delegate_(delegate.Pass()),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   DCHECK(delegate_);
 }
 
 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();
 }
 
 scoped_ptr<InputStreamReader>
 AndroidStreamReaderURLRequestJob::CreateStreamReader(InputStream* stream) {
   return make_scoped_ptr(new InputStreamReader(stream));
 }
 
-void AndroidStreamReaderURLRequestJob::StartAsync() {
-  JNIEnv* env = AttachCurrentThread();
-  DCHECK(env);
+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();
 
-  // 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));
+  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(),
       FROM_HERE,
       base::Bind(&InputStreamReaderWrapper::Seek,
                  input_stream_reader_wrapper_,
                  byte_range_),
       base::Bind(&AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted,
                  weak_factory_.GetWeakPtr()));
 }
 
 void AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted(int result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Clear the IO_PENDING status set in Start().
   SetStatus(net::URLRequestStatus());
   if (result >= 0) {
     set_expected_content_size(result);
     NotifyHeadersComplete();
   } else {
     NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, 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),
   // * NotifyReadComplete be called if false is returned from ReadRawData to
   //   indicate that the IOBuffer will not be used by the job anymore.
   // There might be multiple calls to ReadRawData (and thus multiple calls to
   // NotifyReadComplete), which is why NotifyDone is called only on errors
   // (result < 0) and end of data (result == 0).
   if (result == 0) {
     NotifyDone(net::URLRequestStatus());
   } else if (result < 0) {
     NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
   } else {
     // Clear the IO_PENDING status.
     SetStatus(net::URLRequestStatus());
   }
   NotifyReadComplete(result);
 }
 
 base::TaskRunner* AndroidStreamReaderURLRequestJob::GetWorkerThreadRunner() {
   return static_cast<base::TaskRunner*>(BrowserThread::GetBlockingPool());
 }
 
 bool AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
                                                    int dest_size,
                                                    int* bytes_read) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(input_stream_reader_wrapper_);
 
   PostTaskAndReplyWithResult(
       GetWorkerThreadRunner(),
       FROM_HERE,
       base::Bind(&InputStreamReaderWrapper::ReadRawData,
                  input_stream_reader_wrapper_,
                  make_scoped_refptr(dest),
                  dest_size),
       base::Bind(&AndroidStreamReaderURLRequestJob::OnReaderReadCompleted,
                  weak_factory_.GetWeakPtr()));
 
   SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING,
                                   net::ERR_IO_PENDING));
   return false;
 }
 
 bool AndroidStreamReaderURLRequestJob::GetMimeType(
     std::string* mime_type) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   JNIEnv* env = AttachCurrentThread();
   DCHECK(env);
 
   if (!input_stream_reader_wrapper_)
     return false;
 
   // Since it's possible for this call to alter the InputStream a
   // Seek or ReadRawData operation running in the background is not permitted.
   DCHECK(!request_->status().is_io_pending());
 
   return delegate_->GetMimeType(
       env, request(), input_stream_reader_wrapper_->input_stream(), mime_type);
 }
 
 bool AndroidStreamReaderURLRequestJob::GetCharset(std::string* charset) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   JNIEnv* env = AttachCurrentThread();
   DCHECK(env);
 
   if (!input_stream_reader_wrapper_)
     return false;
 
   // Since it's possible for this call to alter the InputStream a
   // Seek or ReadRawData operation running in the background is not permitted.
   DCHECK(!request_->status().is_io_pending());
 
@@ skipping 14 matching lines @@
       } else {
         // We don't support multiple range requests in one single URL request,
         // because we need to do multipart encoding here.
         NotifyDone(net::URLRequestStatus(
             net::URLRequestStatus::FAILED,
             net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
       }
     }
   }
 }