Make the WebURLLoaderClients in the pepper code destroy (or have some guard) the loader in didFail()

components/nacl/renderer/ppb_nacl_private_impl.cc

 // Copyright 2013 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 "components/nacl/renderer/ppb_nacl_private_impl.h"
 
 #include <numeric>
 #include <string>
 #include <vector>
 
@@ skipping 1517 matching lines @@
         stream_handler_user_data_(stream_handler_user_data),
         success_(false),
         expected_content_length_(-1),
         weak_factory_(this) {}
 
   void Load(const blink::WebURLRequest& request) {
     url_loader_->loadAsynchronously(request, this);
   }
 
  private:
-  virtual void didReceiveResponse(blink::WebURLLoader* loader,
-                                  const blink::WebURLResponse& response) {
+  void didReceiveResponse(blink::WebURLLoader* loader,
+                          const blink::WebURLResponse& response) override {
     success_ = (response.httpStatusCode() == 200);
     if (!success_)
       return;
 
     expected_content_length_ = response.expectedContentLength();
 
     // Defer loading after receiving headers. This is because we may already
     // have a cached translated nexe, so check for that now.
     url_loader_->setDefersLoading(true);
 
@@ skipping 12 matching lines @@
       if (base::StringToLowerASCII(cur) == "no-store")
         has_no_store_header = true;
     }
 
     GetNexeFd(
         instance_, pexe_url_, pexe_opt_level_, last_modified_time, etag,
         has_no_store_header, use_subzero_,
         base::Bind(&PexeDownloader::didGetNexeFd, weak_factory_.GetWeakPtr()));
   }
 
-  virtual void didGetNexeFd(int32_t pp_error,
-                            bool cache_hit,
-                            PP_FileHandle file_handle) {
+  void didGetNexeFd(int32_t pp_error,
+                    bool cache_hit,
+                    PP_FileHandle file_handle) {
     if (!content::PepperPluginInstance::Get(instance_)) {
       delete this;
       return;
     }
 
     HistogramEnumerate("NaCl.Perf.PNaClCache.IsHit", cache_hit, 2);
     HistogramEnumerate(use_subzero_ ? "NaCl.Perf.PNaClCache.IsHit.Subzero"
                                     : "NaCl.Perf.PNaClCache.IsHit.LLC",
                        cache_hit, 2);
     if (cache_hit) {
       stream_handler_->DidCacheHit(stream_handler_user_data_, file_handle);
 
       // We delete the PexeDownloader at this point since we successfully got a
       // cached, translated nexe.
       delete this;
       return;
     }
     stream_handler_->DidCacheMiss(stream_handler_user_data_,
                                   expected_content_length_,
                                   file_handle);
 
     // No translated nexe was found in the cache, so we should download the
     // file to start streaming it.
     url_loader_->setDefersLoading(false);
   }
 
-  virtual void didReceiveData(blink::WebURLLoader* loader,
-                              const char* data,
-                              int data_length,
-                              int encoded_data_length) {
+  void didReceiveData(blink::WebURLLoader* loader,
+                      const char* data,
+                      int data_length,
+                      int encoded_data_length) override {
     if (content::PepperPluginInstance::Get(instance_)) {
       // Stream the data we received to the stream callback.
       stream_handler_->DidStreamData(stream_handler_user_data_,
                                      data,
                                      data_length);
     }
   }
 
-  virtual void didFinishLoading(blink::WebURLLoader* loader,
-                                double finish_time,
-                                int64_t total_encoded_data_length) {
+  void didFinishLoading(blink::WebURLLoader* loader,
+                        double finish_time,
+                        int64_t total_encoded_data_length) override {
     int32_t result = success_ ? PP_OK : PP_ERROR_FAILED;
 
     if (content::PepperPluginInstance::Get(instance_))
       stream_handler_->DidFinishStream(stream_handler_user_data_, result);
     delete this;
   }
 
-  virtual void didFail(blink::WebURLLoader* loader,
-                       const blink::WebURLError& error) {
-    success_ = false;
+  void didFail(blink::WebURLLoader* loader,
+               const blink::WebURLError& error) override {
+    if (content::PepperPluginInstance::Get(instance_))
+      stream_handler_->DidFinishStream(stream_handler_user_data_,
+                                       PP_ERROR_FAILED);
+    delete this;

[bbudge, 2015/08/03 12:59:42]

If ThreadableLoader still calls both didFail and didFinishLoading, doesn't this
do a double-delete?

[tyoshino (SeeGerritForStatus), 2015/08/04 04:51:14]

Please see the reply to the other comment.

   }
 
   PP_Instance instance_;
   scoped_ptr<blink::WebURLLoader> url_loader_;
   std::string pexe_url_;
   int32_t pexe_opt_level_;
   bool use_subzero_;
   const PPP_PexeStreamHandler* stream_handler_;
   void* stream_handler_user_data_;
   bool success_;
@@ skipping 63 matching lines @@
   &StreamPexe
 };
 
 }  // namespace
 
 const PPB_NaCl_Private* GetNaClPrivateInterface() {
   return &nacl_interface;
 }
 
 }  // namespace nacl