DCHECK in NavigatorImpl::RequestOpenURL when opening link from subframe in new tab |
|||||
Issue descriptionVersion: 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>
,
Apr 20 2016
I'll take a look. (Interesting, since Nasko just changed the line after the DCHECK yesterday in r388085, but that's unrelated.)
,
Apr 20 2016
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.
,
Apr 21 2016
Fix and test on the way: https://codereview.chromium.org/1911093002/
,
Apr 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6285aedbb91a0c7895e61975cc5055ae24e11aa0 commit 6285aedbb91a0c7895e61975cc5055ae24e11aa0 Author: creis <creis@chromium.org> Date: Thu Apr 21 23:32:42 2016 Remove incorrect DCHECK in RequestOpenURL. BUG= 605055 TEST=Ctrl click any link in a subframe. CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Review URL: https://codereview.chromium.org/1911093002 Cr-Commit-Position: refs/heads/master@{#388948} [modify] https://crrev.com/6285aedbb91a0c7895e61975cc5055ae24e11aa0/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/6285aedbb91a0c7895e61975cc5055ae24e11aa0/content/browser/frame_host/render_frame_host_manager_browsertest.cc [add] https://crrev.com/6285aedbb91a0c7895e61975cc5055ae24e11aa0/content/test/data/ctrl-click-subframe-link.html
,
Apr 22 2016
Thanks, fix confirmed with the ToT build. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by kochi@chromium.org
, Apr 20 2016Status: Assigned (was: Unconfirmed)