Avoid crashing if MHTMLGenerationManager::JobFinished is called twice.

content/browser/download/mhtml_generation_browsertest.cc

 // Copyright (c) 2012 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 <stdint.h>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/common/frame_messages.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/mhtml_generation_params.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "net/base/filename_util.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ skipping 70 matching lines @@
     ASSERT_TRUE(embedded_test_server()->Start());
     ContentBrowserTest::SetUp();
   }
 
   void GenerateMHTML(const base::FilePath& path, const GURL& url) {
     GenerateMHTML(MHTMLGenerationParams(path), url);
   }
 
   void GenerateMHTML(const MHTMLGenerationParams& params, const GURL& url) {
     NavigateToURL(shell(), url);
+    GenerateMHTMLForCurrentPage(params);
+  }
 
+  void GenerateMHTMLForCurrentPage(const MHTMLGenerationParams& params) {
     base::RunLoop run_loop;
 
     shell()->web_contents()->GenerateMHTML(
         params, base::Bind(&MHTMLGenerationTest::MHTMLGenerated, this,
                            run_loop.QuitClosure()));
 
     // Block until the MHTML is generated.
     run_loop.Run();
 
     EXPECT_TRUE(has_mhtml_callback_run());
@@ skipping 97 matching lines @@
   // Make sure the actual generated file has some contents.
   EXPECT_GT(file_size(), 0);  // Verify the size reported by the callback.
   EXPECT_GT(ReadFileSizeFromDisk(path), 100);  // Verify the actual file size.
 
   std::string mhtml;
   ASSERT_TRUE(base::ReadFileToString(path, &mhtml));
   EXPECT_THAT(mhtml,
               HasSubstr("Content-Transfer-Encoding: quoted-printable"));
 }
 
+class GenerateMHTMLAndExitRendererMessageFilter : public BrowserMessageFilter {
+ public:
+  GenerateMHTMLAndExitRendererMessageFilter(
+      RenderProcessHostImpl* render_process_host)
+      : BrowserMessageFilter(FrameMsgStart),
+        render_process_host_(render_process_host) {}
+
+ protected:
+  ~GenerateMHTMLAndExitRendererMessageFilter() override {}
+
+ private:
+  bool OnMessageReceived(const IPC::Message& message) override {
+    if (message.type() == FrameHostMsg_SerializeAsMHTMLResponse::ID) {
+      // After |return false| below, this IPC message will be handled by the
+      // product code as illustrated below.  (1), (2), (3) depict points in time
+      // when product code runs on UI and FILE threads.  (X), (Y), (Z) depict
+      // when we want test-injected tasks to run - for the repro, (Z) has to
+      // happen between (1) and (3).  (Y?) and (Z?) depict when test tasks can
+      // theoretically happen and ruin the repro.
+      //
+      //     IO thread       UI thread           FILE thread
+      //     ---------       ---------           -----------
+      //        |                |                     |
+      //    WE ARE HERE          |                     |
+      //        |                |                     |
+      // after |return false|    |                     |
+      //        +--------------->+                     |
+      //        |                |                     |
+      //        |               (X)                    |
+      //        |                |                     |
+      //        |                |                    (Y?)
+      //        |               (Z?)                   |
+      //        |                |                     |
+      // (1)    |      MHTMLGenerationManager          |
+      //        |      ::OnSerializeAsMHTMLResponse    |
+      //        |                +-------------------->+
+      //        |                |                     |
+      //        |                |                    (Y)
+      //        |                |                     |
+      // (2)    |                |          MHTMLGenerationManager::Job
+      //        |                |          ::CloseFileOnFileThread
+      //        |                |                     |
+      //        |               (Z)                    |
+      //        |         test needs to inject         |
+      //        |        fast renderer shutdown        |
+      //        |      HERE - between (1) and (3)      |
+      //        |                |                     |
+      //        |                |                     |
+      //        |                +<--------------------+
+      //        |                |                     |
+      // (3)    |      MHTMLGenerationManager          |
+      //        |      ::OnFileClosed                  |
+      //        |                |                     |
+      //
+      // We hope that (Z) happens between (1) and (3) by doing the following:
+      // - From here post TaskX to UI thread.  (X) is guaranteed to happen
+      //   before timepoint (1) (because posting of (1) happens after
+      //   |return false| / before we post TaskX below).
+      // - From (X) post TaskY to FILE thread.  Because this posting is done
+      //   before (1), we can guarantee that (Y) will happen before (2).
+      // - From (Y) post TaskZ to UI thread.  Because this posting is done
+      //   before (2), we can guarantee that (Z) will happen before (3).
+      // - We cannot really guarantee that (Y) and (Z) happen *after* (1) - i.e.
+      //   execution at (Y?) and (Z?) instead is possible.  In practice,
+      //   bouncing off of UI and FILE thread does mean (Z) happens after (1).
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE, base::Bind(
+              &GenerateMHTMLAndExitRendererMessageFilter::TaskX,
+              base::Unretained(this)));
+    }
+
+    return false;
+  };
+
+  void TaskX() {
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE, base::Bind(
+            &GenerateMHTMLAndExitRendererMessageFilter::TaskY,
+            base::Unretained(this)));
+  }
+
+  void TaskY() {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, base::Bind(
+            &GenerateMHTMLAndExitRendererMessageFilter::TaskZ,
+            base::Unretained(this)));
+  }
+
+  void TaskZ() {
+    render_process_host_->FastShutdownIfPossible();
+  }
+
+  RenderProcessHostImpl* render_process_host_;
+
+  DISALLOW_COPY_AND_ASSIGN(GenerateMHTMLAndExitRendererMessageFilter);
+};
+
+// Regression test for the crash/race from https://crbug.com/612098.
+IN_PROC_BROWSER_TEST_F(MHTMLGenerationTest, GenerateMHTMLAndExitRenderer) {
+  NavigateToURL(shell(), embedded_test_server()->GetURL("/simple_page.html"));
+
+  RenderProcessHostImpl* render_process_host =
+      static_cast<RenderProcessHostImpl*>(
+          shell()->web_contents()->GetRenderProcessHost());
+  scoped_refptr<BrowserMessageFilter> filter =
+      new GenerateMHTMLAndExitRendererMessageFilter(render_process_host);
+  render_process_host->AddFilter(filter.get());
+
+  base::FilePath path(temp_dir_.path());
+  path = path.Append(FILE_PATH_LITERAL("test.mht"));
+  GenerateMHTMLForCurrentPage(MHTMLGenerationParams(path));
+
+  EXPECT_GT(ReadFileSizeFromDisk(path), 100);  // Verify the actual file size.
+}
+
 IN_PROC_BROWSER_TEST_F(MHTMLGenerationTest, InvalidPath) {
   base::FilePath path(FILE_PATH_LITERAL("/invalid/file/path"));
 
   GenerateMHTML(path, embedded_test_server()->GetURL(
                           "/download/local-about-blank-subframes.html"));
   ASSERT_FALSE(HasFailure());  // No failures with the invocation itself?
 
   EXPECT_EQ(file_size(), -1);  // Expecting that the callback reported failure.
 }
 
@@ skipping 237 matching lines @@
 
   // Make sure that URLs of both frames are present
   // (note that these are single-line regexes).
   EXPECT_THAT(
       mhtml,
       ContainsRegex("Content-Location:.*/frame_tree/page_with_one_frame.html"));
   EXPECT_THAT(mhtml, ContainsRegex("Content-Location:.*/title1.html"));
 }
 
 }  // namespace content