Fix race conditions in ImageDecoder.

chrome/browser/image_decoder_browsertest.cc

Index: chrome/browser/image_decoder_browsertest.cc
diff --git a/chrome/browser/image_decoder_browsertest.cc b/chrome/browser/image_decoder_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5621815a3bab0fb5a732e41c4c232cf6dc77dd71
--- /dev/null
+++ b/chrome/browser/image_decoder_browsertest.cc
@@ -0,0 +1,70 @@
+// Copyright 2015 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 "chrome/browser/image_decoder.h"
+
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/test_utils.h"
+
+using content::BrowserThread;
+
+namespace {
+
+class TestImageRequest : public ImageDecoder::ImageRequest {
+ public:
+  explicit TestImageRequest(const base::Closure& quit_closure)
+      : quit_closure_(quit_closure),
+        quit_called_(false) {
+  }
+
+  ~TestImageRequest() override {
+    if (!quit_called_) {
+      quit_closure_.Run();
+    }
+  }
+
+ private:
+  void OnImageDecoded(const SkBitmap& decoded_image) override {
+    Quit();
+  }
+
+  void OnDecodeImageFailed() override {
+    Quit();
+  }
+
+  void Quit() {
+    EXPECT_FALSE(quit_called_);
+    quit_called_ = true;
+    quit_closure_.Run();
+  }
+
+  base::Closure quit_closure_;
+  bool quit_called_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestImageRequest);
+};
+
+}  // namespace
+
+class ImageDecoderBrowserTest : public InProcessBrowserTest {
+};
+
+IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, Basic) {
+  scoped_refptr<content::MessageLoopRunner> runner =
+      new content::MessageLoopRunner;
+  TestImageRequest test_request(runner->QuitClosure());
+  ImageDecoder::Start(&test_request, std::string());
+  runner->Run();
+}
+
+IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, StartAndDestroy) {
+  scoped_refptr<content::MessageLoopRunner> runner =
+      new content::MessageLoopRunner;
+  scoped_ptr<TestImageRequest> test_request(
+      new TestImageRequest(runner->QuitClosure()));
+  ImageDecoder::Start(test_request.get(), std::string());
+  test_request.reset();

[asargent_no_longer_on_chrome, 2015/04/21 21:14:57]

At this point aren't we left with a stale pointer to the TestImageRequest object
inside the ImageDecoder's RequestMap? Did you mean to call ImageDecoder::Cancel
inside of ~TestImageRequest if quit_called_ is false?

[Lei Zhang, 2015/04/21 22:19:04]

When the code hits TestImageRequest's dtor, it calls ImageRequest's dtor, which
calls ImageDecoder::Cancel(). Without the fix, this test case ends up causing a
use-after-free because ImageDecoder::DecodeImageInSandbox() still had a raw
(freed) pointer. That no longer happens with the fix.

+  runner->Run();
+}