[Android] Make AndroidStreamReaderURLRequestJob to start loading asynchronously.

android_webview/native/android_stream_reader_url_request_job.h

 // 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.
 
 #ifndef ANDROID_WEBVIEW_NATIVE_ANDROID_STREAM_READER_URL_REQUEST_JOB_H_
 #define ANDROID_WEBVIEW_NATIVE_ANDROID_STREAM_READER_URL_REQUEST_JOB_H_
 
 #include "base/android/scoped_java_ref.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "net/http/http_byte_range.h"
 #include "net/url_request/url_request_job.h"
 
 namespace net {
 class URLRequest;
 }
 
 // A request job that reads data from a Java InputStream.
 class AndroidStreamReaderURLRequestJob : public net::URLRequestJob {
@@ skipping 16 matching lines @@
 
     virtual bool GetCharset(
         JNIEnv* env,
         net::URLRequest* request,
         jobject stream,
         std::string* charset) = 0;
 
     virtual ~Delegate() {}
   };
 
-  explicit AndroidStreamReaderURLRequestJob(
+  AndroidStreamReaderURLRequestJob(
       net::URLRequest* request,
       net::NetworkDelegate* network_delegate,
       scoped_ptr<Delegate> delegate);
 
   // URLRequestJob:
   virtual void Start() OVERRIDE;
   virtual bool ReadRawData(net::IOBuffer* buf,
                            int buf_size,
                            int* bytes_read) OVERRIDE;
   virtual void SetExtraRequestHeaders(
       const net::HttpRequestHeaders& headers) OVERRIDE;
   virtual bool GetMimeType(std::string* mime_type) const OVERRIDE;
   virtual bool GetCharset(std::string* charset) OVERRIDE;
 
  protected:
   virtual ~AndroidStreamReaderURLRequestJob();
 
  private:
   // Verify the requested range against the stream size.
   bool VerifyRequestedRange(JNIEnv* env);
 
   // Skip to the first byte of the requested read range.
   bool SkipToRequestedRange(JNIEnv* env);
 
+  void StartAsync();
+
   net::HttpByteRange byte_range_;
   scoped_ptr<Delegate> delegate_;
   base::android::ScopedJavaGlobalRef<jobject> stream_;
   base::android::ScopedJavaGlobalRef<jbyteArray> buffer_;
+  base::WeakPtrFactory<AndroidStreamReaderURLRequestJob> weak_factory_;

[joth, 2012/10/15 17:01:46]

net::URLRequestJob is refcounted, so it feels like you should just use that to
keep the request alive while the async posttask is in flight, rather than add
weak_ptr semantics into the class too. If there's a specific reason ref-counting
doesn't work, you should add a comment (here, or where the task is posted,
whichever makes more sense).

(If the java-side looses interest and gets GCed I think the jweaks will
automatically turn this into a no-op)

[mnaganov (inactive), 2012/10/15 17:18:25]

I was following the practice that the majority of other request job
implementations employ. I have figured that out from doing a code search for
"public net::URLRequestJob" WeakPtrFactory package:^chrome$

This just seems to be safer than forcing the class to hold a reference to itself
while the task is in flight.

[mkosiba (inactive), 2012/10/15 17:24:03]

hmm.. all of the other URLRequestJob impls use a weak ptr. Maybe if you use a
weak_ptr you're saying that it's OK to delete the job even though the
async_start is in flight (which I guess is remotely possible) rather than
requiring that it's around until it completes (at which point the data isn't
going to be processed either way).

The main concern here is probably about how the URLRequest stuff will behave if
the URLRequestJob lives longer than the URLRequest that it's associated to. 

 
   DISALLOW_COPY_AND_ASSIGN(AndroidStreamReaderURLRequestJob);
 };
 
 bool RegisterAndroidStreamReaderUrlRequestJob(JNIEnv* env);
 
 #endif  // ANDROID_WEBVIEW_NATIVE_ANDROID_STREAM_READER_URL_REQUEST_JOB_H_