Avoid crashing if MHTMLGenerationManager::JobFinished is called twice.

content/browser/download/mhtml_generation_manager.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 "content/browser/download/mhtml_generation_manager.h"
 
 #include <map>
 #include <queue>
 #include <utility>
 
@@ skipping 26 matching lines @@
       WebContents* web_contents,
       const MHTMLGenerationParams& params,
       const GenerateMHTMLCallback& callback);
   ~Job() override;
 
   int id() const { return job_id_; }
   void set_browser_file(base::File file) { browser_file_ = std::move(file); }
 
   const GenerateMHTMLCallback& callback() const { return callback_; }
 
+  // Indicates whether we expect a message from the |sender| at this time.
+  // We expect only one message per frame - therefore calling this method
+  // will always clear |frame_tree_node_id_of_busy_frame_|.
+  bool IsMessageFromFrameExpected(RenderFrameHostImpl* sender);
+
   // Handler for FrameHostMsg_SerializeAsMHTMLResponse (a notification from the
   // renderer that the MHTML generation for previous frame has finished).
   // Returns |true| upon success; |false| otherwise.
   bool OnSerializeAsMHTMLResponse(
-      RenderFrameHostImpl* sender,
       const std::set<std::string>& digests_of_uris_of_serialized_resources);
 
   // Sends IPC to the renderer, asking for MHTML generation of the next frame.
   //
   // Returns true if the message was sent successfully; false otherwise.
   bool SendToNextRenderFrame();
 
   // Indicates if more calls to SendToNextRenderFrame are needed.
   bool IsDone() const {
     bool waiting_for_response_from_renderer =
         frame_tree_node_id_of_busy_frame_ !=
         FrameTreeNode::kFrameTreeNodeInvalidId;
     bool no_more_requests_to_send = pending_frame_tree_node_ids_.empty();
     return !waiting_for_response_from_renderer && no_more_requests_to_send;
   }
 
   // Close the file on the file thread and respond back on the UI thread with
   // file size.
   void CloseFile(base::Callback<void(int64_t file_size)> callback);
 
   // RenderProcessHostObserver:
   void RenderProcessExited(RenderProcessHost* host,
                            base::TerminationStatus status,
                            int exit_code) override;
   void RenderProcessHostDestroyed(RenderProcessHost* host) override;
 
+  void MarkAsFinished();
+
  private:
   static int64_t CloseFileOnFileThread(base::File file);
   void AddFrame(RenderFrameHost* render_frame_host);
 
   // Creates a new map with values (content ids) the same as in
   // |frame_tree_node_to_content_id_| map, but with the keys translated from
   // frame_tree_node_id into a |site_instance|-specific routing_id.
   std::map<int, std::string> CreateFrameRoutingIdToContentId(
       SiteInstance* site_instance);
 
@@ skipping 24 matching lines @@
   // MIME multipart boundary to use in the MHTML doc.
   std::string mhtml_boundary_marker_;
 
   // Digests of URIs of already generated MHTML parts.
   std::set<std::string> digests_of_already_serialized_uris_;
   std::string salt_;
 
   // The callback to call once generation is complete.
   const GenerateMHTMLCallback callback_;
 
+  // Whether the job is finished (set to true only for the short duration of
+  // time between MHTMLGenerationManager::JobFinished is called and the job is
+  // destroyed by MHTMLGenerationManager::OnFileClosed).
+  bool is_finished_;
+
   // RAII helper for registering this Job as a RenderProcessHost observer.
   ScopedObserver<RenderProcessHost, MHTMLGenerationManager::Job>
       observed_renderer_process_host_;
 
   DISALLOW_COPY_AND_ASSIGN(Job);
 };
 
 MHTMLGenerationManager::Job::Job(int job_id,
                                  WebContents* web_contents,
                                  const MHTMLGenerationParams& params,
                                  const GenerateMHTMLCallback& callback)
     : job_id_(job_id),
       params_(params),
       frame_tree_node_id_of_busy_frame_(FrameTreeNode::kFrameTreeNodeInvalidId),
       mhtml_boundary_marker_(net::GenerateMimeMultipartBoundary()),
       salt_(base::GenerateGUID()),
       callback_(callback),
+      is_finished_(false),
       observed_renderer_process_host_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   web_contents->ForEachFrame(base::Bind(
       &MHTMLGenerationManager::Job::AddFrame,
       base::Unretained(this)));  // Safe because ForEachFrame is synchronous.
 
   // Main frame needs to be processed first.
   DCHECK(!pending_frame_tree_node_ids_.empty());
   DCHECK(FrameTreeNode::GloballyFindByID(pending_frame_tree_node_ids_.front())
              ->parent() == nullptr);
@@ skipping 66 matching lines @@
 }
 
 void MHTMLGenerationManager::Job::RenderProcessExited(
     RenderProcessHost* host,
     base::TerminationStatus status,
     int exit_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   MHTMLGenerationManager::GetInstance()->RenderProcessExited(this);
 }
 
+void MHTMLGenerationManager::Job::MarkAsFinished() {
+  DCHECK(!is_finished_);
+  is_finished_ = true;
+
+  // Stopping RenderProcessExited notifications is needed to avoid calling
+  // JobFinished twice.  See also https://crbug.com/612098.
+  observed_renderer_process_host_.RemoveAll();
+}
+
 void MHTMLGenerationManager::Job::AddFrame(RenderFrameHost* render_frame_host) {
   auto* rfhi = static_cast<RenderFrameHostImpl*>(render_frame_host);
   int frame_tree_node_id = rfhi->frame_tree_node()->frame_tree_node_id();
   pending_frame_tree_node_ids_.push(frame_tree_node_id);
 
   std::string guid = base::GenerateGUID();
   std::string content_id = base::StringPrintf("<frame-%d-%s@mhtml.blink>",
                                               frame_tree_node_id, guid.c_str());
   frame_tree_node_to_content_id_[frame_tree_node_id] = content_id;
 }
@@ skipping 13 matching lines @@
     return;
   }
 
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&MHTMLGenerationManager::Job::CloseFileOnFileThread,
                  base::Passed(std::move(browser_file_))),
       callback);
 }
 
-bool MHTMLGenerationManager::Job::OnSerializeAsMHTMLResponse(
-    RenderFrameHostImpl* sender,
-    const std::set<std::string>& digests_of_uris_of_serialized_resources) {
-  // Sanitize renderer input / reject unexpected messages.
+bool MHTMLGenerationManager::Job::IsMessageFromFrameExpected(
+    RenderFrameHostImpl* sender) {
   int sender_id = sender->frame_tree_node()->frame_tree_node_id();
-  if (sender_id != frame_tree_node_id_of_busy_frame_) {
-    ReceivedBadMessage(sender->GetProcess(),
-                       bad_message::DWNLD_INVALID_SERIALIZE_AS_MHTML_RESPONSE);
-    return false;  // Report failure.
-  }
+  if (sender_id != frame_tree_node_id_of_busy_frame_)
+    return false;
+
+  // We only expect one message per frame - let's make sure subsequent messages
+  // from the same |sender| will be rejected.
   frame_tree_node_id_of_busy_frame_ = FrameTreeNode::kFrameTreeNodeInvalidId;
 
+  return true;
+}
+
+bool MHTMLGenerationManager::Job::OnSerializeAsMHTMLResponse(
+    const std::set<std::string>& digests_of_uris_of_serialized_resources) {
   // Renderer should be deduping resources with the same uris.
   DCHECK_EQ(0u, base::STLSetIntersection<std::set<std::string>>(
                     digests_of_already_serialized_uris_,
                     digests_of_uris_of_serialized_resources).size());
   digests_of_already_serialized_uris_.insert(
       digests_of_uris_of_serialized_resources.begin(),
       digests_of_uris_of_serialized_resources.end());
 
   if (pending_frame_tree_node_ids_.empty())
-    return true;  // Report success.
+    return true;  // Report success - all frames have been processed.
 
   return SendToNextRenderFrame();
 }
 
 // static
 int64_t MHTMLGenerationManager::Job::CloseFileOnFileThread(base::File file) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   DCHECK(file.IsValid());
   int64_t file_size = file.GetLength();
   file.Close();
@@ skipping 26 matching lines @@
 }
 
 void MHTMLGenerationManager::OnSerializeAsMHTMLResponse(
     RenderFrameHostImpl* sender,
     int job_id,
     bool mhtml_generation_in_renderer_succeeded,
     const std::set<std::string>& digests_of_uris_of_serialized_resources) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   Job* job = FindJob(job_id);
-  if (!job) {
+  if (!job || !job->IsMessageFromFrameExpected(sender)) {
+    NOTREACHED();
     ReceivedBadMessage(sender->GetProcess(),
                        bad_message::DWNLD_INVALID_SERIALIZE_AS_MHTML_RESPONSE);
     return;
   }
 
   if (!mhtml_generation_in_renderer_succeeded) {
     JobFinished(job, JobStatus::FAILURE);
     return;
   }
 
   if (!job->OnSerializeAsMHTMLResponse(
-          sender, digests_of_uris_of_serialized_resources)) {
+          digests_of_uris_of_serialized_resources)) {
     JobFinished(job, JobStatus::FAILURE);
     return;
   }
 
   if (job->IsDone())
     JobFinished(job, JobStatus::SUCCESS);
 }
 
 // static
 base::File MHTMLGenerationManager::CreateFile(const base::FilePath& file_path) {
@@ skipping 32 matching lines @@
 
   if (!job->SendToNextRenderFrame()) {
     JobFinished(job, JobStatus::FAILURE);
   }
 }
 
 void MHTMLGenerationManager::JobFinished(Job* job, JobStatus job_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(job);
 
+  job->MarkAsFinished();
   job->CloseFile(
       base::Bind(&MHTMLGenerationManager::OnFileClosed,
                  base::Unretained(this),  // Safe b/c |this| is a singleton.
                  job->id(), job_status));
 }
 
 void MHTMLGenerationManager::OnFileClosed(int job_id,
                                           JobStatus job_status,
                                           int64_t file_size) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 25 matching lines @@
   return iter->second;
 }
 
 void MHTMLGenerationManager::RenderProcessExited(Job* job) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(job);
   JobFinished(job, JobStatus::FAILURE);
 }
 
 }  // namespace content