mojo/public/cpp/application/lib/application_impl_chromium.cc - Issue 394903005: mojo: terminate apps if the shell goes away

Unified Diff: mojo/public/cpp/application/lib/application_impl_chromium.cc

Issue 394903005: mojo: terminate apps if the shell goes away (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: rebase again! Created 6 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « mojo/public/cpp/application/lib/application_impl.cc ('k') | mojo/public/cpp/application/lib/application_impl_standalone.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: mojo/public/cpp/application/lib/application_impl_chromium.cc

diff --git a/mojo/views/views_init.cc b/mojo/public/cpp/application/lib/application_impl_chromium.cc

similarity index 54%

copy from mojo/views/views_init.cc

copy to mojo/public/cpp/application/lib/application_impl_chromium.cc

index edd8af34ae9277dd672e8bdd8b948f2255bb4740..1fc8d3b317067246b789cb35ba912f7dd992c477 100644

--- a/mojo/views/views_init.cc

+++ b/mojo/public/cpp/application/lib/application_impl_chromium.cc

@@ -2,17 +2,13 @@

// Use of this source code is governed by a BSD-style license that can be

// found in the LICENSE file.

-#include "mojo/views/views_init.h"

-#include "mojo/views/views_init_internal.h"

+#include "base/message_loop/message_loop.h"

+#include "mojo/public/cpp/application/application_impl.h"

namespace mojo {

-ViewsInit::ViewsInit() {

- InitViews();

-ViewsInit::~ViewsInit() {

+void ApplicationImpl::Terminate() {

+ base::MessageLoop::current()->Quit();

jamesr 2014/07/24 19:52:44 hmm, in core_window_manager_unittests this blows u

hmm, in core_window_manager_unittests this blows up with the following stack: [FATAL:message_loop.cc(314)] Check failed: false. Must be inside Run to call Quit #0 0x7f69178bbcee base::debug::StackTrace::StackTrace() #1 0x7f691794f2e5 logging::LogMessage::~LogMessage() #2 0x7f6917974bfa base::MessageLoop::QuitWhenIdle() #3 0x00000043f2c5 base::MessageLoop::Quit() #4 0x0000006cf1b9 mojo::ApplicationImpl::Terminate() #5 0x00000068b902 mojo::ApplicationImpl::OnShellError() #6 0x00000068ae09 mojo::ApplicationImpl::ShellPtrWatcher::OnConnectionError() #7 0x00000066bee4 mojo::internal::Connector::NotifyError() #8 0x00000066c3fa mojo::internal::Connector::OnHandleReady() #9 0x00000066c323 mojo::internal::Connector::CallOnHandleReady() #10 0x7f69189bda7b mojo::internal::(anonymous namespace)::OnHandleReady() #11 0x7f69189bde97 base::internal::RunnableAdapter<>::Run() #12 0x7f69189bddd4 base::internal::InvokeHelper<>::MakeItSo() #13 0x7f69189bdd2f base::internal::Invoker<>::Run() #14 0x7f69184f8486 base::Callback<>::Run() #15 0x7f69184f817a mojo::common::HandleWatcher::State::OnHandleReady() #16 0x7f69184f82fa mojo::common::HandleWatcher::State::WillDestroyCurrentMessageLoop() #17 0x7f6917973eb8 base::MessageLoop::~MessageLoop() #18 0x00000042135b mojo::WindowManagerApiTest::~WindowManagerApiTest() #19 0x0000004192e5 mojo::WindowManagerApiTest_FocusAndActivateWindow_Test::~WindowManagerApiTest_FocusAndActivateWindow_Test() #20 0x000000419309 mojo::WindowManagerApiTest_FocusAndActivateWindow_Test::~WindowManagerApiTest_FocusAndActivateWindow_Test() #21 0x00000046a06e testing::Test::DeleteSelf_() #22 0x00000047edd3 testing::internal::HandleSehExceptionsInMethodIfSupported<>() #23 0x000000469b8e testing::internal::HandleExceptionsInMethodIfSupported<>() #24 0x00000045fd63 testing::TestInfo::Run() #25 0x00000046030a testing::TestCase::Run() #26 0x000000464918 testing::internal::UnitTestImpl::RunAllTests() #27 0x000000476263 testing::internal::HandleSehExceptionsInMethodIfSupported<>() #28 0x00000046bfee testing::internal::HandleExceptionsInMethodIfSupported<>() #29 0x0000004645a4 testing::UnitTest::Run() #30 0x000000435521 RUN_ALL_TESTS() #31 0x000000434547 base::TestSuite::Run() #32 0x000000428822 base::internal::RunnableAdapter<>::Run() #33 0x00000042878f base::internal::InvokeHelper<>::MakeItSo() #34 0x00000042873a base::internal::Invoker<>::Run() #35 0x00000042c75e base::Callback<>::Run() #36 0x000000428d20 base::(anonymous namespace)::LaunchUnitTestsInternal() #37 0x000000428a17 base::LaunchUnitTests() #38 0x00000042844c main #39 0x7f69150ce76d __libc_start_main #40 0x000000417199 <unknown> In other words, we're destroying the MessageLoop which signals the error handler on the pipe to the shell which then attempts to terminate. Is the shutdown order wrong here? I think the order is supposed to be: 1.) ~MessageLoop to trigger connection errors on all outstanding pipes and run their teardown code with the application still alive (note: order here is not well defined) 2.) ~ApplicationDelegate to tear down the application itself 3.) ~Thread to run TLS destructors with library still loaded 4.) unload library If one of the error handlers tries to quit the message loop, however, it'll blow up in step 1. Maybe this test harness is doing something wrong?

}

} // namespace mojo