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

Issue 890963 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 870128
issue 884065



Sign in to add a comment

chrome://oobe crashes due to nested ContextualSearchJsApiService Mojo request

Project Member Reported by khorimoto@chromium.org, Oct 1

Issue description

I'm in the process of adding Mojo bindings to chrome://oobe. For a work-in-progress CL, see [1].

When I add these bindings, I get this fatal error on startup:

[60469:60469:1001/123811.380270:ERROR:mojo_web_ui_controller.cc(29)] Terminating renderer for requesting contextual_search.mojom.ContextualSearchJsApiService interface from subframe
[60469:60469:1001/123811.411606:FATAL:login_display_host_webui.cc(799)] Renderer crash on login window
#0 0x7fd5338ab92c base::debug::StackTrace::StackTrace()
#1 0x7fd5337df9ab logging::LogMessage::~LogMessage()
#2 0x557b47b1f780 chromeos::LoginDisplayHostWebUI::RenderProcessGone()
#3 0x7fd530d11ff4 content::WebContentsImpl::RenderViewTerminated()
#4 0x7fd530ba26bd content::RenderProcessHostImpl::ProcessDied()
#5 0x7fd531e3e35e _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12ChannelProxy7ContextEFvvEJ13scoped_refptrIS5_EEEEFvvEE3RunEPNS0_13BindStateBaseE
#6 0x7fd5337c2cf5 base::debug::TaskAnnotator::RunTask()
#7 0x7fd5337edc2e base::MessageLoop::RunTask()
#8 0x7fd5337ee053 base::MessageLoop::DoWork()
#9 0x7fd5338ceda9 base::MessagePumpLibevent::Run()
#10 0x7fd5337ed7c4 base::MessageLoop::Run()
#11 0x7fd53381feb9 base::RunLoop::Run()
#12 0x557b47e7867d ChromeBrowserMainParts::MainMessageLoopRun()
#13 0x7fd5307f3367 content::BrowserMainLoop::RunMainMessageLoopParts()
#14 0x7fd5307f6096 content::BrowserMainRunnerImpl::Run()
#15 0x7fd5307ef4d9 content::BrowserMain()
#16 0x7fd53126fe4f content::ContentMainRunnerImpl::Run()
#17 0x7fd533b6d025 service_manager::Main()
#18 0x7fd53126e2e4 content::ContentMain()
#19 0x557b4722fee3 ChromeMain
#20 0x7fd5245422b1 __libc_start_main
#21 0x557b4722fd5a _start

This crash corresponds to the check at [2].

I tried commenting out the ShutdownForBadMessage() call and replacing it with an early return, and things seemed to work normally. Is it truly necessary to kill the renderer in this case? Can we safely ignore such a message?

Alternatively, should the ContextualSearchJsApiService request be removed?

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1255591
[2] https://cs.chromium.org/chromium/src/ui/webui/mojo_web_ui_controller.cc?q=GetParent
 
Cc: dcheng@chromium.org
[+dcheng who may know about mojo render subframe limitations, and helped review the existing code]

It sounds like the Contextual Search JS API Service should not be instantiated from a sub-frame renderer, and that's what's causing the problem? 
[60469:60469:1001/123811.380270:ERROR:mojo_web_ui_controller.cc(29)] Terminating renderer for requesting contextual_search.mojom.ContextualSearchJsApiService interface from subframe
Is this a known limitation?  If a preflight-check needs to be made for subframes I may be able to help or review.
Cc: alemate@chromium.org
Re: comment #1: The limitation was introduced by calamity@ in https://chromium-review.googlesource.com/c/chromium/src/+/991497, but I'm not sure why it needed to be introduced there. Chris, can you chime in here?

alemate@: Any idea why an OOBE subframe is requesting this connection?
I think every renderer when starting up checks to see if it's the Contextual Search overlay panel, but only root frames should be making that check.

What's OOBE? 
OOBE stands for Out-of-box Experience, which is the setup wizard shown to users when they turn their Chromebooks on for the first time or log in as a new user on a Chromebook.

It's implemented as a WebUI page, and its WebUIController is https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/chromeos/login/oobe_ui.h.
Cc: sa...@chromium.org
So, after a chat with sammc@, IIUC, the subframe renderer is being taken down when requesting any Mojo interface, not just WebUI ones.

The fix is just going to be adding 

if (!registry_.CanBindInterface(interface_name))
  return;

at the top of OnInterfaceRequestFromFrame().

Does that sound right to everyone?
SGTM. Are you sending out a patch, or do you want me to do so?
Owner: khorimoto@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 4

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

commit 304f457b17ba2d3d70e5d3a1dd28f2b3ad2df630
Author: Kyle Horimoto <khorimoto@google.com>
Date: Thu Oct 04 16:06:03 2018

[CrOS MultiDevice] Ignore Mojo WebUI requests that cannot be handled.

Before this CL, when such a request was received, the renderer was
shut down. This prevents a crash in chrome://oobe resulting from a
rejected request.

Bug:  890963 
Change-Id: Icd54728b877126af218ce762f888f1c252f56ef7
Reviewed-on: https://chromium-review.googlesource.com/c/1258981
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596691}
[modify] https://crrev.com/304f457b17ba2d3d70e5d3a1dd28f2b3ad2df630/ui/webui/mojo_web_ui_controller.cc

Status: Fixed (was: Started)

Sign in to add a comment