BackForwardCache: Add a GPU test for the back-forward cache [chromium/src : master]
Hajime Hoshi (Gerrit)
Attention is currently required from: Kenneth Russell.
Hajime Hoshi would like Kenneth Russell to review this change.
BackForwardCache: Add a GPU test for the back-forward cache
This test adds a new GPU test that confirms a page with WebGL is put in
the back-forward cache when navigating, and is restored when navigating
back.
Bug: 1146922
Change-Id: Ic153e74b4b251df74c5b0a99ed6f1bc40863eb89
---
M content/test/gpu/gpu_tests/context_lost_integration_test.py
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/content/test/gpu/gpu_tests/context_lost_integration_test.py b/content/test/gpu/gpu_tests/context_lost_integration_test.py
index dc18720..225b491 100644
--- a/content/test/gpu/gpu_tests/context_lost_integration_test.py
+++ b/content/test/gpu/gpu_tests/context_lost_integration_test.py
@@ -140,7 +140,8 @@
'webgl2-multisampling-high-power-switch-loses-context.html'),
('ContextLost_MacWebGLPreserveDBHighPowerSwitchLosesContext',
'webgl2-preserve-db-high-power-switch-loses-context.html'),
- ('GpuCrash_InfoForHardwareGpu', 'simple.html'))
+ ('GpuCrash_InfoForHardwareGpu',
+ 'simple.html'), ('BackForwardCache', 'webgl.html'))
for t in tests:
yield (t[0], t[1], ('_' + t[0]))
@@ -501,6 +502,25 @@
'but got %s' % webgl_status_for_hardware_gpu)
self._RestartBrowser('must restart after tests that kill the GPU process')
+ def _BackForwardCache(self, test_path):
+ self.RestartBrowserIfNecessaryWithArgs(
+ ['--enable-features=BackForwardCache:enable_same_site/true'])
+
+ # Navigate to a page with WebGL.
+ tab = self.tab
+ tab.Navigate(self.UrlOfStaticFilePath(test_path))
+ tab.EvaluateJavaScript('window.testValueThatMustBeKept = 1;')
+
+ # Navigate to another page.
+ self._NavigateAndWaitForLoad('simple.html')
+
+ # Go back to the orignal page and confirm the page state is kept.
+ tab.EvaluateJavaScript('window.window.history.go(-1);')
+
+ if tab.EvaluateJavaScript('window.testValueThatMustBeKept') != 1:
+ self.fail(
+ 'Page with WebGL should have been puts into the back-forward cache')
+
@classmethod
def GetPlatformTags(cls, browser):
tags = super(ContextLostIntegrationTest, cls).GetPlatformTags(browser)
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Kenneth Russell.
1 comment:
Patchset:
This is a follow-up CL for a CL enabling WebGL for BFcache.
PTAL
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Kenneth Russell (Gerrit)
Attention is currently required from: Hajime Hoshi.
Patch set 1:Code-Review +1
3 comments:
Patchset:
lgtm, thanks for adding the test!
Please rebase and double-check that the trybot failures are unrelated to this test (they look unrelated to me).
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #1, Line 517:
orignal"orignal" is a possible misspelling of "original".
Please fix.
puts -> put
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
1 comment:
Patchset:
lgtm, thanks for adding the test! […]
Hmm, looks like the test failed with the new test:
https://ci.chromium.org/ui/p/chromium/builders/try/android-marshmallow-arm64-rel/749891/overview
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
3 comments:
Patchset:
Thank you!
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #1, Line 517:
orignal
> "orignal" is a possible misspelling of "original". […]
Done
puts -> put
Done
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Patch set 2:Commit-Queue +1
Kenneth Russell (Gerrit)
Attention is currently required from: Hajime Hoshi.
Hajime Hoshi has uploaded this change for review.
BackForwardCache: Add a GPU test for the back-forward cache
This test adds a new GPU test that confirms a page with WebGL is put in
the back-forward cache when navigating, and is restored when navigating
back.
Bug: 1146922
Change-Id: Ic153e74b4b251df74c5b0a99ed6f1bc40863eb89
---
M content/test/gpu/gpu_tests/context_lost_integration_test.py
1 file changed, 23 insertions(+), 3 deletions(-)
Kenneth Russell (Gerrit)
Attention is currently required from: Hajime Hoshi.
1 comment:
Patchset:
The logs are very verbose (cc'ing bsheedy in case there's anything that can be done to improve this):
https://chromium-swarm.appspot.com/task?id=510e8b771990b010#BackForwardCachebut overall it looks like there's an assertion failure probably from trying to manually turn on the back/forward cache:
4 libchrome.so!base::debug::BreakDebugger() [debugger_posix.cc : 311 + 0x0]
fp = 0x0000007fd7890ab0 lr = 0x0000007f7c5415e4
sp = 0x0000007fd7890440 pc = 0x0000007f7c5e3544
Found by: previous frame's frame pointer
5 libchrome.so!logging::LogMessage::~LogMessage() [logging.cc : 558 + 0x0]
x19 = 0x0000000000000030 fp = 0x0000007fd7890b10
sp = 0x0000007fd7890ad0 pc = 0x0000007f7c541964
Found by: call frame info
6 libchrome.so!base::FeatureList::RegisterFieldTrialOverride(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::FeatureList::OverrideState, base::FieldTrial*) [feature_list.cc : 308 + 0x4]
x19 = 0x0000007f93b8ce00 fp = 0x0000007fd7890b30
sp = 0x0000007fd7890b30 pc = 0x0000007f7c52e388
Found by: call frame info
7 libchrome.so!variations::AssociateParamsFromFieldTrialConfig(variations::FieldTrialTestingConfig const&, base::RepeatingCallback<void (unsigned int, std::__1::basic_string<char16_t, std::__1::char_traits<char16_t>, std::__1::allocator<char16_t> > const&)> const&, variations::Study_Platform, base::FeatureList*) [field_trial_util.cc : 117 + 0x10]
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
1 comment:
Patchset:
The logs are very verbose (cc'ing bsheedy in case there's anything that can be done to improve this) […]
Thanks. Do you know hot to test this with swarm (./tools/mb/mb)?
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Kenneth Russell (Gerrit)
Attention is currently required from: Hajime Hoshi.
1 comment:
Patchset:
Thanks. Do you know hot to test this with swarm (. […]
Please see https://chromium.googlesource.com/chromium/src/+/master/docs/gpu/gpu_testing.md#Running-Locally-Built-Binaries-on-the-GPU-Bots (you can find this by Googling for "chromium gpu testing") and ask on the internal chrome-gpu-infra alias if you run into problems. Thanks.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Brian Sheedy (Gerrit)
Attention is currently required from: Hajime Hoshi.
2 comments:
Patchset:
Thanks. Do you know hot to test this with swarm (. […]
I believe the following should work:
1. Use the following for your GN args:
dcheck_always_on = true
disable_android_lint = true
enable_resource_allowlist_generation = true
ffmpeg_branding = "Chrome"
is_component_build = false
is_debug = false
proprietary_codecs = true
symbol_level = 1
system_webview_package_name = "com.google.android.webview"
target_cpu = "arm64"
target_os = "android"
use_errorprone_java_compiler = false
use_goma = true
use_jacoco_coverage = true
use_static_angle = true2. tools/mb/mb.py run -s --no-default-dimensions -d pool chromium.tests -d device_type bullhead -d device_os MMB29Q out/Release telemetry_gpu_integration_test -- context_lost --show-stdout --browser android-chromium --passthrough -v --extra-browser-args "--enable-logging=stderr --js-flags=--expose-gc --use-cmd-decoder=validating" --isolated-script-test-output '${ISOLATED_OUTDIR}/output.json' --isolated-script-test-perf-output '${ISOLATED_OUTDIR}/perftest-output.json'
Patchset:
It would help if I actually published the comment I wrote this morning...
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Hajime Hoshi has uploaded this change for review.
BackForwardCache: Add a GPU test for the back-forward cache
This test adds a new GPU test that confirms a page with WebGL is put in
the back-forward cache when navigating, and is restored when navigating
back.
Bug: 1146922
Change-Id: Ic153e74b4b251df74c5b0a99ed6f1bc40863eb89
---
M content/test/gpu/gpu_tests/context_lost_integration_test.py
1 file changed, 23 insertions(+), 3 deletions(-)
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
1 comment:
Patchset:
CC +fergal
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy.
1 comment:
Patchset:
I believe the following should work: […]
Thanks, as Brian said, specifying telemetry_gpu_integration_test at tools/mb/mb.py worked well!
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy.
Hajime Hoshi has uploaded this change for review.
BackForwardCache: Add a GPU test for the back-forward cache
This test adds a new GPU test that confirms a page with WebGL is put in
the back-forward cache when navigating, and is restored when navigating
back.
Bug: 1146922
Change-Id: Ic153e74b4b251df74c5b0a99ed6f1bc40863eb89
---
M content/test/gpu/gpu_tests/context_lost_integration_test.py
1 file changed, 23 insertions(+), 3 deletions(-)
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])My understanding is that this doesn't work on Android, where same-site navigations are partially enabled, and this setting is d-checked at base::FeatureList::RegisterFieldTrialOverride. (I have confirmed that any DCHECK failure seems just a crash on the try bots.)
I changed `self._NavigateAndWaitForLoad('simple.html')` to `self._NavigateAndWaitForLoad('https://example.com')` but navigating back by go(-1) did not work.
1. Is it possible to navigate a different domain page in this test, and navigate back?
2. If not, I think we should wait for enabling BFcache for same-site navigations on all the environment.CC +rakina for insights of the current same-site navigations situation.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Kenneth Russell (Gerrit)
Attention is currently required from: Brian Sheedy, Hajime Hoshi.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
My understanding is that this doesn't work on Android, where same-site navigations are partially ena […]
It's not simple to navigate to a different domain for another page in the same test. +bsheedy in case he has any comments on the feasibility of this in Telemetry and the HTTP server it starts.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Brian Sheedy (Gerrit)
Attention is currently required from: Hajime Hoshi, Kenneth Russell.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
It's not simple to navigate to a different domain for another page in the same test. […]
I don't think there's a way to have different domains with the default HTTP server, as everything is accessed through 127.0.0.1. The closest you can probably get is with chrome:// URLs, but those are generally pretty special so I don't know if they'll get you the behavior you want.
It *is* possible to have different domains in Telemetry tests, but the only way I'm aware of is to use WebPageReplay and WPR archives for different domains. AFAIK the GPU tests don't currently have support for WPR since there hasn't been a use case before. Not sure off-hand how much work it would be to add that, either.
If Android is currently the only issue, you could add the test as-is and skip it via the expectation files until the root cause is fixed https://source.chromium.org/chromium/chromium/src/+/master:content/test/gpu/gpu_tests/test_expectations/context_lost_expectations.txt
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Fergal Daly (Gerrit)
Attention is currently required from: Brian Sheedy, Hajime Hoshi, Kenneth Russell.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
I don't think there's a way to have different domains with the default HTTP server, as everything is […]
I should be possible to navigate away to about:blank and navigate back as cross-site, I think.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy, Kenneth Russell.
Patch set 4:Commit-Queue +1
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Kenneth Russell.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
I should be possible to navigate away to about:blank and navigate back as cross-site, I think.
|self._NavigateAndWaitForLoad('about:blank')| was stuck, and |tab.Navigate('about:blank')| didn't pass the test (probably the page went back before loading was finished. Hmm?
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Kenneth Russell (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi.
Patch set 4:Code-Review +1
2 comments:
Patchset:
Hmm, zhanliang@ doesn't have a Chromium account?
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
I don't think there's a way to have different domains with the default HTTP server, as everything is […]
Argh. I composed this on Jan 21 but forgot to click "Reply":
-----
The Maps test used to use a WPR archive, but no longer does.src/content/test/gpu/gpu_tests/maps_integration_test.py
src/tools/perf/page_sets/rendering/maps.pyIt was replaced in https://chromium-review.googlesource.com/828541/ .
StartWPRServer still exists in the superclass:
https://source.chromium.org/chromium/chromium/src/+/master:third_party/catapult/telemetry/telemetry/testing/serially_executed_browser_test_case.py;l=109?q=StartWPRServerTheoretically it should be possible to record a WPR archive containing two different domains.
It's also OK in my opinion to skip the test just on Android for the moment, as long as there's a bug filed about enabling it.
-----Sorry, not sure why that's not working. Also not 100% sure who the Telemetry owners are right now. zhanliang@ maybe. Will CC:.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
John Chen (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi, Kenneth Russell.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
Argh. I composed this on Jan 21 but forgot to click "Reply": […]
|self._NavigateAndWaitForLoad| only works with specially created test pages, and doesn't work with |about:blank| or any "normal" pages. It's defined at [1] to wait for a specific JavaScript variable to be set, and thus only works with web pages that has code to set that variable. For |tab.Navigate('about:blank')|, I agree with Hajime's theory.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Kenneth Russell (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi, John Chen.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
|self. […]
Thanks John for your input. Could you please help Hajime figure out how to get past this issue? We have the ability to call internal APIs like GpuBenchmarkingExtension, so we could do something like mark the next navigation as being cross-origin. Or we could potentially spin up another web server on another port. Other suggestions?
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
John Chen (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi, Kenneth Russell.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
Thanks John for your input. […]
One potential fix is to call |tab.Navigate('about:blank')| followed by |tab.WaitForNavigate()|. This should allow waiting for the navigation to about:blank to complete before continuing.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Kenneth Russell, John Chen.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
One potential fix is to call |tab.Navigate('about:blank')| followed by |tab.WaitForNavigate()|. […]
Thank you! Interestingly, tab.WaitForNavigate() got stuck forever...
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
John Chen (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi, Kenneth Russell.
1 comment:
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
Thank you! Interestingly, tab.WaitForNavigate() got stuck forever...
Interesting. I think about:blank is a special page, and it might not emit all the events needed by tab.WaitForNavigate. Since navigate to about:blank should be quick, I think using a simple time.sleep(1) should suffice. If you want to be more certain, you can wait until JavaScript location.href == 'about:blank' && document.readyState == 'complete'.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Kenneth Russell.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
John Chen (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi, Kenneth Russell.
4 comments:
Patchset:
I have added a few suggestions below, and hopefully they will help resolving the test failures. Please let me know if you need any additional help.
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #4, Line 591:
tab.NavigateI think this should be self._NavigateAndWaitForLoad instead. tab.Navigate can return before the navigation is complete, and then |window.testValueThatMustBeKept = 1| can be set on the wrong page.
Patch Set #4, Line 598:
window.window.I guess the repetition was unintentional?
At this point, the back navigation might still be in progress, so the JavaScript might be run on the wrong page. I'd suggest using JavaScript location.href to get the URL, and wait until it has the expected value.
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Kenneth Russell, John Chen.
5 comments:
Patchset:
Thank you!
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #3, Line 581:
['--enable-features=BackForwardCache:enable_same_site/true'])
Interesting. […]
I'm sorry but I found that when about:blank is used as a source or a destination, back-forward cache is not used...
File content/test/gpu/gpu_tests/context_lost_integration_test.py:
Patch Set #4, Line 591:
tab.Navigate
I think this should be self._NavigateAndWaitForLoad instead. tab. […]
It looks like |_NavigateAndWaitForLoad| with |self.UrlOfStaticFilePath(test_path)| takes forever. |test_path| is file:// URL while |self.UrlOfStaticFilePath(test_path)| is http:// URL, and they made the difference, but I am not sure.
I added while loop to confirm that the URL has changed instead.
Patch Set #4, Line 598:
window.window.I guess the repetition was unintentional?
Done
At this point, the back navigation might still be in progress, so the JavaScript might be run on the […]
Done
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.
John Chen (Gerrit)
Attention is currently required from: Brian Sheedy, Fergal Daly, Hajime Hoshi, Kenneth Russell.
1 comment:
Patchset:
The test code now looks correct to me, so I'm not sure why the test is still failing. Could it be possible that the back-forward cache itself is working in some unexpected way? Is there some way to enable more debugging output from back-forward cache?
To view, visit change 2617413. To unsubscribe, or for help writing mail filters, visit settings.