Commit 2325ddf1 authored by Daniel Cheng's avatar Daniel Cheng Committed by Commit Bot

CHECK-fail if a renderer tries to commit a no-op navigation.

An HTTP status code of 204/205 indicates that no document should
actually be committed. Since this should all be handled by PlzNavigate
on the browser side, CHECK-fail if this somehow reaches the renderer.

Content-Disposition: attachment has the same behavior, so also
CHECK-fail if this somehow reaches the renderer.

Bug: 1117282
Change-Id: I83a04f4a68963da0ffebede327380365e6f9852e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2380604Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802492}
parent cdd7edf8
...@@ -917,17 +917,19 @@ static bool ShouldNavigate(WebNavigationParams* params, LocalFrame* frame) { ...@@ -917,17 +917,19 @@ static bool ShouldNavigate(WebNavigationParams* params, LocalFrame* frame) {
return true; return true;
int status_code = params->response.HttpStatusCode(); int status_code = params->response.HttpStatusCode();
if (status_code == 204 || status_code == 205) { // If the server sends 204 or 205, this means the server does not want to
// The server does not want us to replace the page contents. // replace the page contents. However, PlzNavigate should have handled it
return false; // browser-side and never sent a commit request to the renderer.
} if (status_code == 204 || status_code == 205)
CHECK(false);
// If the server attached a Content-Disposition indicating that the resource
// is an attachment, this is actually a download. However, PlzNavigate should
// have handled it browser-side and never sent a commit request to the
// renderer.
if (IsContentDispositionAttachment( if (IsContentDispositionAttachment(
params->response.HttpHeaderField(http_names::kContentDisposition))) { params->response.HttpHeaderField(http_names::kContentDisposition))) {
// The server wants us to download instead of replacing the page contents. CHECK(false);
// Downloading is handled by the embedder, but we still get the initial
// response so that we can ignore it and clean up properly.
return false;
} }
const String& mime_type = params->response.MimeType(); const String& mime_type = params->response.MimeType();
...@@ -936,6 +938,8 @@ static bool ShouldNavigate(WebNavigationParams* params, LocalFrame* frame) { ...@@ -936,6 +938,8 @@ static bool ShouldNavigate(WebNavigationParams* params, LocalFrame* frame) {
PluginData* plugin_data = frame->GetPluginData(); PluginData* plugin_data = frame->GetPluginData();
bool can_load_with_plugin = !mime_type.IsEmpty() && plugin_data && bool can_load_with_plugin = !mime_type.IsEmpty() && plugin_data &&
plugin_data->SupportsMimeType(mime_type); plugin_data->SupportsMimeType(mime_type);
// TODO(dcheng): Ideally, this should commit a failed navigation or something,
// instead of silently ignoring the commit request.
if (!can_load_with_plugin) { if (!can_load_with_plugin) {
WebLocalFrameImpl::FromFrame(frame)->Client()->WillFailCommitNavigation( WebLocalFrameImpl::FromFrame(frame)->Client()->WillFailCommitNavigation(
WebLocalFrameClient::CommitFailureReason::kNoPluginForMimeType); WebLocalFrameClient::CommitFailureReason::kNoPluginForMimeType);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment