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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1730023004: Revert of Decouple browser-specific GPU IPC messages from GPU service IPCs (Closed)

Created:
4 years, 10 months ago by shrike
Modified:
4 years, 10 months ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, creis+watch_chromium.org, extensions-reviews_chromium.org, nasko+codewatch_chromium.org, jam, darin-cc_chromium.org, chromium-apps-reviews_chromium.org, mkwst+moarreviews-renderer_chromium.org, piman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Decouple browser-specific GPU IPC messages from GPU service IPCs (patchset #19 id:360001 of https://codereview.chromium.org/1711533002/ ) Reason for revert: Regression: Win7 Tests (dbg)(1) browser_test failures https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29 Sheriff-o-matic suspecting Change #198012 in Build 46233 for cause of Win7 Tests (dbg)(1) browser_test failures: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/46233 In step 91. browser_tests failures: DevToolsPixelOutputTests.TestLatencyInfoInstrumentation DevToolsPixelOutputTests.TestScreenshotRecording Logs: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/46233/steps/browser_tests%20on%20Windows-7-SP1/logs/DevToolsPixelOutputTests.TestLatencyInfoInstrumentation https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/46233/steps/browser_tests%20on%20Windows-7-SP1/logs/DevToolsPixelOutputTests.TestScreenshotRecording [ RUN ] DevToolsPixelOutputTests.TestLatencyInfoInstrumentation [2840:5856:0224/124138:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Could not create D3D11 device. [2840:5856:0224/124138:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D11 failed with error EGL_NOT_INITIALIZED, trying next display type [2840:5856:0224/124139:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Renderer does not support PS 3.0.aborting! [2840:5856:0224/124139:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D9 failed with error EGL_NOT_INITIALIZED [2840:5856:0224/124139:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Could not create D3D11 device. [2840:5856:0224/124139:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D11 failed with error EGL_NOT_INITIALIZED, trying next display type HTTP server started on http://127.0.0.1:58543... sending server_data: {"host": "127.0.0.1", "port": 58543} (36 bytes) [2840:5856:0224/124140:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Renderer does not support PS 3.0.aborting! [2840:5856:0224/124140:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D9 failed with error EGL_NOT_INITIALIZED [2840:5856:0224/124140:ERROR:gl_surface_win.cc(69)] GLSurfaceEGL::InitializeOneOff failed. [2840:5856:0224/124140:ERROR:gpu_child_thread.cc(347)] Exiting GPU process due to errors during initialization [2840:5856:0224/124140:FATAL:gpu_child_thread.cc(484)] Check failed: gpu_channel_manager_. Backtrace: base::SysInfo::AmountOfVirtualMemory [0x100A8DB1+570092] base::SysInfo::AmountOfVirtualMemory [0x10100BCB+930054] content::ImageTransportFactory::ImageTransportFactory [0x176BA2F8+29889885] content::ImageTransportFactory::ImageTransportFactory [0x176B25D9+29857854] content::ImageTransportFactory::ImageTransportFactory [0x176B2118+29856637] content::ImageTransportFactory::ImageTransportFactory [0x176B23C6+29857323] content::ImageTransportFactory::ImageTransportFactory [0x176B141C+29853313] content::ImageTransportFactory::ImageTransportFactory [0x176BB492+29894391] IPC::Message::type [0x0FA2EA2B+166218] IPC::Message::type [0x0FA26F90+134831] IPC::Message::type [0x0FA26CE7+134150] IPC::Message::type [0x0FA2F6C2+169441] base::SysInfo::AmountOfVirtualMemory [0x1007C9BF+388858] base::SysInfo::AmountOfVirtualMemory [0x100AF147+595586] base::SysInfo::AmountOfVirtualMemory [0x1012E576+1116849] base::SysInfo::AmountOfVirtualMemory [0x1012BC74+1106351] base::SysInfo::AmountOfVirtualMemory [0x1012C43D+1108344] base::SysInfo::AmountOfVirtualMemory [0x10137E44+1155967] base::SysInfo::AmountOfVirtualMemory [0x10139E62+1164189] base::SysInfo::AmountOfVirtualMemory [0x10139DBC+1164023] base::SysInfo::AmountOfVirtualMemory [0x1012E1FF+1115962] base::SysInfo::AmountOfVirtualMemory [0x101C11C6+1718017] base::SysInfo::AmountOfVirtualMemory [0x1012E02D+1115496] content::ImageTransportFactory::ImageTransportFactory [0x176C0E20+29917317] content::ImageTransportFactory::ImageTransportFactory [0x15BEB179+1778654] content::ImageTransportFactory::ImageTransportFactory [0x15BEAF90+1778165] content::ImageTransportFactory::ImageTransportFactory [0x15BD4B70+1686997] content::LaunchTests [0x0849E65E+734] LaunchChromeTests [0x037F990D+125] main [0x01D5643E+110] __tmainCRTStartup [0x09B5DBE9+409] (f:\dd ctools\crt\crtw32\dllstuff\crtexe.c:626) mainCRTStartup [0x09B5DD2D+13] (f:\dd ctools\crt\crtw32\dllstuff\crtexe.c:466) BaseThreadInitThunk [0x767C337A+18] RtlInitializeExceptionChain [0x77D29882+99] RtlInitializeExceptionChain [0x77D29855+54] ================= [ RUN ] DevToolsPixelOutputTests.TestScreenshotRecording [2512:3528:0224/124026:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Could not create D3D11 device. [2512:3528:0224/124026:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D11 failed with error EGL_NOT_INITIALIZED, trying next display type [2512:3528:0224/124026:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Renderer does not support PS 3.0.aborting! [2512:3528:0224/124026:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D9 failed with error EGL_NOT_INITIALIZED [2512:3528:0224/124026:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Could not create D3D11 device. [2512:3528:0224/124026:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D11 failed with error EGL_NOT_INITIALIZED, trying next display type [2512:3528:0224/124026:ERROR:angle_platform_impl.cc(33)] ANGLE Display::initialize error 4: Renderer does not support PS 3.0.aborting! [2512:3528:0224/124026:ERROR:gl_surface_egl.cc(586)] eglInitialize D3D9 failed with error EGL_NOT_INITIALIZED [2512:3528:0224/124026:ERROR:gl_surface_win.cc(69)] GLSurfaceEGL::InitializeOneOff failed. [2512:3528:0224/124026:ERROR:gpu_child_thread.cc(347)] Exiting GPU process due to errors during initialization HTTP server started on http://127.0.0.1:51161... sending server_data: {"host": "127.0.0.1", "port": 51161} (36 bytes) [4708:1384:0224/124028:WARNING:message_queue.cc(38)] Leaking 2 ports in unreceived messages [2512:3528:0224/124026:FATAL:gpu_child_thread.cc(484)] Check failed: gpu_channel_manager_. Backtrace: base::SysInfo::AmountOfVirtualMemory [0x100A8DB1+570092] base::SysInfo::AmountOfVirtualMemory [0x10100BCB+930054] content::ImageTransportFactory::ImageTransportFactory [0x1778A2F8+29889885] content::ImageTransportFactory::ImageTransportFactory [0x177825D9+29857854] content::ImageTransportFactory::ImageTransportFactory [0x17782118+29856637] content::ImageTransportFactory::ImageTransportFactory [0x177823C6+29857323] content::ImageTransportFactory::ImageTransportFactory [0x1778141C+29853313] content::ImageTransportFactory::ImageTransportFactory [0x1778B492+29894391] IPC::Message::type [0x0FAFEA2B+166218] IPC::Message::type [0x0FAF6F90+134831] IPC::Message::type [0x0FAF6CE7+134150] IPC::Message::type [0x0FAFF6C2+169441] base::SysInfo::AmountOfVirtualMemory [0x1007C9BF+388858] base::SysInfo::AmountOfVirtualMemory [0x100AF147+595586] base::SysInfo::AmountOfVirtualMemory [0x1012E576+1116849] base::SysInfo::AmountOfVirtualMemory [0x1012BC74+1106351] base::SysInfo::AmountOfVirtualMemory [0x1012C43D+1108344] base::SysInfo::AmountOfVirtualMemory [0x10137E44+1155967] base::SysInfo::AmountOfVirtualMemory [0x10139E62+1164189] base::SysInfo::AmountOfVirtualMemory [0x10139DBC+1164023] base::SysInfo::AmountOfVirtualMemory [0x1012E1FF+1115962] base::SysInfo::AmountOfVirtualMemory [0x101C11C6+1718017] base::SysInfo::AmountOfVirtualMemory [0x1012E02D+1115496] content::ImageTransportFactory::ImageTransportFactory [0x17790E20+29917317] content::ImageTransportFactory::ImageTransportFactory [0x15CBB179+1778654] content::ImageTransportFactory::ImageTransportFactory [0x15CBAF90+1778165] content::ImageTransportFactory::ImageTransportFactory [0x15CA4B70+1686997] content::LaunchTests [0x0849E65E+734] LaunchChromeTests [0x037F990D+125] main [0x01D5643E+110] __tmainCRTStartup [0x09B5DBE9+409] (f:\dd ctools\crt\crtw32\dllstuff\crtexe.c:626) mainCRTStartup [0x09B5DD2D+13] (f:\dd ctools\crt\crtw32\dllstuff\crtexe.c:466) BaseThreadInitThunk [0x76C2337A+18] RtlInitializeExceptionChain [0x778692B2+99] RtlInitializeExceptionChain [0x77869285+54] [3768:3420:0224/124033:ERROR:singleton_hwnd.cc(34)] Cannot create windows on non-UI thread! [4708:1384:0224/124034:ERROR:CONSOLE(72)] "Uncaught TypeError: Cannot read property 'addExtensions' of undefined", source: (72) [4708:1384:0224/124035:INFO:CONSOLE(1)] "DONE:performActionsInPage.1", source: (1) c:uild\slave\win_builder__dbg_uild\src\chromerowser\devtools\devtools_sanity_browsertest.cc(124): error: Value of: result Actual: "[FAILED] Expected: 'true', but was 'false'" Expected: "[OK]" [4708:1384:0224/124038:WARNING:url_request_context_getter.cc(42)] URLRequestContextGetter leaking due to no owning thread. [ FAILED ] DevToolsPixelOutputTests.TestScreenshotRecording, where TypeParam = and GetParam() = (12746 ms) Original issue's description: > Decouple browser-specific GPU IPC messages from GPU service IPCs > > We would like Mus to reuse GPU service IPC messages. However, > browser-specific<=>gpu messages don't necessarily > make sense in Mus+Ash. This CL decouples browser messages > from Gpu channel messages in the following ways: > > 1. Browser channel messages are moved to a gpu_host_messages.h file. > > 2. Browser channel message handling is moved entirely to GpuChildThread. > > 3. GpuChannelManager is decoupled from GpuChildThread by making > GpuChildThread a GpuChannelManagerDelegate. > > The intent here is for Mus to implement an alternative > GpuChannelManagerDelegate. > > BUG=586374 > > Committed: https://crrev.com/6904b50eb105956b954279f9632f680251e41aa7 > Cr-Commit-Position: refs/heads/master@{#377341} TBR=sievers@chromium.org,piman@chromium.org,markdittmer@chromium.org,tsepez@chromium.org,fsamuel@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=586374 Committed: https://crrev.com/d45509a44318afe5643302e0c1edcb3760d8b2fe Cr-Commit-Position: refs/heads/master@{#377431}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+569 lines, -1007 lines) Patch
M content/browser/gpu/gpu_data_manager_impl_private.cc View 5 chunks +13 lines, -11 lines 0 comments Download
M content/browser/gpu/gpu_process_host.cc View 5 chunks +12 lines, -8 lines 0 comments Download
M content/browser/gpu/gpu_process_host_ui_shim.h View 3 chunks +5 lines, -4 lines 0 comments Download
M content/browser/gpu/gpu_process_host_ui_shim.cc View 4 chunks +7 lines, -8 lines 0 comments Download
M content/browser/renderer_host/gpu_message_filter.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/common/content_message_generator.h View 1 chunk +0 lines, -1 line 0 comments Download
D content/common/gpu/accelerated_surface_buffers_swapped_params_mac.h View 1 chunk +0 lines, -28 lines 0 comments Download
D content/common/gpu/accelerated_surface_buffers_swapped_params_mac.cc View 1 chunk +0 lines, -14 lines 0 comments Download
D content/common/gpu/buffer_presented_params_mac.h View 1 chunk +0 lines, -23 lines 0 comments Download
D content/common/gpu/buffer_presented_params_mac.cc View 1 chunk +0 lines, -13 lines 0 comments Download
M content/common/gpu/child_window_surface_win.cc View 2 chunks +2 lines, -3 lines 0 comments Download
D content/common/gpu/establish_channel_params.h View 1 chunk +0 lines, -27 lines 0 comments Download
D content/common/gpu/establish_channel_params.cc View 1 chunk +0 lines, -18 lines 0 comments Download
M content/common/gpu/gpu_channel.cc View 3 chunks +6 lines, -4 lines 0 comments Download
M content/common/gpu/gpu_channel_manager.h View 4 chunks +44 lines, -43 lines 0 comments Download
M content/common/gpu/gpu_channel_manager.cc View 12 chunks +66 lines, -52 lines 0 comments Download
D content/common/gpu/gpu_channel_manager_delegate.h View 1 chunk +0 lines, -79 lines 0 comments Download
M content/common/gpu/gpu_channel_manager_unittest.cc View 3 chunks +23 lines, -14 lines 0 comments Download
M content/common/gpu/gpu_channel_test_common.h View 6 chunks +12 lines, -49 lines 0 comments Download
M content/common/gpu/gpu_channel_test_common.cc View 4 chunks +22 lines, -64 lines 0 comments Download
M content/common/gpu/gpu_channel_unittest.cc View 3 chunks +7 lines, -7 lines 0 comments Download
M content/common/gpu/gpu_command_buffer_stub.cc View 4 chunks +10 lines, -7 lines 0 comments Download
D content/common/gpu/gpu_host_messages.h View 1 chunk +0 lines, -276 lines 0 comments Download
M content/common/gpu/gpu_memory_manager.h View 1 chunk +1 line, -1 line 0 comments Download
M content/common/gpu/gpu_memory_manager.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M content/common/gpu/gpu_messages.h View 4 chunks +243 lines, -2 lines 0 comments Download
M content/common/gpu/image_transport_surface.h View 6 chunks +21 lines, -10 lines 0 comments Download
M content/common/gpu/image_transport_surface.cc View 8 chunks +30 lines, -21 lines 0 comments Download
M content/common/gpu/image_transport_surface_overlay_mac.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/common/gpu/image_transport_surface_overlay_mac.mm View 3 chunks +4 lines, -5 lines 0 comments Download
M content/content_common.gypi View 2 chunks +0 lines, -8 lines 0 comments Download
M content/gpu/gpu_child_thread.h View 4 chunks +1 line, -44 lines 0 comments Download
M content/gpu/gpu_child_thread.cc View 7 chunks +22 lines, -147 lines 0 comments Download
M content/gpu/gpu_main.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M content/renderer/gpu/gpu_benchmarking_extension.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/render_thread_impl.cc View 2 chunks +5 lines, -3 lines 0 comments Download
M ipc/ipc_message_start.h View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 9 (2 generated)
shrike
Created Revert of Decouple browser-specific GPU IPC messages from GPU service IPCs
4 years, 10 months ago (2016-02-25 00:01:04 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1730023004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1730023004/1
4 years, 10 months ago (2016-02-25 00:03:38 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-02-25 00:12:52 UTC) #4
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/d45509a44318afe5643302e0c1edcb3760d8b2fe Cr-Commit-Position: refs/heads/master@{#377431}
4 years, 10 months ago (2016-02-25 00:14:07 UTC) #6
no sievers
Hehe, the GpuProcessHost factory blocks on Init() which sends a sync Initialize() IPC. However OnInitialize() ...
4 years, 10 months ago (2016-02-25 02:01:26 UTC) #7
Fady Samuel
On 2016/02/25 02:01:26, sievers wrote: > Hehe, the GpuProcessHost factory blocks on Init() which sends ...
4 years, 10 months ago (2016-02-25 02:25:18 UTC) #8
no sievers
4 years, 10 months ago (2016-02-25 19:56:48 UTC) #9
Message was sent while issue was closed. 
On 2016/02/25 02:25:18, Fady Samuel wrote:
> On 2016/02/25 02:01:26, sievers wrote:
> > Hehe, the GpuProcessHost factory blocks on Init() which sends a sync
> > Initialize() IPC. However OnInitialize() does not return a value indicating
> > success, so currently the renderer (and potentially others) will still be
able
> > to end up with messages sent out to the GPU process that never initialized
and
> > will wait for the GPU process to terminate itself instead which will then
> later
> > deal with the outstanding replies.
> > 
> > So I'd do the if (gpu_channel_manager_) check everywhere to maintain the
> > behavior. But maybe it's worth revisiting since the init IPC is already sync
> > anyways.
> 
> Actually GpuMsg_Initialize is not sync.
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...
> 
> GpuCommandBufferMsg_Initialize is sync.
> 
> We could consider making GpuMsg_Initialize sync but this seems like a step in
> the wrong direction? I'll restore the logic to what it was previously and file
a
> bug.

yea, sorry, i wasn't thinking right there. either way, just checking
|gpu_channel_manager_| should be ok.

Powered by Google App Engine
This is Rietveld 408576698