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

Issue 870456 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 857575



Sign in to add a comment

Don't update the fallback in DelegatedFrameHost::OnFirstSurfaceActivation

Project Member Reported by samans@chromium.org, Aug 2

Issue description

Once fallbacks are optional, we shouldn't update the fallback in OnFirstSurfaceActivation in DelegatedFrameHost. Updating the fallback causes an unnecessary commit and draw.
 
Components: Internals>Services>Viz
Owner: samans@chromium.org
Status: Assigned (was: Available)
 Issue 876733  has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 24

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

commit f03a9dbadf9f69ee774123ef50ca82bef8e8731d
Author: Saman Sami <samans@chromium.org>
Date: Mon Sep 24 18:05:07 2018

Use primary surface in TakeFallbackContentFrom when there is no fallback

Also when a fallback exists but has a different FrameSinkId or embed token
than the primary, don't use the fallback because the resulting range in
the new view will not cover any surface with the FrameSinkId or embed
token of the old view's primary.

Bug:  857575 , 870456 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia4736f354160dfd510793bf35ae4292c276bf115
Reviewed-on: https://chromium-review.googlesource.com/1236456
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593598}
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/local_surface_id.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/local_surface_id.h
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/surface_id.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/surface_id.h
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/ui/android/delegated_frame_host_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 26

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

commit 5dea8c2f26f49b400fa2c0a507f918a5fe16b622
Author: Saman Sami <samans@chromium.org>
Date: Wed Sep 26 17:55:58 2018

Don't update the surface range at every activation

Updating the fallback lowerbound is not necessary anymore and will just
waste time by causing an unnecessary commit and draw.

Bug:  870456 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Icd553c179c5a2eb98c1608524d9c1caf42a27835
Reviewed-on: https://chromium-review.googlesource.com/1226366
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@{#594380}
[modify] https://crrev.com/5dea8c2f26f49b400fa2c0a507f918a5fe16b622/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/5dea8c2f26f49b400fa2c0a507f918a5fe16b622/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/5dea8c2f26f49b400fa2c0a507f918a5fe16b622/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/5dea8c2f26f49b400fa2c0a507f918a5fe16b622/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/5dea8c2f26f49b400fa2c0a507f918a5fe16b622/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/5dea8c2f26f49b400fa2c0a507f918a5fe16b622/ui/android/delegated_frame_host_android.cc

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 28

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

commit 2112dbffdd286898c8777fde20ec5451a04fcdea
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Sep 28 03:39:36 2018

Revert "Don't update the surface range at every activation"

This reverts commit 5dea8c2f26f49b400fa2c0a507f918a5fe16b622.

Reason for revert: Causes TabCaptureApiPixelTest.OffscreenTabEvilTests is flaky


Original change's description:
> Don't update the surface range at every activation
>
> Updating the fallback lowerbound is not necessary anymore and will just
> waste time by causing an unnecessary commit and draw.
>
> Bug:  870456 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: Icd553c179c5a2eb98c1608524d9c1caf42a27835
> Reviewed-on: https://chromium-review.googlesource.com/1226366
> 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@{#594380}

TBR=fsamuel@chromium.org,khushalsagar@chromium.org,samans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  870456 ,  890084 
Change-Id: Ia3cb2aa4b405f9e9f9c2a282ae0ec7fdc62e8682
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1249284
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594973}
[modify] https://crrev.com/2112dbffdd286898c8777fde20ec5451a04fcdea/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/2112dbffdd286898c8777fde20ec5451a04fcdea/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/2112dbffdd286898c8777fde20ec5451a04fcdea/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/2112dbffdd286898c8777fde20ec5451a04fcdea/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/2112dbffdd286898c8777fde20ec5451a04fcdea/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/2112dbffdd286898c8777fde20ec5451a04fcdea/ui/android/delegated_frame_host_android.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 4

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

commit af5e170faab6bcf9d0897f65b047da5edc9ec955
Author: Saman Sami <samans@chromium.org>
Date: Thu Oct 04 20:22:37 2018

Reland "Don't update the surface range at every activation"

Relands crrev.com/5dea8c2 but also makes SurfaceAggregator take into
account offscreen clients in damage_ranges_.

Updating the fallback lowerbound is not necessary anymore and will just
waste time by causing an unnecessary commit and draw.

TBR=khushalsagar@chromium.org

Bug:  870456 , 890084 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I8eb13587511f072583f8d746e41b93900fa5d695
Reviewed-on: https://chromium-review.googlesource.com/c/1259607
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596828}
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/af5e170faab6bcf9d0897f65b047da5edc9ec955/ui/android/delegated_frame_host_android.cc

Sign in to add a comment