Add diagnostics to catch commits of URLs in incompatible processes.
These should be removed once we have some data indicating how it is
occurring in practice.
BUG=931895
Change-Id: Ic8977b194892328a32d1db6f4492f9bb84938ba4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524835
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641079}diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 428767b..74ecd97 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -13,6 +13,7 @@
#include "base/command_line.h"
#include "base/containers/queue.h"
#include "base/debug/alias.h"
+#include "base/debug/dump_without_crashing.h"
#include "base/hash.h"
#include "base/lazy_instance.h"
#include "base/memory/ptr_util.h"
@@ -4544,6 +4545,33 @@
FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type) ||
!IsURLHandledByNetworkStack(common_params.url));
+ // If this is an attempt to commit a URL in an incompatible process, capture a
+ // crash dump to diagnose why it is occurring.
+ // TODO(creis): Remove this check after we've gathered enough information to
+ // debug issues with browser-side security checks. https://crbug.com/931895.
+ auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+ const GURL& lock_url = GetSiteInstance()->lock_url();
+ if (lock_url != GURL(kUnreachableWebDataURL) &&
+ common_params.url.IsStandard() &&
+ !policy->CanAccessDataForOrigin(GetProcess()->GetID(),
+ common_params.url)) {
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("lock_url",
+ base::debug::CrashKeySize::Size64),
+ lock_url.spec());
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("commit_origin",
+ base::debug::CrashKeySize::Size64),
+ common_params.url.GetOrigin().spec());
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("is_main_frame",
+ base::debug::CrashKeySize::Size32),
+ frame_tree_node_->IsMainFrame() ? "true" : "false");
+ NOTREACHED() << "Commiting in incompatible process for URL: " << lock_url
+ << " lock vs " << common_params.url.GetOrigin();
+ base::debug::DumpWithoutCrashing();
+ }
+
const bool is_first_navigation = !has_committed_any_navigation_;
has_committed_any_navigation_ = true;
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index ad9bf0b..998cab7 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -710,6 +710,37 @@
GetNavigatingWebUI()->RenderFrameCreated(navigation_rfh);
}
+ // If this function picked an incompatible process for the URL, capture a
+ // crash dump to diagnose why it is occurring.
+ // TODO(creis): Remove this check after we've gathered enough information to
+ // debug issues with browser-side security checks. https://crbug.com/931895.
+ auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+ const GURL& lock_url = navigation_rfh->GetSiteInstance()->lock_url();
+ if (lock_url != GURL(kUnreachableWebDataURL) &&
+ request.common_params().url.IsStandard() &&
+ !policy->CanAccessDataForOrigin(navigation_rfh->GetProcess()->GetID(),
+ request.common_params().url)) {
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("lock_url",
+ base::debug::CrashKeySize::Size64),
+ lock_url.spec());
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("commit_origin",
+ base::debug::CrashKeySize::Size64),
+ request.common_params().url.GetOrigin().spec());
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("is_main_frame",
+ base::debug::CrashKeySize::Size32),
+ frame_tree_node_->IsMainFrame() ? "true" : "false");
+ base::debug::SetCrashKeyString(
+ base::debug::AllocateCrashKeyString("use_current_rfh",
+ base::debug::CrashKeySize::Size32),
+ use_current_rfh ? "true" : "false");
+ NOTREACHED() << "Picked an incompatible process for URL: " << lock_url
+ << " lock vs " << request.common_params().url.GetOrigin();
+ base::debug::DumpWithoutCrashing();
+ }
+
return navigation_rfh;
}
