Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(81)

Issue 2230283003: Revert of Establish MojoChildConnection from BrowserChildProcessHostImpl (Closed)

Created:
4 years, 4 months ago by foolip
Modified:
4 years, 4 months ago
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, jam, piman+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@renderer-channel
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Establish MojoChildConnection from BrowserChildProcessHostImpl (patchset #9 id:190001 of https://codereview.chromium.org/2221153003/ ) Reason for revert: ContentBrowserTest.BrowserCrashCallStack/RendererCrashCallStack are failing on "Linux Tests (dbg)(1)" and "Linux Tests (dbg)(1)(32)" since this CL. Reproduced locally and bisected to "Establish MojoChildConnection from BrowserChildProcessHostImpl", also verified that reverting it on master locally fixes the problem. When it fails, the tests stall for a long time before failing. Sample from bot: ContentBrowserTest.RendererCrashCallStack (run #1): [ RUN ] ContentBrowserTest.RendererCrashCallStack [11795:11795:0810/064214:8507100974:WARNING:audio_manager.cc(317)] Multiple instances of AudioManager detected [11795:11795:0810/064214:8507101034:WARNING:audio_manager.cc(278)] Multiple instances of AudioManager detected Xlib: extension "RANDR" missing on display ":9". BrowserTestBase signal handler received SIGTERM. Backtrace: #0 0x7fdba1ca413e base::debug::StackTrace::StackTrace() #1 0x00000145ffaa content::(anonymous namespace)::DumpStackTraceSignalHandler() #2 0x7fdb929cf0b0 <unknown> #3 0x7fdb92d65d2b __libc_read #4 0x7fdba1db7e42 base::GetAppOutputInternal() #5 0x7fdba1db7ff6 base::GetAppOutputAndError() #6 0x000000993436 content::ContentBrowserTest_RendererCrashCallStack_Test::RunTestOnMainThread() #7 0x00000144f6ae content::ContentBrowserTest::RunTestOnMainThreadLoop() #8 0x00000145fd24 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() #9 0x0000005b969d _ZN4base8internal13FunctorTraitsIMN4mojo13StrongBindingIN6device14BatteryMonitorEEEFvvEvE6InvokeIPS6_JEEEvS8_OT_DpOT0_ #10 0x0000005b95f1 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN4mojo13StrongBindingIN6device14BatteryMonitorEEEFvvEJPS8_EEEvOT_DpOT0_ #11 0x000001461247 _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE #12 0x0000014611ac _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE #13 0x0000005b971e base::Callback<>::Run() #14 0x0000015920a8 content::ShellBrowserMainParts::PreMainMessageLoopRun() #15 0x7fdba3457031 content::BrowserMainLoop::PreMainMessageLoopRun() #16 0x7fdba29ee2bd _ZN4base8internal13FunctorTraitsIMN4mojo10BindingSetIN7content5mojom14ProcessControlEE7ElementEFvvEvE6InvokeIPS8_JEEEvSA_OT_DpOT0_ #17 0x7fdba345f091 _ZN4base8internal12InvokeHelperILb0EiE8MakeItSoIRKMN7content15BrowserMainLoopEFivEJPS5_EEEiOT_DpOT0_ #18 0x7fdba345f037 _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEiOT_OT0_NS_13IndexSequenceIJXspT1_EEEE #19 0x7fdba345ef9c _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE3RunEPNS0_13BindStateBaseE #20 0x7fdba29ee36e base::Callback<>::Run() #21 0x7fdba3ef057b content::StartupTaskRunner::RunAllTasksNow() #22 0x7fdba3455033 content::BrowserMainLoop::CreateStartupTasks() #23 0x7fdba346164d content::BrowserMainRunnerImpl::Initialize() #24 0x000001541fc4 ShellBrowserMain() #25 0x000001521e46 content::ShellMainDelegate::RunProcess() #26 0x7fdba5057e3b content::RunNamedProcessTypeMain() #27 0x7fdba505a172 content::ContentMainRunnerImpl::Run() #28 0x7fdba5056f42 content::ContentMain() #29 0x00000145fa46 content::BrowserTestBase::SetUp() #30 0x00000144f52d content::ContentBrowserTest::SetUp() #31 0x0000016a05da testing::internal::HandleSehExceptionsInMethodIfSupported<>() #32 0x000001691b0e testing::internal::HandleExceptionsInMethodIfSupported<>() #33 0x0000016868a3 testing::Test::Run() #34 0x000001687098 testing::TestInfo::Run() #35 0x00000168763a testing::TestCase::Run() #36 0x00000168c98c testing::internal::UnitTestImpl::RunAllTests() #37 0x0000016a4d6a testing::internal::HandleSehExceptionsInMethodIfSupported<>() #38 0x0000016931ee testing::internal::HandleExceptionsInMethodIfSupported<>() #39 0x00000168c62f testing::UnitTest::Run() #40 0x0000014d1d11 RUN_ALL_TESTS() #41 0x0000014d0bc2 base::TestSuite::Run() #42 0x00000145edcc content::ContentTestLauncherDelegate::RunTestSuite() #43 0x00000149b712 content::LaunchTests() #44 0x00000145ec28 main #45 0x7fdb929ba7ed __libc_start_main #46 0x0000005612c5 <unknown> Original issue's description: > Establish MojoChildConnection from BrowserChildProcessHostImpl > > Rather than have individual BrowserChildProcessHostImpl users > create their own connection, this moves all that junk into BCPHI > and allows its users to configure it with a service name and > instance ID. > > Also changes ChildProcessHost to support ChannelMojo connection > via remote interfaces provided by its delegate, and uses this > in conjunction with the BCPHI changes. Eliminates one more use > of kMojoChannelToken. > > BUG=623396 > R=ben@chromium.org > > Committed: https://crrev.com/8efa71b85282fb4df1d3836a20d086d223df714f > Cr-Commit-Position: refs/heads/master@{#410937} TBR=ben@chromium.org,rockot@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=623396 Committed: https://crrev.com/83cff6863fbd3949ed21debdf642105ebff9825d Cr-Commit-Position: refs/heads/master@{#411039}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+145 lines, -240 lines) Patch
M chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/task_manager/providers/child_process_task.cc View 4 chunks +11 lines, -9 lines 0 comments Download
M content/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/browser_child_process_host_impl.h View 5 chunks +3 lines, -10 lines 0 comments Download
M content/browser/browser_child_process_host_impl.cc View 9 chunks +13 lines, -31 lines 0 comments Download
M content/browser/gpu/gpu_process_host.h View 4 chunks +10 lines, -6 lines 0 comments Download
M content/browser/gpu/gpu_process_host.cc View 9 chunks +25 lines, -12 lines 0 comments Download
M content/browser/mojo/mojo_shell_context.cc View 3 chunks +2 lines, -3 lines 0 comments Download
M content/browser/ppapi_plugin_process_host.h View 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/ppapi_plugin_process_host.cc View 6 chunks +14 lines, -6 lines 0 comments Download
M content/browser/utility_process_host_impl.h View 2 chunks +6 lines, -0 lines 0 comments Download
M content/browser/utility_process_host_impl.cc View 7 chunks +23 lines, -6 lines 0 comments Download
M content/child/child_thread_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/common/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M content/common/DEPS View 1 chunk +0 lines, -2 lines 0 comments Download
M content/common/child_process_host_impl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/common/child_process_host_impl.cc View 3 chunks +0 lines, -26 lines 0 comments Download
M content/common/mojo/constants.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/common/mojo/constants.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M content/content_resources.grd View 1 chunk +0 lines, -1 line 0 comments Download
M content/content_tests.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M content/public/app/BUILD.gn View 1 chunk +0 lines, -6 lines 0 comments Download
M content/public/app/mojo/content_browser_manifest.json View 1 chunk +1 line, -2 lines 0 comments Download
D content/public/app/mojo/content_plugin_manifest.json View 1 chunk +0 lines, -12 lines 0 comments Download
M content/public/browser/browser_child_process_host.h View 2 chunks +8 lines, -4 lines 0 comments Download
M content/public/browser/browser_child_process_host_delegate.h View 2 chunks +8 lines, -0 lines 0 comments Download
M content/public/browser/browser_child_process_host_delegate.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M content/public/common/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M content/public/common/child_process_host.h View 2 chunks +0 lines, -15 lines 0 comments Download
M content/public/common/child_process_host_delegate.h View 2 chunks +0 lines, -8 lines 0 comments Download
M content/public/common/child_process_host_delegate.cc View 1 chunk +0 lines, -4 lines 0 comments Download
D content/public/test/test_mojo_shell_context.h View 1 chunk +0 lines, -31 lines 0 comments Download
D content/public/test/test_mojo_shell_context.cc View 1 chunk +0 lines, -16 lines 0 comments Download
M content/public/test/test_utils.h View 2 chunks +0 lines, -2 lines 0 comments Download
M content/public/test/test_utils.cc View 2 chunks +1 line, -2 lines 0 comments Download
M services/catalog/entry.h View 4 chunks +4 lines, -6 lines 0 comments Download
M services/catalog/entry.cc View 2 chunks +2 lines, -1 line 0 comments Download
M services/catalog/reader.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M services/shell/service_manager.cc View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
foolip
Created Revert of Establish MojoChildConnection from BrowserChildProcessHostImpl
4 years, 4 months ago (2016-08-10 14:49:52 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2230283003/1
4 years, 4 months ago (2016-08-10 14:50:16 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-10 14:51:28 UTC) #5
commit-bot: I haz the power
4 years, 4 months ago (2016-08-10 14:53:33 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/83cff6863fbd3949ed21debdf642105ebff9825d
Cr-Commit-Position: refs/heads/master@{#411039}

Powered by Google App Engine
This is Rietveld 408576698