Force deletion of test_shell's RequestProxy on the IO thread.

webkit/tools/test_shell/simple_resource_loader_bridge.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.
 //
 // This file contains an implementation of the ResourceLoaderBridge class.
 // The class is implemented using net::URLRequest, meaning it is a "simple"
 // version that directly issues requests. The more complicated one used in the
 // browser uses IPC.
 //
 // Because net::URLRequest only provides an asynchronous resource loading API,
@@ skipping 255 matching lines @@
   scoped_refptr<net::UploadData> upload;
 };
 
 // The interval for calls to RequestProxy::MaybeUpdateUploadProgress
 static const int kUpdateUploadProgressIntervalMsec = 100;
 
 // The RequestProxy does most of its work on the IO thread.  The Start and
 // Cancel methods are proxied over to the IO thread, where an net::URLRequest
 // object is instantiated.
 class RequestProxy : public net::URLRequest::Delegate,
                      public base::RefCountedThreadSafe<RequestProxy> {

[darin (slow to review), 2012/04/05 20:45:21]

wouldn't it be simpler to just use a Traits class here to force destruction of
the RequestProxy onto the proper thread?  then you wouldn't need to use
PostTaskAndReply, right?

[Ami GONE FROM CHROMIUM, 2012/04/05 22:14:31]

Hell yes.  That'd qualify as "a better way to express the semantics I'm trying
to enforce".  Thanks for the tip.

Done.

  public:
   // Takes ownership of the params.
   RequestProxy()
       : download_to_file_(false),
         file_stream_(NULL),
         buf_(new net::IOBuffer(kDataSize)),
         last_upload_position_(0) {
   }
 
   void DropPeer() {
     peer_ = NULL;
   }
 
   void Start(ResourceLoaderBridge::Peer* peer, RequestParams* params) {
     peer_ = peer;
-    owner_loop_ = MessageLoop::current();
+    owner_loop_ = base::MessageLoopProxy::current();
 
     ConvertRequestParamsForFileOverHTTPIfNeeded(params);
     // proxy over to the io thread
     g_io_thread->message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&RequestProxy::AsyncStart, this, params));
   }
 
   void Cancel() {
     // proxy over to the io thread
     g_io_thread->message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&RequestProxy::AsyncCancel, this));
   }
 
  protected:
   friend class base::RefCountedThreadSafe<RequestProxy>;
 
   virtual ~RequestProxy() {
-    // If we have a request, then we'd better be on the io thread!
-    DCHECK(!request_.get() ||
-           MessageLoop::current() == g_io_thread->message_loop());
+    // Ensure we are deleted on the IO thread because base::Timer requires that.
+    DCHECK(MessageLoop::current() == g_io_thread->message_loop());
   }
 
   // --------------------------------------------------------------------------
   // The following methods are called on the owner's thread in response to
   // various net::URLRequest callbacks.  The event hooks, defined below, trigger
   // these methods asynchronously.
 
   void NotifyReceivedRedirect(const GURL& new_url,
                               const ResourceResponseInfo& info) {
     bool has_new_first_party_for_cookies = false;
@@ skipping 147 matching lines @@
       }  // else wait for OnReadCompleted
     } else {
       Done();
     }
   }
 
   // --------------------------------------------------------------------------
   // The following methods are event hooks (corresponding to net::URLRequest
   // callbacks) that run on the IO thread.  They are designed to be overridden
   // by the SyncRequestProxy subclass.
+  //
+  // Note that since |this| must be ultimately destroyed on the IO thread, each
+  // of these uses a do-nothing reply closure to extend |this|'s lifetime long
+  // enough to avoid being deleted on the owner thread.
 
   virtual void OnReceivedRedirect(
       const GURL& new_url,
       const ResourceResponseInfo& info,
       bool* defer_redirect) {
     *defer_redirect = true;  // See AsyncFollowDeferredRedirect
-    owner_loop_->PostTask(
+    owner_loop_->PostTaskAndReply(
         FROM_HERE,
-        base::Bind(&RequestProxy::NotifyReceivedRedirect, this, new_url, info));
+        base::Bind(&RequestProxy::NotifyReceivedRedirect, this, new_url, info),
+        base::Bind(&RequestProxy::DoNothing, this));
   }
 
   virtual void OnReceivedResponse(
       const ResourceResponseInfo& info) {
-    owner_loop_->PostTask(
+    owner_loop_->PostTaskAndReply(
         FROM_HERE,
-        base::Bind(&RequestProxy::NotifyReceivedResponse, this, info));
+        base::Bind(&RequestProxy::NotifyReceivedResponse, this, info),
+        base::Bind(&RequestProxy::DoNothing, this));
   }
 
   virtual void OnReceivedData(int bytes_read) {
     if (download_to_file_) {
       file_stream_.WriteSync(buf_->data(), bytes_read);
-      owner_loop_->PostTask(
+      owner_loop_->PostTaskAndReply(
           FROM_HERE,
-          base::Bind(&RequestProxy::NotifyDownloadedData, this, bytes_read));
+          base::Bind(&RequestProxy::NotifyDownloadedData, this, bytes_read),
+          base::Bind(&RequestProxy::DoNothing, this));
       return;
     }
 
-    owner_loop_->PostTask(
+    owner_loop_->PostTaskAndReply(
         FROM_HERE,
-        base::Bind(&RequestProxy::NotifyReceivedData, this, bytes_read));
+        base::Bind(&RequestProxy::NotifyReceivedData, this, bytes_read),
+        base::Bind(&RequestProxy::DoNothing, this));
   }
 
   virtual void OnCompletedRequest(const net::URLRequestStatus& status,
                                   const std::string& security_info,
                                   const base::TimeTicks& complete_time) {
     if (download_to_file_)
       file_stream_.CloseSync();
-    owner_loop_->PostTask(
+    owner_loop_->PostTaskAndReply(
         FROM_HERE,
         base::Bind(&RequestProxy::NotifyCompletedRequest, this, status,
-                   security_info, complete_time));
+                   security_info, complete_time),
+        base::Bind(&RequestProxy::DoNothing, this));
   }
 
   // --------------------------------------------------------------------------
   // net::URLRequest::Delegate implementation:
 
   virtual void OnReceivedRedirect(net::URLRequest* request,
                                   const GURL& new_url,
                                   bool* defer_redirect) OVERRIDE {
     DCHECK(request->status().is_success());
     ResourceResponseInfo info;
@@ skipping 77 matching lines @@
     uint64 amt_since_last = position - last_upload_position_;
     base::TimeDelta time_since_last = base::TimeTicks::Now() -
                                       last_upload_ticks_;
 
     bool is_finished = (size == position);
     bool enough_new_progress = (amt_since_last > (size /
                                                   kHalfPercentIncrements));
     bool too_much_time_passed = time_since_last > kOneSecond;
 
     if (is_finished || enough_new_progress || too_much_time_passed) {
-      owner_loop_->PostTask(
+      owner_loop_->PostTaskAndReply(
           FROM_HERE,
-          base::Bind(&RequestProxy::NotifyUploadProgress, this, position,
-                     size));
+          base::Bind(&RequestProxy::NotifyUploadProgress, this, position, size),
+          base::Bind(&RequestProxy::DoNothing, this));
       last_upload_ticks_ = base::TimeTicks::Now();
       last_upload_position_ = position;
     }
   }
 
   void PopulateResponseInfo(net::URLRequest* request,
                             ResourceResponseInfo* info) const {
     info->request_time = request->request_time();
     info->response_time = request->response_time();
     info->headers = request->response_headers();
@@ skipping 90 matching lines @@
   bool download_to_file_;
   net::FileStream file_stream_;
   scoped_refptr<ShareableFileReference> downloaded_file_;
 
   // Size of our async IO data buffers
   static const int kDataSize = 16*1024;
 
   // read buffer for async IO
   scoped_refptr<net::IOBuffer> buf_;
 
-  MessageLoop* owner_loop_;
+  scoped_refptr<base::MessageLoopProxy> owner_loop_;
 
   // This is our peer in WebKit (implemented as ResourceHandleInternal). We do
   // not manage its lifetime, and we may only access it from the owner's
   // message loop (owner_loop_).
   ResourceLoaderBridge::Peer* peer_;
 
   // Timer used to pull upload progress info.
   base::RepeatingTimer<RequestProxy> upload_progress_timer_;
 
   // Info used to determine whether or not to send an upload progress update.
   uint64 last_upload_position_;
   base::TimeTicks last_upload_ticks_;
 
   // Save the real FILE URL prefix for the FILE URL which converts to HTTP URL.
   std::string file_url_prefix_;
   // Save a failed file request status to pass it to webkit.
   scoped_ptr<net::URLRequestStatus> failed_file_request_status_;
+
+ private:
+  // Helper used to extend the lifetime of |this| to ensure deletion on the IO
+  // thread.
+  void DoNothing() {}
 };
 
 //-----------------------------------------------------------------------------
 
 class SyncRequestProxy : public RequestProxy {
  public:
   explicit SyncRequestProxy(ResourceLoaderBridge::SyncLoadResponse* result)
       : result_(result), event_(true, false) {
   }
 
@@ skipping 338 matching lines @@
          (http_prefix.SchemeIs("http") || http_prefix.SchemeIs("https")));
   g_file_over_http_params = new FileOverHTTPParams(file_path_template,
                                                    http_prefix);
 }
 
 // static
 webkit_glue::ResourceLoaderBridge* SimpleResourceLoaderBridge::Create(
     const webkit_glue::ResourceLoaderBridge::RequestInfo& request_info) {
   return new ResourceLoaderBridgeImpl(request_info);
 }