New issue
Advanced search Search tips

Issue 663458 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

CHECK when running content_browsertests with --isolate-sites-for-testing=*.is

Project Member Reported by lukasza@chromium.org, Nov 8 2016

Issue description

Quite a few failing content_browsertests on Site Isolation Win bot (e.g. https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Win/builds/16885).

Running locally, I see the following DCHECK being hit with --isolate-sites-for-testing=*.is (but not with --site-per-process nor without any OOPIFs):

$ DISPLAY=:20 out/gn/content_browsertests --isolate-sites-for-testing=*.is --gtest_filter=*RenderFrameHostImplBrowserTest.GetVisibilityState_Override*    
...
[77203:77203:1108/111905:508925365633:FATAL:shell_content_browser_client.cc(161)] Check failed: origin.scheme() == effec
tive_site_url.scheme() ( vs. data)a site url should have the same scheme as its origin.
#0 0x0000005b4941 __interceptor_backtrace
#1 0x7efff815bf93 base::debug::StackTrace::StackTrace()
#2 0x7efff81ce7fa logging::LogMessage::~LogMessage()
#3 0x000001612903 content::ShellContentBrowserClient::DoesSiteRequireDedicatedProcess()
#4 0x7efffac00d8d content::SiteInstanceImpl::DoesSiteRequireDedicatedProcess()
#5 0x7efffabff4b8 content::SiteInstanceImpl::LockToOrigin()
#6 0x7efffabff903 content::SiteInstanceImpl::SetSite()
#7 0x7efffa2f8ee1 content::NavigatorImpl::DidNavigate()
#8 0x7efffa30ebe6 content::RenderFrameHostImpl::OnDidCommitProvisionalLoad()

The CHECK_EQ responsible for the test crash is replicated below:

bool ShellContentBrowserClient::DoesSiteRequireDedicatedProcess(
    BrowserContext* browser_context,
    const GURL& effective_site_url) {
  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
  DCHECK(command_line->HasSwitch(switches::kIsolateSitesForTesting));
  std::string pattern =
      command_line->GetSwitchValueASCII(switches::kIsolateSitesForTesting);
  url::Origin origin(effective_site_url);

  // Schemes like blob or filesystem, which have an embedded origin, should
  // already have been canonicalized to the origin site.
  CHECK_EQ(origin.scheme(), effective_site_url.scheme())
      << "a site url should have the same scheme as its origin.";


 
Cc: -nick@chromium.org alex...@chromium.org
Owner: nick@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by nick@chromium.org, Nov 8 2016

The test does a top-level navigation to a "data:text/html,foo".

I believe this CHECK is simply not correct in the case of a data: SiteInstance, and this codepath is only active when --isolate-sites-for-testing is enabled.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd25e8398cf600c73594b88ca85676f44b207efa

commit fd25e8398cf600c73594b88ca85676f44b207efa
Author: nick <nick@chromium.org>
Date: Tue Nov 08 22:18:27 2016

Allow data: SiteInstances in --isolate-sites-for-testing mode

Fixes several crashes in content_browsertests when run with that flag.

BUG=663458

Review-Url: https://codereview.chromium.org/2483333002
Cr-Commit-Position: refs/heads/master@{#430735}

[modify] https://crrev.com/fd25e8398cf600c73594b88ca85676f44b207efa/content/shell/browser/shell_content_browser_client.cc

Owner: ----

Sign in to add a comment