Fix cancellation of a pair of URLRequestJob subclasses

content/browser/webui/url_data_manager_backend.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 "content/browser/webui/url_data_manager_backend.h"
 
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 93 matching lines @@
     headers.GetHeader(net::HttpRequestHeaders::kOrigin, &result);
   return result;
 }
 
 }  // namespace
 
 // URLRequestChromeJob is a net::URLRequestJob that manages running
 // chrome-internal resource requests asynchronously.
 // It hands off URL requests to ChromeURLDataManager, which asynchronously
 // calls back once the data is available.
-class URLRequestChromeJob : public net::URLRequestJob,
-                            public base::SupportsWeakPtr<URLRequestChromeJob> {
+class URLRequestChromeJob : public net::URLRequestJob {
  public:
   // |is_incognito| set when job is generated from an incognito profile.
   URLRequestChromeJob(net::URLRequest* request,
                       net::NetworkDelegate* network_delegate,
                       URLDataManagerBackend* backend,
                       bool is_incognito);
 
   // net::URLRequestJob implementation.
   void Start() override;
   void Kill() override;
   bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
   bool GetMimeType(std::string* mime_type) const override;
   int GetResponseCode() const override;
   void GetResponseInfo(net::HttpResponseInfo* info) override;
 
   // Used to notify that the requested data's |mime_type| is ready.
   void MimeTypeAvailable(const std::string& mime_type);
 
   // Called by ChromeURLDataManager to notify us that the data blob is ready
   // for us.
   void DataAvailable(base::RefCountedMemory* bytes);
 
+  // Returns a weak pointer to the job.
+  base::WeakPtr<URLRequestChromeJob> AsWeakPtr();

[davidben, 2015/08/05 18:56:39]

Yuck. :-(

[davidben, 2015/08/05 18:57:08]

[To be clear, nothing actionable. I found the place where we call it and fixing
that probably needn't be done in this CL.]

[mmenke, 2015/08/05 20:00:24]

I completely agree on both counts.  The way the 3 classes are intertwined here
is really ugly.  Spent a little time looking for a really simple fix for this,
but seemed like it would require more work than I'm prepared to spend on it.

+
   void set_mime_type(const std::string& mime_type) {
     mime_type_ = mime_type;
   }
 
   void set_allow_caching(bool allow_caching) {
     allow_caching_ = allow_caching;
   }
 
   void set_add_content_security_policy(bool add_content_security_policy) {
     add_content_security_policy_ = add_content_security_policy;
@@ skipping 112 matching lines @@
 void URLRequestChromeJob::Start() {
   int render_process_id, unused;
   bool is_renderer_request = ResourceRequestInfo::GetRenderFrameForRequest(
       request_, &render_process_id, &unused);
   if (!is_renderer_request)
     render_process_id = kNoRenderProcessId;
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&URLRequestChromeJob::CheckStoragePartitionMatches,
-                 render_process_id, request_->url(), AsWeakPtr()));
+                 render_process_id, request_->url(),
+                 weak_factory_.GetWeakPtr()));
   TRACE_EVENT_ASYNC_BEGIN1("browser", "DataManager:Request", this, "URL",
       request_->url().possibly_invalid_spec());
 }
 
 void URLRequestChromeJob::Kill() {
+  weak_factory_.InvalidateWeakPtrs();
   backend_->RemoveRequest(this);
+  URLRequestJob::Kill();
 }
 
 bool URLRequestChromeJob::GetMimeType(std::string* mime_type) const {
   *mime_type = mime_type_;
   return !mime_type_.empty();
 }
 
 int URLRequestChromeJob::GetResponseCode() const {
   return net::HTTP_OK;
 }
@@ skipping 55 matching lines @@
       pending_buf_ = NULL;
       NotifyReadComplete(bytes_read);
     }
   } else {
     // The request failed.
     NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
                                      net::ERR_FAILED));
   }
 }
 
+base::WeakPtr<URLRequestChromeJob> URLRequestChromeJob::AsWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
 bool URLRequestChromeJob::ReadRawData(net::IOBuffer* buf, int buf_size,
                                       int* bytes_read) {
   if (!data_.get()) {
     SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
     DCHECK(!pending_buf_.get());
     CHECK(buf->data());
     pending_buf_ = buf;
     pending_buf_size_ = buf_size;
     return false;  // Tell the caller we're still waiting for data.
   }
@@ skipping 402 matching lines @@
 
 }  // namespace
 
 net::URLRequestJobFactory::ProtocolHandler*
 CreateDevToolsProtocolHandler(content::ResourceContext* resource_context,
                               bool is_incognito) {
   return new DevToolsJobFactory(resource_context, is_incognito);
 }
 
 }  // namespace content