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

components/nacl/renderer/manifest_downloader.cc

 // Copyright 2014 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/manifest_downloader.h"
 
 #include "base/callback.h"
 #include "components/nacl/renderer/histogram.h"
 #include "components/nacl/renderer/nexe_load_manager.h"
 #include "net/base/net_errors.h"
@@ skipping 36 matching lines @@
     int encoded_data_length) {
   if (buffer_.size() + data_length > kNaClManifestMaxFileBytes) {
     pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_TOO_LARGE;
     buffer_.clear();
   }
 
   if (pp_nacl_error_ == PP_NACL_ERROR_LOAD_SUCCESS)
     buffer_.append(data, data_length);
 }
 
-void ManifestDownloader::didFinishLoading(
-    blink::WebURLLoader* loader,
-    double finish_time,
-    int64_t total_encoded_data_length) {
+void ManifestDownloader::done() {

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

nit: Rename this to Close or something similar. Lower case names are a Blink
convention. Lower case names in this file implement a Blink interface.

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

Oops! Fixed!

   // We log the status code here instead of in didReceiveResponse so that we
   // always log a histogram value, even when we never receive a status code.
   HistogramHTTPStatusCode(
       is_installed_ ? "NaCl.HttpStatusCodeClass.Manifest.InstalledApp" :
                       "NaCl.HttpStatusCodeClass.Manifest.NotInstalledApp",
       status_code_);
 
   cb_.Run(pp_nacl_error_, buffer_);
   delete this;

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

If you haven't changed the Blink ThreadableLoader yet, doesn't this now do a
double delete?

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

Please see this comment. url_loader_ also gets deleted when deleting this.
https://codereview.chromium.org/1267713003/diff/20001/components/nacl/rendere...

 }
 
+void ManifestDownloader::didFinishLoading(
+    blink::WebURLLoader* loader,
+    double finish_time,
+    int64_t total_encoded_data_length) {
+  done();
+}
+
 void ManifestDownloader::didFail(
     blink::WebURLLoader* loader,
     const blink::WebURLError& error) {
   // TODO(teravest): Find a place to share this code with PepperURLLoaderHost.
   pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_LOAD_URL;
   if (error.domain.equals(blink::WebString::fromUTF8(net::kErrorDomain))) {
     switch (error.reason) {
       case net::ERR_ACCESS_DENIED:
       case net::ERR_NETWORK_ACCESS_DENIED:
         pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_NOACCESS_URL;
         break;
     }
   } else {
     // It's a WebKit error.
     pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_NOACCESS_URL;
   }
+
+  done();
 }
 
 }  // namespace nacl