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

Unified Diff: content/renderer/render_frame_impl.cc

Issue 1417323006: OOPIFs: Deduplicating MHTML parts across frames. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@mhtml-serialization-per-frame
Patch Set: Introduced MHTMLPartsGenerationDelegate interface. Created 5 years 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 9e01406593394ecf970e84632b087772fde715ad..04944817cd39c4d36dc98a3950cfd1fa5ab00072 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -126,6 +126,7 @@
#include "content/renderer/web_frame_utils.h"
#include "content/renderer/web_ui_extension.h"
#include "content/renderer/websharedworker_proxy.h"
+#include "crypto/sha2.h"
#include "gin/modules/module_registry.h"
#include "media/audio/audio_output_device.h"
#include "media/base/audio_renderer_mixer_input.h"
@@ -581,6 +582,45 @@ WebString ConvertRelativePathToHtmlAttribute(const base::FilePath& path) {
path.NormalizePathSeparatorsTo(FILE_PATH_LITERAL('/')).AsUTF8Unsafe());
}
+// Implementation of WebPageSerializer::MHTMLPartsGenerationDelegate that
+// 1. Bases shouldSkipResource and getContentID responses on contents of
+// FrameMsg_SerializeAsMHTML_Params.
+// 2. Stores urls of serialized resources (reported via shouldSkipResource)
+// into |uris_of_serialized_resources| passed to the constructor.
+class MHTMLPartsGenerationDelegate
+ : public WebPageSerializer::MHTMLPartsGenerationDelegate {
+ public:
+ MHTMLPartsGenerationDelegate(const FrameMsg_SerializeAsMHTML_Params& params,
+ std::set<GURL>* uris_of_serialized_resources_)
+ : params_(params),
+ uris_of_serialized_resources_(uris_of_serialized_resources_) {
+ DCHECK(uris_of_serialized_resources_);
+ }
+
+ bool shouldSkipResource(const WebURL& url) override {
+ std::string digest =
+ crypto::SHA256HashString(params_.salt + GURL(url).spec());
+
+ if (0 != params_.digests_of_uris_to_skip.count(digest))
dcheng 2015/12/22 01:01:45 ContainsKey? Or find()? Either seems to be more co
Łukasz Anforowicz 2015/12/22 21:06:46 Thanks for pointing out ContainsKey. Done. Side-
+ return true;
+
+ uris_of_serialized_resources_->insert(url);
dcheng 2015/12/22 01:01:45 Any particular reason not to just send the hashes
Łukasz Anforowicz 2015/12/22 21:06:46 Done - I changed the code to send digests (instead
+ return false;
+ }
+
+ WebString getContentID(const WebFrame& frame) override {
+ int routing_id = GetRoutingIdForFrameOrProxy(const_cast<WebFrame*>(&frame));
+ auto it = params_.frame_routing_id_to_content_id.find(routing_id);
+ DCHECK(it != params_.frame_routing_id_to_content_id.end());
+ const std::string& content_id = it->second;
+ return WebString::fromUTF8(content_id);
+ }
+
+ private:
+ const FrameMsg_SerializeAsMHTML_Params& params_;
+ std::set<GURL>* uris_of_serialized_resources_;
+};
dcheng 2015/12/22 01:01:45 Nit: DISALLOW_COPY_AND_ASSIGN
Łukasz Anforowicz 2015/12/22 21:06:46 Oh, right. Done.
+
bool IsContentWithCertificateErrorsRelevantToUI(
const blink::WebURL& url,
const blink::WebCString& security_info,
@@ -4778,28 +4818,17 @@ void RenderFrameImpl::OnGetSerializedHtmlWithLocalLinks(
}
void RenderFrameImpl::OnSerializeAsMHTML(
- int job_id,
- IPC::PlatformFileForTransit file_for_transit,
- const std::string& std_mhtml_boundary,
- const std::map<int, std::string>& frame_routing_id_to_content_id,
- bool is_last_frame) {
+ const FrameMsg_SerializeAsMHTML_Params& params) {
// Unpack IPC payload.
- base::File file = IPC::PlatformFileForTransitToFile(file_for_transit);
- const WebString mhtml_boundary = WebString::fromUTF8(std_mhtml_boundary);
+ base::File file = IPC::PlatformFileForTransitToFile(params.destination_file);
+ const WebString mhtml_boundary =
+ WebString::fromUTF8(params.mhtml_boundary_marker);
DCHECK(!mhtml_boundary.isEmpty());
- std::vector<std::pair<WebFrame*, WebString>> web_frame_to_content_id;
- for (const auto& it : frame_routing_id_to_content_id) {
- const std::string& content_id = it.second;
- WebFrame* web_frame = GetWebFrameFromRoutingIdForFrameOrProxy(it.first);
- if (!web_frame)
- continue;
-
- web_frame_to_content_id.push_back(
- std::make_pair(web_frame, WebString::fromUTF8(content_id)));
- }
WebData data;
bool success = true;
+ std::set<GURL> uris_of_serialized_resources;
+ MHTMLPartsGenerationDelegate delegate(params, &uris_of_serialized_resources);
// Generate MHTML header if needed.
if (IsMainFrame()) {
@@ -4812,8 +4841,8 @@ void RenderFrameImpl::OnSerializeAsMHTML(
// Generate MHTML parts.
if (success) {
- data = WebPageSerializer::generateMHTMLParts(
- mhtml_boundary, GetWebFrame(), false, web_frame_to_content_id);
+ data = WebPageSerializer::generateMHTMLParts(mhtml_boundary, GetWebFrame(),
+ false, delegate);
// TODO(jcivelli): write the chunks in deferred tasks to give a chance to
// the message loop to process other events.
if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
@@ -4822,7 +4851,7 @@ void RenderFrameImpl::OnSerializeAsMHTML(
}
// Generate MHTML footer if needed.
- if (success && is_last_frame) {
+ if (success && params.is_last_frame) {
data = WebPageSerializer::generateMHTMLFooter(mhtml_boundary);
if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
success = false;
@@ -4831,7 +4860,8 @@ void RenderFrameImpl::OnSerializeAsMHTML(
// Cleanup and notify the browser process about completion.
file.Close(); // Need to flush file contents before sending IPC response.
- Send(new FrameHostMsg_SerializeAsMHTMLResponse(routing_id_, job_id, success));
+ Send(new FrameHostMsg_SerializeAsMHTMLResponse(
+ routing_id_, params.job_id, success, uris_of_serialized_resources));
}
void RenderFrameImpl::OpenURL(const GURL& url,

Powered by Google App Engine
This is Rietveld 408576698