Commit 1656c678 authored by Alex Moshchuk's avatar Alex Moshchuk Committed by Commit Bot

Don't set crash keys if there's no crash in LockToOriginIfNeeded().

Bug: 837009
Change-Id: Ia239d85b546c2265e77564356d2c1a2e2fc3aa74
Reviewed-on: https://chromium-review.googlesource.com/1029006Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554607}
parent 72cf16c0
...@@ -28,6 +28,9 @@ namespace content { ...@@ -28,6 +28,9 @@ namespace content {
int32_t SiteInstanceImpl::next_site_instance_id_ = 1; int32_t SiteInstanceImpl::next_site_instance_id_ = 1;
using CheckOriginLockResult =
ChildProcessSecurityPolicyImpl::CheckOriginLockResult;
SiteInstanceImpl::SiteInstanceImpl(BrowsingInstance* browsing_instance) SiteInstanceImpl::SiteInstanceImpl(BrowsingInstance* browsing_instance)
: id_(next_site_instance_id_++), : id_(next_site_instance_id_++),
active_frame_count_(0), active_frame_count_(0),
...@@ -564,7 +567,7 @@ void SiteInstanceImpl::LockToOriginIfNeeded() { ...@@ -564,7 +567,7 @@ void SiteInstanceImpl::LockToOriginIfNeeded() {
CHECK(!process_->IsForGuestsOnly()); CHECK(!process_->IsForGuestsOnly());
switch (lock_state) { switch (lock_state) {
case ChildProcessSecurityPolicyImpl::CheckOriginLockResult::NO_LOCK: { case CheckOriginLockResult::NO_LOCK: {
// TODO(nick): When all sites are isolated, this operation provides // TODO(nick): When all sites are isolated, this operation provides
// strong protection. If only some sites are isolated, we need // strong protection. If only some sites are isolated, we need
// additional logic to prevent the non-isolated sites from requesting // additional logic to prevent the non-isolated sites from requesting
...@@ -572,8 +575,7 @@ void SiteInstanceImpl::LockToOriginIfNeeded() { ...@@ -572,8 +575,7 @@ void SiteInstanceImpl::LockToOriginIfNeeded() {
policy->LockToOrigin(process_->GetID(), site_); policy->LockToOrigin(process_->GetID(), site_);
break; break;
} }
case ChildProcessSecurityPolicyImpl::CheckOriginLockResult:: case CheckOriginLockResult::HAS_WRONG_LOCK:
HAS_WRONG_LOCK:
// We should never attempt to reassign a different origin lock to a // We should never attempt to reassign a different origin lock to a
// process. // process.
base::debug::SetCrashKeyString(bad_message::GetRequestedSiteURLKey(), base::debug::SetCrashKeyString(bad_message::GetRequestedSiteURLKey(),
...@@ -585,8 +587,7 @@ void SiteInstanceImpl::LockToOriginIfNeeded() { ...@@ -585,8 +587,7 @@ void SiteInstanceImpl::LockToOriginIfNeeded() {
<< " but the process is already locked to " << " but the process is already locked to "
<< policy->GetOriginLock(process_->GetID()); << policy->GetOriginLock(process_->GetID());
break; break;
case ChildProcessSecurityPolicyImpl::CheckOriginLockResult:: case CheckOriginLockResult::HAS_EQUAL_LOCK:
HAS_EQUAL_LOCK:
// Process already has the right origin lock assigned. This case will // Process already has the right origin lock assigned. This case will
// happen for commits to |site_| after the first one. // happen for commits to |site_| after the first one.
break; break;
...@@ -597,15 +598,16 @@ void SiteInstanceImpl::LockToOriginIfNeeded() { ...@@ -597,15 +598,16 @@ void SiteInstanceImpl::LockToOriginIfNeeded() {
// If the site that we've just committed doesn't require a dedicated // If the site that we've just committed doesn't require a dedicated
// process, make sure we aren't putting it in a process for a site that // process, make sure we aren't putting it in a process for a site that
// does. // does.
base::debug::SetCrashKeyString(bad_message::GetRequestedSiteURLKey(), if (lock_state != CheckOriginLockResult::NO_LOCK) {
site_.spec()); base::debug::SetCrashKeyString(bad_message::GetRequestedSiteURLKey(),
base::debug::SetCrashKeyString( site_.spec());
bad_message::GetKilledProcessOriginLockKey(), base::debug::SetCrashKeyString(
policy->GetOriginLock(process_->GetID()).spec()); bad_message::GetKilledProcessOriginLockKey(),
CHECK_EQ(lock_state, policy->GetOriginLock(process_->GetID()).spec());
ChildProcessSecurityPolicyImpl::CheckOriginLockResult::NO_LOCK) CHECK(false) << "Trying to commit non-isolated site " << site_
<< "Trying to commit non-isolated site " << site_ << " in process locked to "
<< " in process locked to " << policy->GetOriginLock(process_->GetID()); << policy->GetOriginLock(process_->GetID());
}
} }
} }
......
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