New issue
Advanced search Search tips

Issue 859068 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 787097



Sign in to add a comment

VizProcessContextProvider should handle lost context

Project Member Reported by kylec...@chromium.org, Jun 29 2018

Issue description

VizProcessContextProvider AddObserver() and RemoveObserver() both do nothing. VizProcessContextProvider should have an observer list and should register a context lost callback with gles2::GLES2Implementation so it can notify observers.

We also need RootCompositorFrameSinkImpl (or something similar) to register itself as a ContextLostObserver and handle lost display compositor contexts.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 11

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

commit 589b466c7f8442af8d38c0515216b9683d33cc9d
Author: kylechar <kylechar@chromium.org>
Date: Wed Jul 11 19:10:41 2018

Cleanup ContextType enum and strings.

This CL cleans up the ContextType enum. The enum values correspond to a
UMA histogram and string for logging. Make the enum name, UMA histogram
name and string the same for clarity. The UMA names aren't perfect, but
they are the one thing we can't easily change.

A few other small changes at the same time:
1. Remove //services/ui/public/cpp/gpu:internal, it was used to restrict
   access to a ClientGpuMemoryBufferManager but the same thing can be
   accomplished by not making the header public.
2. Remove UI_COMPOSITOR context type. Originally OOP-D had a context per
   ui::Compositor. This is no longer the case and there is just a single
   shared context for the main thread. Deprecate the UMA histogram
   associated with UI_COMPOSITOR.

Bug:  859068 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I31b6f6f460b99eeefd231c9f334d468e58659b51
Reviewed-on: https://chromium-review.googlesource.com/1129102
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574277}
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/test/gpu_browsertest_helpers.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/content/test/layouttest_support.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/services/ui/public/cpp/gpu/BUILD.gn
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/services/ui/public/cpp/gpu/command_buffer_metrics.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/services/ui/public/cpp/gpu/command_buffer_metrics.h
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/services/ui/public/cpp/gpu/gpu.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/services/video_capture/device_factory_provider_impl.cc
[modify] https://crrev.com/589b466c7f8442af8d38c0515216b9683d33cc9d/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13

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

commit 899d48111bce2be643d6da2fcd236ad1b33d53da
Author: kylechar <kylechar@chromium.org>
Date: Fri Jul 13 19:25:50 2018

Handle context loss in VizProcessContextProvider.

Register a context loss callback with the GLES2Implementation to find
out about lost context. Implement AddObserver() and RemoveObserver() for
VizProcessContextProvider to notify the observers on context loss.

viz::Display is already registered as the context loss observer for the
ContextProvider. It gets notified and calls out to DisplayClient.
Implement a handler for DisplayOutputSurfaceLost() so the root
CompositorFrameSink shuts down the display on context loss. The client
will see a connection error and reinitialize the display.

Also make VizProcessContextProvider log UMA on context loss. This
requires moving ContextLossReason enum somewhere visible to
VizProcessContextProvider and adding a new histogram for
GPU.ContextLost.DisplayCompositor.

Bug:  859068 
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
Change-Id: I4c1fb6f00a893003e9b65dde1a7c5802a8f27692
Reviewed-on: https://chromium-review.googlesource.com/1126482
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575016}
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/components/viz/common/BUILD.gn
[add] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/components/viz/common/gpu/context_lost_reason.cc
[add] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/components/viz/common/gpu/context_lost_reason.h
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/components/viz/service/display_embedder/viz_process_context_provider.cc
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/components/viz/service/display_embedder/viz_process_context_provider.h
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/gpu/ipc/gl_in_process_context.cc
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/gpu/ipc/gl_in_process_context.h
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/services/ui/public/cpp/gpu/command_buffer_metrics.cc
[modify] https://crrev.com/899d48111bce2be643d6da2fcd236ad1b33d53da/tools/metrics/histograms/histograms.xml

Status: Assigned (was: Available)
Status: Fixed (was: Assigned)

Sign in to add a comment