OOPIFs: Transitioning MHTML generation from view-oriented to frame-oriented.

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 <sstream>
+
 #include "base/bind.h"
 #include "base/files/file.h"
+#include "base/guid.h"
+#include "base/rand_util.h"
 #include "base/scoped_observer.h"
 #include "base/stl_util.h"
-#include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/frame_host/frame_tree_node.h"
+#include "content/common/frame_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/web_contents.h"
-#include "content/common/view_messages.h"
 
 namespace content {
 
 // The class and all of its members live on the UI thread.  Only static methods
 // are executed on other threads.
 class MHTMLGenerationManager::Job : public RenderProcessHostObserver {
  public:
   Job(int job_id, WebContents* web_contents, GenerateMHTMLCallback callback);
   ~Job() override;
 
   void set_browser_file(base::File file) { browser_file_ = file.Pass(); }
 
   GenerateMHTMLCallback callback() const { return callback_; }
 
-  // Sends IPC to the renderer, asking for MHTML generation.
+  // Sends IPC to the renderer, asking for MHTML generation of the next frame.
+  //
   // Returns true if the message was sent successfully; false otherwise.
-  bool SendToRenderView();
+  bool SendToNextRenderFrame();
+
+  // Indicates if more calls to SendToNextRenderFrame are needed.
+  bool GotMoreFramesToProcess() {
+    return !pending_frame_tree_node_ids_.empty();
+  }
 
   // Close the file on the file thread and respond back on the UI thread with
   // file size.
   void CloseFile(base::Callback<void(int64 file_size)> callback);
 
   // RenderProcessHostObserver:
   void RenderProcessExited(RenderProcessHost* host,
                            base::TerminationStatus status,
                            int exit_code) override;
   void RenderProcessHostDestroyed(RenderProcessHost* host) override;
 
  private:
+  static std::string GenerateMhtmlBoundaryMarker();
   static int64 CloseFileOnFileThread(base::File file);
+  void AddFrame(RenderFrameHost* render_frame_host);
+
+  // Translates |frame_tree_node_to_content_id_| into
+  // a |site_instance|-specific, routing-id-based map.
+  std::map<int, std::string> CreateFrameRoutingIdToContentId(
+      SiteInstance* site_instance);
 
   // Id used to map renderer responses to jobs.
   // See also MHTMLGenerationManager::id_to_job_ map.
   int job_id_;
 
   // The handle to the file the MHTML is saved to for the browser process.
   base::File browser_file_;
 
-  // The IDs mapping to a specific contents.
-  int process_id_;
-  int routing_id_;
+  // The IDs of frames we still need to process.
+  std::queue<int> pending_frame_tree_node_ids_;
+
+  // Map from frames into content ids (see WebPageSerializer::generateMHTMLParts
+  // for more details about what "content ids" are and how they are used).
+  std::map<int, std::string> frame_tree_node_to_content_id_;
+
+  // MIME multipart boundary to use in the MHTML doc.
+  std::string mhtml_boundary_marker_;
 
   // The callback to call once generation is complete.
   GenerateMHTMLCallback callback_;
 
   // 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,
                                  GenerateMHTMLCallback callback)
     : job_id_(job_id),
-      process_id_(web_contents->GetRenderProcessHost()->GetID()),
-      routing_id_(web_contents->GetRenderViewHost()->GetRoutingID()),
+      mhtml_boundary_marker_(GenerateMhtmlBoundaryMarker()),
       callback_(callback),
       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())

[dcheng, 2015/12/08 06:22:27]

DCHECK_EQ works here

[Łukasz Anforowicz, 2015/12/08 21:26:43]

I've tried this:
109   DCHECK_EQ(nullptr, FrameTreeNode::GloballyFindByID(
110                          pending_frame_tree_node_ids_.front())
111                          ->parent());
and it doesn't compile:
./../base/logging.h:523:23: error: use of overloaded operator '<<' is ambiguous
(with 
operand types 'basic_ostream<char, std::char_traits<char> >' and 'const
nullptr_t')
Candidate functions in include/c++/4.6/ostream included a void*, wchar_t* and
some other overloads.

So I think I'll just stick with the original.  

+             ->parent() == nullptr);
 }
 
 MHTMLGenerationManager::Job::~Job() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
-bool MHTMLGenerationManager::Job::SendToRenderView() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+std::map<int, std::string>
+MHTMLGenerationManager::Job::CreateFrameRoutingIdToContentId(
+    SiteInstance* site_instance) {
+  std::map<int, std::string> result;
+  for (const auto& it : frame_tree_node_to_content_id_) {
+    int ftn_id = it.first;
+    const std::string& content_id = it.second;
+
+    FrameTreeNode* ftn = FrameTreeNode::GloballyFindByID(ftn_id);
+    if (!ftn)
+      continue;
+
+    int routing_id =
+        ftn->render_manager()->GetRoutingIdForSiteInstance(site_instance);
+    if (routing_id == MSG_ROUTING_NONE)
+      continue;
+
+    result[routing_id] = content_id;
+  }
+  return result;
+}
+
+bool MHTMLGenerationManager::Job::SendToNextRenderFrame() {
   DCHECK(browser_file_.IsValid());
+  DCHECK_LT(0u, pending_frame_tree_node_ids_.size());
 
-  RenderViewHost* rvh = RenderViewHost::FromID(process_id_, routing_id_);
-  if (!rvh) {  // The contents went away.
+  int frame_tree_node_id = pending_frame_tree_node_ids_.front();
+  pending_frame_tree_node_ids_.pop();
+  bool is_last_frame = pending_frame_tree_node_ids_.empty();
+
+  FrameTreeNode* ftn = FrameTreeNode::GloballyFindByID(frame_tree_node_id);
+  if (!ftn)  // The contents went away.
     return false;
-  }
+  RenderFrameHost* rfh = ftn->current_frame_host();
 
-  observed_renderer_process_host_.Add(rvh->GetMainFrame()->GetProcess());
+  // Get notified if the target of the IPC message dies between responding.
+  observed_renderer_process_host_.RemoveAll();
+  observed_renderer_process_host_.Add(rfh->GetProcess());
 
   IPC::PlatformFileForTransit renderer_file = IPC::GetFileHandleForProcess(
-      browser_file_.GetPlatformFile(), rvh->GetProcess()->GetHandle(),
+      browser_file_.GetPlatformFile(), rfh->GetProcess()->GetHandle(),
       false);  // |close_source_handle|.
-  rvh->Send(
-      new ViewMsg_SavePageAsMHTML(rvh->GetRoutingID(), job_id_, renderer_file));
+  rfh->Send(new FrameMsg_SerializeAsMHTML(
+      rfh->GetRoutingID(), job_id_, renderer_file, mhtml_boundary_marker_,
+      CreateFrameRoutingIdToContentId(rfh->GetSiteInstance()), is_last_frame));
   return true;
 }
 
 void MHTMLGenerationManager::Job::RenderProcessExited(
     RenderProcessHost* host,
     base::TerminationStatus status,
     int exit_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   MHTMLGenerationManager::GetInstance()->RenderProcessExited(this);
 }
 
+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::ostringstream content_id_builder;
+  content_id_builder << "<frame" << frame_tree_node_id << "@save"

[dcheng, 2015/12/08 06:22:27]

I was going to say that streams aren't permitted in Google style, but it seems
like that restriction has been relaxed.

That being said, I think using a stream typically results in larger generated
code, and it seems like most usages of streams in Chrome are still related to
logging. So maybe we should preserve the status quo? Switching this one to
base::StringPrintf() is pretty easy anyway.

[Łukasz Anforowicz, 2015/12/08 21:26:43]

Yeah, I guess StringPrintf will be clearer indeed.

I guess I try too hard to do thing via "standard C++ way" (sometimes overlooking
simple solutions like string's += vs ostringstream), rather than looking for the
"standard Chromium way".  I'll try to repent... :-)  In the latest patches I've
even tried to improve by reusing a whitespace collapsing function from base in
save_page_browsertest.cc :-)

And I guess .NET has tainted my mind with "strings are immutable and inefficient
to add or append to;  you need StringBuilders" and I keep forgetting that
std::string's += is effectively a nice string builder.

+                     << base::GenerateGUID() << ".mhtml.blink>";
+  std::string content_id = content_id_builder.str();
+  frame_tree_node_to_content_id_[frame_tree_node_id] = content_id;
+}
+
 void MHTMLGenerationManager::Job::RenderProcessHostDestroyed(
     RenderProcessHost* host) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   observed_renderer_process_host_.Remove(host);
 }
 
 void MHTMLGenerationManager::Job::CloseFile(
     base::Callback<void(int64)> callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ skipping 11 matching lines @@
 
 // static
 int64 MHTMLGenerationManager::Job::CloseFileOnFileThread(base::File file) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   DCHECK(file.IsValid());
   int64 file_size = file.GetLength();
   file.Close();
   return file_size;
 }
 
+// static
+std::string MHTMLGenerationManager::Job::GenerateMhtmlBoundaryMarker() {
+  // TODO(lukasza): Introduce and use a shared helper function in
+  // net/base/mime_util.h instead of having the ad-hoc code below.
+
+  // Trying to generate random boundaries similar to IE/UnMHT
+  // (ex: ----=_NextPart_000_001B_01CC157B.96F808A0).
+  const size_t kRandomValuesLength = 10;
+  uint8_t random_values[kRandomValuesLength];
+  base::RandBytes(&random_values, kRandomValuesLength);
+
+  std::ostringstream ss;
+  ss << "----=_NextPart_000_";
+  for (size_t i = 0; i < kRandomValuesLength; ++i) {
+    if (i == 2)
+      ss << '_';
+    else if (i == 6)
+      ss << '.';
+    ss << std::hex << static_cast<uint32_t>(random_values[i]);

[dcheng, 2015/12/08 06:22:27]

https://google.github.io/styleguide/cppguide.html#Streams

If you do use streams, avoid the stateful parts of the streams API (other than
error state), such as imbue(), xalloc(), and register_callback(). Use explicit
formatting functions rather than stream manipulators or formatting flags to
control formatting details such as number base, precision, or padding.

This one is almost certainly easier to read as StringPrintf. There are some
helpers in
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...
for hex<->string conversions.

[ncarter (slow), 2015/12/08 19:31:01]

Agree with Daniel here. I'd use StringPrintf (which is more readable) or
string's operator+= (robust against \0's, though that's not an issue here).

[Łukasz Anforowicz, 2015/12/08 21:26:43]

Done.

+  }
+  return ss.str();
+}
+
 MHTMLGenerationManager* MHTMLGenerationManager::GetInstance() {
   return base::Singleton<MHTMLGenerationManager>::get();
 }
 
 MHTMLGenerationManager::MHTMLGenerationManager() : next_job_id_(0) {}
 
 MHTMLGenerationManager::~MHTMLGenerationManager() {
   STLDeleteValues(&id_to_job_);
 }
 
 void MHTMLGenerationManager::SaveMHTML(WebContents* web_contents,
                                        const base::FilePath& file_path,
                                        const GenerateMHTMLCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   int job_id = NewJob(web_contents, callback);
 
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&MHTMLGenerationManager::CreateFile, file_path),
       base::Bind(&MHTMLGenerationManager::OnFileAvailable,
                  base::Unretained(this),  // Safe b/c |this| is a singleton.
                  job_id));
 }
 
 void MHTMLGenerationManager::OnSavedPageAsMHTML(
     int job_id,
     bool mhtml_generation_in_renderer_succeeded) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  JobStatus job_status = mhtml_generation_in_renderer_succeeded
-                             ? JobStatus::SUCCESS
-                             : JobStatus::FAILURE;
-  JobFinished(job_id, job_status);
+
+  if (!mhtml_generation_in_renderer_succeeded) {
+    JobFinished(job_id, JobStatus::FAILURE);
+    return;
+  }
+
+  Job* job = FindJob(job_id);
+  if (!job)
+    return;
+
+  if (job->GotMoreFramesToProcess()) {
+    if (!job->SendToNextRenderFrame()) {
+      JobFinished(job_id, JobStatus::FAILURE);
+    }
+    return;
+  }
+
+  JobFinished(job_id, JobStatus::SUCCESS);
 }
 
 // static
 base::File MHTMLGenerationManager::CreateFile(const base::FilePath& file_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  base::File browser_file(
-      file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+
+  // SECURITY NOTE: A file descriptor to the file created below will be passed
+  // to multiple renderer processes which (in out-of-process iframes mode) can
+  // act on behalf of separate web principals.  Therefore it is important to
+  // only allow writing to the file and forbid reading from the file (as this
+  // would allow reading content generated by other renderers / other web
+  // principals).
+  uint32 file_flags = base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE;
+
+  base::File browser_file(file_path, file_flags);
   if (!browser_file.IsValid()) {
     LOG(ERROR) << "Failed to create file to save MHTML at: " <<
         file_path.value();
   }
   return browser_file.Pass();
 }
 
 void MHTMLGenerationManager::OnFileAvailable(int job_id,
                                              base::File browser_file) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!browser_file.IsValid()) {
     LOG(ERROR) << "Failed to create file";
     JobFinished(job_id, JobStatus::FAILURE);
     return;
   }
 
   Job* job = FindJob(job_id);
   if (!job)
     return;
 
   job->set_browser_file(browser_file.Pass());
 
-  if (!job->SendToRenderView()) {
+  if (!job->SendToNextRenderFrame()) {
     JobFinished(job_id, JobStatus::FAILURE);
   }
 }
 
 void MHTMLGenerationManager::JobFinished(int job_id, JobStatus job_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   Job* job = FindJob(job_id);
   if (!job)
     return;
@@ skipping 46 matching lines @@
        ++it) {
     if (it->second == job) {
       JobFinished(it->first, JobStatus::FAILURE);
       return;
     }
   }
   NOTREACHED();
 }
 
 }  // namespace content