Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(738)

Unified Diff: content/renderer/render_frame_impl.cc

Issue 2379823003: Move MHTML file writing out of the renderer main thread. (Closed)
Patch Set: A few move semantics fixes (I think). Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 54972feabefcc319e1758e46937b7a1ce1d11736..762b604ea6283ffb7165e10c12ae01ece4fe3255 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -10,6 +10,7 @@
#include <vector>
#include "base/auto_reset.h"
+#include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/debug/alias.h"
#include "base/debug/asan_invalid_access.h"
@@ -28,6 +29,7 @@
#include "base/stl_util.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task_runner_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
@@ -729,18 +731,17 @@ class LinkRewritingDelegate : public WebFrameSerializer::LinkRewritingDelegate {
// 1. Bases shouldSkipResource and getContentID responses on contents of
// FrameMsg_SerializeAsMHTML_Params.
// 2. Stores digests of urls of serialized resources (i.e. urls reported via
-// shouldSkipResource) into |digests_of_uris_of_serialized_resources| passed
+// shouldSkipResource) into |serialized_resources_uri_digests| passed
// to the constructor.
class MHTMLPartsGenerationDelegate
: public WebFrameSerializer::MHTMLPartsGenerationDelegate {
public:
MHTMLPartsGenerationDelegate(
const FrameMsg_SerializeAsMHTML_Params& params,
- std::set<std::string>* digests_of_uris_of_serialized_resources)
+ std::set<std::string>* serialized_resources_uri_digests)
: params_(params),
- digests_of_uris_of_serialized_resources_(
- digests_of_uris_of_serialized_resources) {
- DCHECK(digests_of_uris_of_serialized_resources_);
+ serialized_resources_uri_digests_(serialized_resources_uri_digests) {
+ DCHECK(serialized_resources_uri_digests_);
}
bool shouldSkipResource(const WebURL& url) override {
@@ -752,7 +753,7 @@ class MHTMLPartsGenerationDelegate
return true;
// Let's record |url| as being serialized for the *current* frame.
- auto pair = digests_of_uris_of_serialized_resources_->insert(digest);
+ auto pair = serialized_resources_uri_digests_->insert(digest);
bool insertion_took_place = pair.second;
DCHECK(insertion_took_place); // Blink should dedupe within a frame.
@@ -778,11 +779,42 @@ class MHTMLPartsGenerationDelegate
private:
const FrameMsg_SerializeAsMHTML_Params& params_;
- std::set<std::string>* digests_of_uris_of_serialized_resources_;
+ std::set<std::string>* serialized_resources_uri_digests_;
DISALLOW_COPY_AND_ASSIGN(MHTMLPartsGenerationDelegate);
};
+// Writes to file the data serialized and encoded into the WebData instances.
+bool WriteMHTMLToDisk(bool success,
+ std::vector<std::vector<char>> mhtml_contents,
+ base::File file) {
+ TRACE_EVENT_BEGIN0("page-serialization",
+ "WriteMHTMLToDisk (RenderFrameImpl)");
+ DCHECK(!RenderThread::Get()) << "Should not run in the main renderer thread";
+ base::TimeTicks start;
+ if (success) {
+ start = base::TimeTicks::Now();
+ for (const std::vector<char>& data : mhtml_contents) {
+ if (file.WriteAtCurrentPos(&data.front(), data.size()) < 0) {
+ success = false;
+ break;
+ }
+ }
+ }
+ file.Close(); // Need to flush file contents before sending IPC response.
+
+ // Only record histogram if this writing call was not a no-op (|success| being
+ // false from the start).
+ if (!start.is_null()) {
+ UMA_HISTOGRAM_TIMES(
+ "PageSerialization.MhtmlGeneration.WriteToDiskTime.SingleFrame",
+ base::TimeTicks::Now() - start);
+ }
+ TRACE_EVENT_END1("page-serialization", "WriteMHTMLToDisk (RenderFrameImpl)",
+ "success", success);
+ return success;
+}
+
bool IsHttpPost(const blink::WebURLRequest& request) {
return request.httpMethod().utf8() == "POST";
}
@@ -5202,11 +5234,11 @@ void RenderFrameImpl::OnSerializeAsMHTML(
WebString::fromUTF8(params.mhtml_boundary_marker);
DCHECK(!mhtml_boundary.isEmpty());
- // Three WebData instances for header, parts and footer.
- WebData mhtml_contents[3];
- std::set<std::string> digests_of_uris_of_serialized_resources;
- MHTMLPartsGenerationDelegate delegate(
- params, &digests_of_uris_of_serialized_resources);
+ // Holds WebData instances all or some of header, contents/parts and footer.
+ std::vector<std::vector<char>> mhtml_contents;
+ std::set<std::string> serialized_resources_uri_digests;
+ MHTMLPartsGenerationDelegate delegate(params,
+ &serialized_resources_uri_digests);
bool success = true;
@@ -5217,9 +5249,9 @@ void RenderFrameImpl::OnSerializeAsMHTML(
// |data| can be empty if the main frame should be skipped. If the main
// frame is skipped, then the whole archive is bad, so bail to the error
// condition.
- mhtml_contents[0] = WebFrameSerializer::generateMHTMLHeader(
- mhtml_boundary, GetWebFrame(), &delegate);
- success = !mhtml_contents[0].isEmpty();
+ mhtml_contents.emplace_back(WebFrameSerializer::generateMHTMLHeader(
+ mhtml_boundary, GetWebFrame(), &delegate));
+ success = !mhtml_contents.back().empty();
}
// Generate MHTML parts. Note that if this is not the main frame, then even
@@ -5229,42 +5261,49 @@ void RenderFrameImpl::OnSerializeAsMHTML(
TRACE_EVENT0("page-serialization",
"RenderFrameImpl::OnSerializeAsMHTML parts serialization");
// |data| can be empty if the frame should be skipped, but this is OK.
- mhtml_contents[1] = WebFrameSerializer::generateMHTMLParts(
- mhtml_boundary, GetWebFrame(), &delegate);
+ mhtml_contents.emplace_back(WebFrameSerializer::generateMHTMLParts(
+ mhtml_boundary, GetWebFrame(), &delegate));
}
// Generate MHTML footer if needed.
if (success && params.is_last_frame) {
TRACE_EVENT0("page-serialization",
"RenderFrameImpl::OnSerializeAsMHTML footer");
- mhtml_contents[2] = WebFrameSerializer::generateMHTMLFooter(mhtml_boundary);
+ mhtml_contents.emplace_back(
+ WebFrameSerializer::generateMHTMLFooter(mhtml_boundary));
}
- // Writes all serialized data to file.
- // TODO(jcivelli): write the chunks in deferred tasks to give a chance to
- // the message loop to process other events.
- if (success) {
- TRACE_EVENT0("page-serialization",
- "RenderFrameImpl::OnSerializeAsMHTML writing to file");
- SCOPED_UMA_HISTOGRAM_TIMER(
- "PageSerialization.MhtmlGeneration.WriteToDiskTime.SingleFrame");
- for (const WebData& data : mhtml_contents) {
- if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
- success = false;
- break;
- }
- }
- }
-
- // Cleanup and notify the browser process about completion.
- file.Close(); // Need to flush file contents before sending IPC response.
+ // Note: we assume RenderFrameImpl::OnWriteMHTMLToDiskComplete and the rest of
+ // this function will be fast enough to not need to be accounted for in this
+ // metric.
base::TimeDelta main_thread_use_time = base::TimeTicks::Now() - start_time;
- Send(new FrameHostMsg_SerializeAsMHTMLResponse(
- routing_id_, params.job_id, success,
- digests_of_uris_of_serialized_resources, main_thread_use_time));
UMA_HISTOGRAM_TIMES(
"PageSerialization.MhtmlGeneration.RendererMainThreadTime.SingleFrame",
main_thread_use_time);
+
+ base::PostTaskAndReplyWithResult(
+ RenderThreadImpl::current()->GetFileThreadTaskRunner().get(), FROM_HERE,
+ base::Bind(&WriteMHTMLToDisk, success,
+ base::Passed(std::move(mhtml_contents)),
+ base::Passed(std::move(file))),
+ base::Bind(&RenderFrameImpl::OnWriteMHTMLToDiskComplete,
+ base::Unretained(this), params.job_id,
+ base::Passed(std::move(serialized_resources_uri_digests)),
+ main_thread_use_time));
+}
+
+void RenderFrameImpl::OnWriteMHTMLToDiskComplete(
+ int job_id,
+ std::set<std::string> serialized_resources_uri_digests,
+ base::TimeDelta main_thread_use_time,
+ bool success) {
+ DCHECK(RenderThread::Get()) << "Must run in the main renderer thread";
+ // Notify the browser process about completion.
+ // Note: this method must be short enough to not need to be accounted for in
+ // PageSerialization.MhtmlGeneration.RendererMainThreadTime.SingleFrame.
+ Send(new FrameHostMsg_SerializeAsMHTMLResponse(
+ routing_id_, job_id, success, serialized_resources_uri_digests,
+ main_thread_use_time));
}
void RenderFrameImpl::OnFind(int request_id,

Powered by Google App Engine
This is Rietveld 408576698