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

components/nacl/renderer/manifest_downloader.cc

Index: components/nacl/renderer/manifest_downloader.cc
diff --git a/components/nacl/renderer/manifest_downloader.cc b/components/nacl/renderer/manifest_downloader.cc
index 2ea1171cf691b78e70825f07d80bd4d2725e7e0c..c6eadc90b2a95e1df95c5eb5df333ddc89fe2f70 100644
--- a/components/nacl/renderer/manifest_downloader.cc
+++ b/components/nacl/renderer/manifest_downloader.cc
@@ -54,10 +54,7 @@ void ManifestDownloader::didReceiveData(
     buffer_.append(data, data_length);
 }
 
-void ManifestDownloader::didFinishLoading(
-    blink::WebURLLoader* loader,
-    double finish_time,
-    int64_t total_encoded_data_length) {
+void ManifestDownloader::done() {
   // 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(
@@ -69,6 +66,13 @@ void ManifestDownloader::didFinishLoading(
   delete this;
 }
 
+void ManifestDownloader::didFinishLoading(
+    blink::WebURLLoader* loader,
+    double finish_time,
+    int64_t total_encoded_data_length) {
+  done();

[hiroshige, 2015/08/03 07:27:42]

Since didFinishLoading() is still called before the ThreadableLoader fix is
landed, does simply calling done() cause use-after-free?
(done() called from didFail() deletes |this|)

[tyoshino (SeeGerritForStatus), 2015/08/03 07:59:05]

I thought it's fine as deleting this also deletes url_loader_ to kill the loader
so that the loader gets removed from the resource.

- RawResource::didAddClient() doesn't continue to call Resource::didAddClient()
(which contains notifyFinished() call) when hasClient() returns false.
- Resource::checkNotify() uses ResourceClientWalker not to call clients that
have been already removed.

+}
+
 void ManifestDownloader::didFail(
     blink::WebURLLoader* loader,
     const blink::WebURLError& error) {
@@ -85,6 +89,8 @@ void ManifestDownloader::didFail(
     // It's a WebKit error.
     pp_nacl_error_ = PP_NACL_ERROR_MANIFEST_NOACCESS_URL;
   }
+
+  done();
 }
 
 }  // namespace nacl