New issue
Advanced search Search tips

Issue 860090 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: SessionRestoreTest.MemoryPressureLoadsNotAllTabs



Sign in to add a comment

SessionRestoreTest.MemoryPressureLoadsNotAllTabs is Flaky

Project Member Reported by Findit, Jul 4

Issue description

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 4

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

commit a49d91b6992b8d3357efeb32a2005d5b11ada26a
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Jul 04 00:30:07 2018

Revert "CC: Make LayerTreeHostImpl::OnMemoryPressure work for the desktop as well"

This reverts commit 18ba607af1f7ed07275e15824b430ae10976a9b8.

Reason for revert: Speculative revert for consistent failure on linux-chromeos-rel CQ builder.

Original change's description:
> CC: Make LayerTreeHostImpl::OnMemoryPressure work for the desktop as well
> 
> LayerTreeHostImpl::OnMemoryPressure has only worked for low-end devices.
> But it would be good if it works for all devices.
> 
> Bug: 839687
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I9f670ab08587ad4c5f626eb74fe5d7c9db256150
> Reviewed-on: https://chromium-review.googlesource.com/1107025
> Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
> Reviewed-by: Eric Karl <ericrk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572216}

TBR=ericrk@chromium.org,gyuyoung.kim@lge.com

Change-Id: I5e8d81f540311373a3d79854310ec59896494cae
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 839687, 860090
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1125459
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572405}
[modify] https://crrev.com/a49d91b6992b8d3357efeb32a2005d5b11ada26a/cc/trees/layer_tree_host_impl.cc

Cc: sky@chromium.org chrisha@chromium.org
 Issue 860111  has been merged into this issue.
Cc: ericrk@chromium.org gyuyoung...@lge.com
Labels: -Sheriff-Chromium
Let me remove Sheriff-Chromium label since the suspicious CL has been reverted.
Thank you for reverting it. I'm taking a look the CL again.
Owner: gyuyoung...@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 26

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

commit 069adb163044004e3f0676d97baac83a2867050c
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Thu Jul 26 04:09:58 2018

Re-land "CC: Make LayerTreeHostImpl::OnMemoryPressure work for the desktop as well"

This reverts commit a49d91b6992b8d3357efeb32a2005d5b11ada26a.

The original commit made SessionRestoreTest.MemoryPressureLoadsNotAllTabs crash.
According to my investigation, it was caused by calling SingleThreadProxy::OnCanDrawStateChanged
on the main thread. To avoid it, this CL makes LayerTreeHostImpl::OnMemoryPressure call
OnPurgeMemory on the impl thread through the Proxy.

Bug: 839687, 860090
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ifaef5bcb4029da8645f393d85313be4637aea994
Reviewed-on: https://chromium-review.googlesource.com/1130567
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578192}
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/test/fake_layer_tree_host_impl_client.h
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/proxy_impl.cc
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/proxy_impl.h
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/069adb163044004e3f0676d97baac83a2867050c/cc/trees/single_thread_proxy.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 31

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

commit b555ecd8a69a1cbd27c8a604a7a520d40970480b
Author: Ned Nguyen <nednguyen@google.com>
Date: Tue Jul 31 14:02:50 2018

Revert "Re-land "CC: Make LayerTreeHostImpl::OnMemoryPressure work for the desktop as well""

This reverts commit 069adb163044004e3f0676d97baac83a2867050c.

Reason for revert: crash rendering.mobile/microsoft_fireflies benchmark test on chromium.perf/android-nexus5x-perf 

BUG:869188

Original change's description:
> Re-land "CC: Make LayerTreeHostImpl::OnMemoryPressure work for the desktop as well"
> 
> This reverts commit a49d91b6992b8d3357efeb32a2005d5b11ada26a.
> 
> The original commit made SessionRestoreTest.MemoryPressureLoadsNotAllTabs crash.
> According to my investigation, it was caused by calling SingleThreadProxy::OnCanDrawStateChanged
> on the main thread. To avoid it, this CL makes LayerTreeHostImpl::OnMemoryPressure call
> OnPurgeMemory on the impl thread through the Proxy.
> 
> Bug: 839687, 860090
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: Ifaef5bcb4029da8645f393d85313be4637aea994
> Reviewed-on: https://chromium-review.googlesource.com/1130567
> Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
> Reviewed-by: enne <enne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578192}

TBR=enne@chromium.org,ericrk@chromium.org,gyuyoung.kim@lge.com

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

Bug: 839687, 860090
Change-Id: I0468da7dd808d545b4a6fe9d59f94d5427e40228
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1156564
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#579392}
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/test/fake_layer_tree_host_impl_client.h
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/proxy_impl.cc
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/proxy_impl.h
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/b555ecd8a69a1cbd27c8a604a7a520d40970480b/cc/trees/single_thread_proxy.h

Status: Assigned (was: Available)
Cc: -gyuyoung...@lge.com
Owner: gyuyoung...@lge.com
Once my commit was reverted to fix this issue. And I had fixed this issue on https://chromium-review.googlesource.com/c/chromium/src/+/1183012. I think this issue should not happen when this CL is landed. But the CL is still ongoing review because of another issue.
Cc: gyuyo...@igalia.com

Sign in to add a comment