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

Issue 893731 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 893850



Sign in to add a comment

DelegatedFrameHost::OnFirstSurfaceActivation should go away

Project Member Reported by fsam...@chromium.org, Oct 9

Issue description

We create a new surface when we start scrolling if top bar controls are showing on android. We should avoid this extra IPC in order to keep scrolls smooth on android.
 
Blocking: 893850
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 11

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

commit 5c236080c772cacebe27f3b83781adc6abe17066
Author: Saman Sami <samans@chromium.org>
Date: Thu Oct 11 19:15:07 2018

Don't use active device scale factor for copy requests

Grab the device scale factor from the client instead (which is more
correct anyway). This will pave the way for getting rid of
OnFirstSurfaceActivation.

Bug:  893731 
Change-Id: Ib274ade2fcb3295cc667bf7bcf389e9c83e66d47
Reviewed-on: https://chromium-review.googlesource.com/c/1274452
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598881}
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/delegated_frame_host_client_aura.cc
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/delegated_frame_host_client_aura.h
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/headless/lib/headless_web_contents_browsertest.cc
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/5c236080c772cacebe27f3b83781adc6abe17066/ui/android/delegated_frame_host_android.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 17

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

commit d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0
Author: Saman Sami <samans@chromium.org>
Date: Wed Oct 17 18:55:57 2018

Make frame eviction independent of OnFirstSurfaceActivation

Make EvictSurfaces take the primary surface instead of the last active
surface. Any surface that activates in Viz that is older than the
primary will get evicted.

Bug:  893731 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ibfaed7f4ddfaf73f502c79e4cdf753ec237ea588
Reviewed-on: https://chromium-review.googlesource.com/c/1277826
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600502}
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/frame_sinks/surface_references_unittest.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/components/viz/service/hit_test/hit_test_aggregator_unittest.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/delegated_frame_host_client_aura.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/delegated_frame_host_client_aura.h
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/ui/compositor/layer.cc
[modify] https://crrev.com/d8a5c1b6ae3734e1e9a4bf9607c59e7c1f34ddb0/ui/compositor/layer.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 19

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

commit 1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be
Author: Saman Sami <samans@chromium.org>
Date: Fri Oct 19 22:27:30 2018

Make OnFirstSurfaceActivation optional for top-level renderers

Only Mac needs to know about surface activations, so avoid sending
OnFirstSurfaceActivation on other platforms.

Bug:  893731 
Change-Id: Id5de1043d0c432964c1897af50440bf02277506f
Reviewed-on: https://chromium-review.googlesource.com/c/1286354
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601322}
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/1bbd1469a7cb1ba8a2ac07f5f84e2048c00207be/ui/android/delegated_frame_host_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 21

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

commit 4580e632c7a23892d94d4ec468cba6bfb437ee7b
Author: Saman Sami <samans@chromium.org>
Date: Sun Oct 21 03:46:59 2018

Fix blank page after switching out of simplified view

Sometimes EmbedSurface is called right before WasShown when the tab
is hidden. When frame evictor is notified of this new surface, it
might immediately evict it because it doesn't know the tab will be
visible soon. This will result in an evicted surface getting embedded
which will show blank. Do two things:
- Only call SwappedFrame when the tab is visible so don't have the
problem mention above. Generally there is no point in calling
SwappedFrame when it's invisible because even though a new id
is allocated, there will not be any CompositorFrames submitted to this
surface until the tab is shown, so there is no point in notifying the
frame evictor.
- Notify RHWVAndroid via WasEvicted when the surface is evicted so that
it can allocate a new id for the next time it's shown.

Bug: 896633, 893731 
Change-Id: I6fbc339f606d70e65ecc79b0f74bcd44133bd4fc
Reviewed-on: https://chromium-review.googlesource.com/c/1289953
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601408}
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/content/browser/renderer_host/delegated_frame_host_client_android.cc
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/content/browser/renderer_host/delegated_frame_host_client_android.h
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/4580e632c7a23892d94d4ec468cba6bfb437ee7b/ui/android/delegated_frame_host_android_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 22

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

commit d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666
Author: Saman Sami <samans@chromium.org>
Date: Mon Oct 22 17:25:31 2018

Fix frame eviction when surface sync is off

Since surface resurrection doens't work anymore, make sure the renderer
allocates a new id every time it's visible, as the old surface might
have been evicted.

Bug:  893731 
Change-Id: I44d715949f2ccc76a270feac9e6fb899a472cbbc
Reviewed-on: https://chromium-review.googlesource.com/c/1293765
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601630}
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/cc/mojo_embedder/async_layer_tree_frame_sink.cc
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/cc/mojo_embedder/async_layer_tree_frame_sink.h
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/cc/trees/layer_tree_frame_sink.h
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/components/viz/client/local_surface_id_provider.cc
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/components/viz/client/local_surface_id_provider.h
[modify] https://crrev.com/d19be0fc4b9ce3d8bb3c9fae486a411d9eb1d666/content/renderer/render_thread_impl.cc

Status: Fixed (was: Assigned)

Sign in to add a comment