ImageDataFetcher: handle being deleted when delivering results

This is consistent with the bug backtrace, but not clearly with it
being a regression.

Also don't rely on sequencing within the same expression when setting up
the map.

Bug: 881905
Change-Id: I50ab04196e06e326d4489325fb5bb8adda786c8c
Reviewed-on: https://chromium-review.googlesource.com/1219806
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590326}

components/image_fetcher/core/image_data_fetcher.cc

@@ -120,7 +120,8 @@ void ImageDataFetcher::FetchImageData(
   std::unique_ptr<ImageDataFetcherRequest> request_track(
       new ImageDataFetcherRequest(std::move(callback), std::move(loader)));
 
-  pending_requests_[request_track->loader.get()] = std::move(request_track);
+  network::SimpleURLLoader* loader_raw = request_track->loader.get();
+  pending_requests_[loader_raw] = std::move(request_track);
 }
 
 void ImageDataFetcher::OnURLLoaderComplete(
@@ -155,8 +156,10 @@ void ImageDataFetcher::FinishRequest(const network::SimpleURLLoader* source,
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto request_iter = pending_requests_.find(source);
   DCHECK(request_iter != pending_requests_.end());
-  std::move(request_iter->second->callback).Run(image_data, metadata);
+  auto callback = std::move(request_iter->second->callback);
   pending_requests_.erase(request_iter);
+  std::move(callback).Run(image_data, metadata);
+  // |this| might be destroyed now.
 }
 
 void ImageDataFetcher::InjectResultForTesting(const RequestMetadata& metadata,

components/image_fetcher/core/image_data_fetcher_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
+#include "base/test/bind_test_util.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
@@ -265,4 +266,20 @@ TEST_F(ImageDataFetcherTest, FetchImageData_CancelFetchIfImageExceedsMaxSize) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST_F(ImageDataFetcherTest, DeleteFromCallback) {
+  // Test to make sure that deleting an ImageDataFetcher from the callback
+  // passed to its FetchImageData() does not crash.
+  auto heap_fetcher = std::make_unique<ImageDataFetcher>(shared_factory_);
+  heap_fetcher->FetchImageData(
+      GURL(kImageURL),
+      base::BindLambdaForTesting(
+          [&](const std::string&, const RequestMetadata&) {
+            heap_fetcher = nullptr;
+          }),
+      TRAFFIC_ANNOTATION_FOR_TESTS);
+
+  test_url_loader_factory_.AddResponse(kImageURL, "");
+  base::RunLoop().RunUntilIdle();
+}
+
 }  // namespace image_fetcher