New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 605055 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK in NavigatorImpl::RequestOpenURL when opening link from subframe in new tab

Project Member Reported by kochi@chromium.org, Apr 20 2016

Issue description

Version: TOT (As of Apr 20, 2016)
Platform: Linux

Steps to reproduce:
 1. Build debug version of ToT chrome 
 2. go to http://jsbin.com/yipozul/edit?html,output
 3. There are 3 "Google" links on the jsbin output pane.
    Click any of them with Ctrl key pressed (i.e. open in new tab)

Expected:
Open the link in new tab

Actual:
Got the following DCHECK()

[21028:21028:0420/180229:FATAL:navigator_impl.cc(652)] Check failed: !render_frame_host->GetParent() || SiteIsolationPolicy::AreCrossProcessFramesPossible(). 
#0 0x7f51722cfbbe base::debug::StackTrace::StackTrace()
#1 0x7f51723304af logging::LogMessage::~LogMessage()
#2 0x7f516bcf5e4f content::NavigatorImpl::RequestOpenURL()
#3 0x7f516bd03a38 content::RenderFrameHostImpl::OpenURL()
#4 0x7f516bcfec8c content::RenderFrameHostImpl::OnOpenURL()
#5 0x7f516bd18049 _ZN4base20DispatchToMethodImplIPN7content19RenderFrameHostImplEMS2_FvRK27FrameHostMsg_OpenURL_ParamsEJS4_EJLm0EEEEvRKT_T0_RKSt5tupleIJDpT1_EENS_13IndexSequenceIJXspT2_EEEE
#6 0x7f516bd17fa5 _ZN4base16DispatchToMethodIPN7content19RenderFrameHostImplEMS2_FvRK27FrameHostMsg_OpenURL_ParamsEJS4_EEEvRKT_T0_RKSt5tupleIJDpT1_EE
#7 0x7f516bd17f4f _ZN3IPC16DispatchToMethodIN7content19RenderFrameHostImplEMS2_FvRK27FrameHostMsg_OpenURL_ParamsEvSt5tupleIJS3_EEEEvPT_T0_PT1_RKT2_
#8 0x7f516bd0c0e9 _ZN3IPC8MessageTI25FrameHostMsg_OpenURL_MetaSt5tupleIJ27FrameHostMsg_OpenURL_ParamsEEvE8DispatchIN7content19RenderFrameHostImplES8_vMS8_FvRKS3_EEEbPKNS_7MessageEPT_PT0_PT1_T2_
#9 0x7f516bcfbdbb content::RenderFrameHostImpl::OnMessageReceived()
#10 0x7f516c1dc783 content::RenderProcessHostImpl::OnMessageReceived()
#11 0x7f516a12e0a8 IPC::ChannelProxy::Context::OnDispatchMessage()
#12 0x7f516a132736 _ZN4base8internal15RunnableAdapterIMN3IPC12ChannelProxy7ContextEFvRKNS2_7MessageEEE3RunIS4_JS7_EEEvRK13scoped_refptrIT_EDpOT0_
#13 0x7f516a13264e _ZN4base8internal12InvokeHelperILb0EvNS0_15RunnableAdapterIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEEEE8MakeItSoIJRK13scoped_refptrIS5_ES8_EEEvSB_DpOT_
#14 0x7f516a1325fd _ZN4base8internal7InvokerINS_13IndexSequenceIJLm0ELm1EEEENS0_9BindStateINS0_15RunnableAdapterIMN3IPC12ChannelProxy7ContextEFvRKNS6_7MessageEEEEFvPS8_SB_EJSF_SB_EEENS0_12InvokeHelperILb0EvSE_EEFvvEE3RunEPNS0_13BindStateBaseE
#15 0x7f51722aff6e base::Callback<>::Run()
#16 0x7f51722d55ae base::debug::TaskAnnotator::RunTask()
#17 0x7f517234d30c base::MessageLoop::RunTask()
#18 0x7f517234d5a8 base::MessageLoop::DeferOrRunPendingTask()
#19 0x7f517234d772 base::MessageLoop::DoWork()
#20 0x7f517235fb0c base::MessagePumpGlib::HandleDispatch()
#21 0x7f51723602d1 base::(anonymous namespace)::WorkSourceDispatch()
#22 0x7f5163940e04 g_main_context_dispatch
#23 0x7f5163941048 <unknown>
#24 0x7f51639410ec g_main_context_iteration
#25 0x7f517235fc0f base::MessagePumpGlib::Run()
#26 0x7f517234cd1f base::MessageLoop::RunHandler()
#27 0x7f51723f32c4 base::RunLoop::Run()
#28 0x7f5172fb50ef ChromeBrowserMainParts::MainMessageLoopRun()
#29 0x7f516ba4d509 content::BrowserMainLoop::RunMainMessageLoopParts()
#30 0x7f516ba57505 content::BrowserMainRunnerImpl::Run()
#31 0x7f516ba472c6 content::BrowserMain()
#32 0x7f516d5c2a56 content::RunNamedProcessTypeMain()
#33 0x7f516d5c46b9 content::ContentMainRunnerImpl::Run()
#34 0x7f516d5c1ed2 content::ContentMain()
#35 0x7f5172ead490 ChromeMain
#36 0x7f5172ead422 main
#37 0x7f515e3faec5 __libc_start_main
#38 0x7f5172ead325 <unknown>
 

Comment 1 by kochi@chromium.org, Apr 20 2016

Owner: creis@chromium.org
Status: Assigned (was: Unconfirmed)
Assigning creis@, it seems related to OOPIF

Comment 2 by creis@chromium.org, Apr 20 2016

Cc: nasko@chromium.org
I'll take a look.  (Interesting, since Nasko just changed the line after the DCHECK yesterday in r388085, but that's unrelated.)

Comment 3 by creis@chromium.org, Apr 20 2016

Summary: DCHECK in NavigatorImpl::RequestOpenURL when opening link from subframe in new tab (was: DCHECK: [FATAL:navigator_impl.cc(652)] Check failed:)
Ah, it looks like that DCHECK (from my https://codereview.chromium.org/1540463002/) is just mistaken.  We can get to RequestOpenURL for subframes in default Chrome when the WindowOpenDisposition calls for a different tab (e.g., NEW_BACKGROUND_TAB).

This doesn't happen via decidePolicyForNavigation, but rather loadURLExternally:

#0  content::RenderFrameImpl::OpenURL (this=0xb0c7184320, url=..., 
    referrer=..., policy=blink::WebNavigationPolicyNewBackgroundTab, 
    should_replace_current_entry=false, 
    is_history_navigation_in_new_child=false)
    at ../../content/renderer/render_frame_impl.cc:5206
#1  0x00007efc7521bdc4 in content::RenderFrameImpl::loadURLExternally (
    this=0xb0c7184320, request=..., 
    policy=blink::WebNavigationPolicyNewBackgroundTab, suggested_name=..., 
    should_replace_current_entry=false)
    at ../../content/renderer/render_frame_impl.cc:2833
#2  0x00007efc6de7b36a in blink::FrameLoaderClientImpl::loadURLExternally (
    this=0x1646bf4a18c8, request=..., 
    policy=blink::NavigationPolicyNewBackgroundTab, suggestedName=..., 
    shouldReplaceCurrentEntry=false)
    at ../../third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:663
#3  0x00007efc6349e54f in blink::FrameLoader::shouldContinueForNavigationPolicy
    (this=0x83446311138, request=..., substituteData=..., loader=0x0, 
    shouldCheckMainWorldContentSecurityPolicy=blink::CheckContentSecurityPolicy, type=blink::NavigationTypeLinkClicked, 
    policy=blink::NavigationPolicyNewBackgroundTab, 
    replacesCurrentHistoryItem=false, isClientRedirect=false)
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:1361

I don't think the check was preventing anything, so it should be safe to leave the remaining code as is.  I'll see about removing it.

I'm a little surprised we don't have tests for this already, but it looks like targeted links don't hit this bug.  You would need a click modifier like ctrl or shift to trigger it.

Comment 4 by creis@chromium.org, Apr 21 2016

Status: Started (was: Assigned)
Fix and test on the way: https://codereview.chromium.org/1911093002/
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 21 2016

Comment 6 by kochi@chromium.org, Apr 22 2016

Status: Fixed (was: Started)
Thanks, fix confirmed with the ToT build.

Sign in to add a comment