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

Issue 849478 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

viz/mac: Browser not usable with viz

Project Member Reported by ccameron@chromium.org, Jun 5 2018

Issue description

This is a recent regression:
- the browser hangs almost immediately on launch in release builds
- CHECK hit in RootCompositorFrameSinkImpl::Resize due to a zero size
- CHECK hit in ScopedGpuMemoryBuffer for caps.texture_storage_image
 
Cc: kylec...@chromium.org
RootCompositorFrameSinkImpl::Resize DCHECK was reverted in https://crrev.com/c/1085971.
Cc: sunn...@chromium.org
I found the problem here -- my patch https://chromium-review.googlesource.com/c/chromium/src/+/1076531 is completely wrong.

I mis-read "gpu_memory_manager" as "gpu_memory_buffer_manager". That this happened to have the desired effect in tests was near-magic.

But now I have to somehow deal with the fact if you support GPU raster and export texture_storage_image, a bunch of tests will try to use it. Tests that use cc::TestInProcessContextProvider, which CHECK when you try to CreateAnonymousImage.

Somehow cc::TestInProcessContextProvider needs to reach out into either FeatureInfo or GLES2CmdDecoder and tell it "you can't actually use texture_storage_image". I don't see any obvious path between the two of them.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 7 2018

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

commit a494fc167ef21cb67ae68d59dd9e84eff507e06a
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Jun 07 01:52:21 2018

Fix CHROMIUM_texture_storage_image detection

Whether or not CHROMIUM_texture_storage_image is supported depends
entirely on the underlying ImageFactory.

Add a function to query if the ImageFactory has the capability to create
anonymous images, and use this to enable relevant parts of FeatureInfo.
Remove the ifdef that was being used to determine whether or not to
enable CHROMIUM_texture_storage_image, because that was just attempting
to match the capabilities of the ImageFactory (and sometimes failing).

I screwed this up in the previous patch because I was checking for
gpu_memory_manager, thinking that it was gpu_memory_buffer_manager.
As it happens CHROMIUM_gpu_memory_manager is unused, so just remove
it.

Bug:  845213 ,  849478 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I9d97749863c61954151e9f06dc23949613c2181f
Reviewed-on: https://chromium-review.googlesource.com/1088313
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565140}
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/cc/test/test_image_factory.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/cc/test/test_image_factory.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/service/feature_info.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/service/feature_info.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/service/image_factory.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/a494fc167ef21cb67ae68d59dd9e84eff507e06a/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 7 2018

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

commit fb932c42c8e9e5d9c4f7177bab975938b820789d
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Jun 07 04:23:07 2018

Revert "Fix CHROMIUM_texture_storage_image detection"

This reverts commit a494fc167ef21cb67ae68d59dd9e84eff507e06a.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified this CL at revision 565140 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2E0OTRmYzE2N2VmMjFjYjY3YWU2OGQ1OWRkOWU4NGVmZjUwN2UwNmEM

Original change's description:
> Fix CHROMIUM_texture_storage_image detection
> 
> Whether or not CHROMIUM_texture_storage_image is supported depends
> entirely on the underlying ImageFactory.
> 
> Add a function to query if the ImageFactory has the capability to create
> anonymous images, and use this to enable relevant parts of FeatureInfo.
> Remove the ifdef that was being used to determine whether or not to
> enable CHROMIUM_texture_storage_image, because that was just attempting
> to match the capabilities of the ImageFactory (and sometimes failing).
> 
> I screwed this up in the previous patch because I was checking for
> gpu_memory_manager, thinking that it was gpu_memory_buffer_manager.
> As it happens CHROMIUM_gpu_memory_manager is unused, so just remove
> it.
> 
> Bug:  845213 ,  849478 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I9d97749863c61954151e9f06dc23949613c2181f
> Reviewed-on: https://chromium-review.googlesource.com/1088313
> Reviewed-by: ccameron <ccameron@chromium.org>
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Commit-Queue: ccameron <ccameron@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#565140}

TBR=ccameron@chromium.org,sunnyps@chromium.org

Change-Id: I09717f0aa60f911ee339c85c99d8fe9c4d325dd0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  845213 ,  849478 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1090450
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565177}
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/cc/test/test_image_factory.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/cc/test/test_image_factory.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/service/feature_info.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/service/feature_info.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/service/image_factory.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/fb932c42c8e9e5d9c4f7177bab975938b820789d/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 7 2018

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

commit f9656207a24252cb5486d98a700dbc72e5430fee
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Jun 07 07:04:57 2018

Fix CHROMIUM_texture_storage_image detection

Whether or not CHROMIUM_texture_storage_image is supported depends
entirely on the underlying ImageFactory.

Add a function to query if the ImageFactory has the capability to create
anonymous images, and use this to enable relevant parts of FeatureInfo.
Remove the ifdef that was being used to determine whether or not to
enable CHROMIUM_texture_storage_image, because that was just attempting
to match the capabilities of the ImageFactory (and sometimes failing).

I screwed this up in the previous patch because I was checking for
gpu_memory_manager, thinking that it was gpu_memory_buffer_manager. As
it happens CHROMIUM_gpu_memory_manager is unused, so just remove it.

Re-landed due to mid-air collision.

TBR=sunnyps

Bug:  845213 ,  849478 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I35ca390cccd05416799c0afa549694f2ac45f14d
Reviewed-on: https://chromium-review.googlesource.com/1090324
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565204}
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/cc/test/test_image_factory.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/cc/test/test_image_factory.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/feature_info.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/feature_info.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/image_factory.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/service/raster_decoder.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/f9656207a24252cb5486d98a700dbc72e5430fee/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.h

Status: Fixed (was: Assigned)

Sign in to add a comment