Issue metadata
Sign in to add a comment
|
Web contents randomly darker than expected |
||||||||||||||||||||
Issue descriptionSeemingly at random, the web contents in Chrome on Android can be slightly darker than expected. For example, on a blank page with a red background, we expect an RGB value of (255, 0, 0), but it's sometimes (218, 12, 12). This was originally caught in a VR pixel diff test [1], as the color difference was causing flakiness. However, I hacked in some code to sample some of the content's pixels from a screenshot before entering VR, and the issue still occurs then. So, this appears to be an issue in Chrome as a whole. Unfortunately, this test is rather new and seems to have been exhibiting the issue from the start, so we can't use it to help track down a potential culprit CL. This appears to be happening much more frequently on the Pixel 1 XL devices with O (N repros much less commonly), and I haven't been able to repro locally on my Pixel 2 with O. This should be reproducible by just loading a blank page (although I haven't tried with about:blank) and checking the web content's color. [1] https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/vr/VrBrowserDialogTest.java?q=vrbrowserdialogtest&sq=package:chromium&dr=CSs&l=163
,
Nov 27
,
Nov 27
,
Nov 28
That should be suitable. I was specifically using: <!doctype html> <html> <body bgcolor="FF0000"></body> </html> but I imagine the problem should repro with a div, too.
,
Nov 29
Unable to reproduce this issue on Pixel 2 , Pixel 2 XL and Pixel XL using file in c#4 and c#5. Always seeing light/expected color as in first screenshot in c#1. From c#0 this seems to be inconsistent. Could you please help in triaging this further. Thanks!
,
Nov 29
I can reproduce pretty consistently on swarming in instrumentation tests - would that be of any use? If there's some sort of tracing or something we can enable to see what's happening, I could enable that and trigger a test run. For still trying to reproduce locally, the devices that reproduce most consistently are https://chromium-swarm.appspot.com/botlist?c=id&c=task&c=os&c=status&d=asc&f=device_type%3Amarlin&f=device_os%3AO&k=device_os&s=id So that's a Pixel XL with OPR3.170623.008-userdebug and the CPU governor set to powersave.
,
Nov 29
I threw together an automated test to specifically try to reproduce the issue https://chromium-review.googlesource.com/c/chromium/src/+/1355254 I ran it locally on my Pixel 2, and it seems to reproduce almost 100% of the time with that. Once that CL is patched in,you can build the "chrome_public_test_apk" target then run the following command to run the test: out/<output_directory>/bin/run_chrome_public_test_apk --num-retries 0 --test-filter ChromeActivityTest#testDebugDarkContentIssue --local-output --json-results-file output.json The --local-output and --json-results-file output.json arguments will cause a file URL to be output at the end that allows you to get logcat output and a screenshot of the test when it failed (to visually confirm that the content is indeed darker). The GN args I used are: ffmpeg_branding = "Chrome" is_component_build = false is_debug = true proprietary_codecs = true strip_absolute_paths_from_debug_symbols = true symbol_level = 1 target_cpu = "arm64" target_os = "android" use_goma = true
,
Nov 29
Well, it's managed to stop reproducing locally for me without any changes. It was definitely hitting the same failure state initially, though. See attached screenshot, which was automatically taken when the red value was found to not be pure red.
,
Nov 30
@bsheedy: As per c#9 this is not consistently reproducible. Could you please let us know how to proceed further. Thanks!
,
Nov 30
The NextAction date has arrived: 2018-11-30
,
Nov 30
chelamcherla@, did you attempt the repro steps in c#8 with either (or both) of a Pixel 2 or a Pixel XL (ideally with OPR3.170623.008-userdebug and the CPU governor set to powersave)? With this level of instability and hardware-specificity, this doesn't seem like a paint issue to me. But I could be totally wrong. ericrk@, pdr@, what do you think?
,
Nov 30
So I managed to get it reproducing again locally (not sure what I did), and with that, I have a bit more information about the potential cause. It looks like this might be some issue with the automatic screen dimming on Android devices when they haven't received any input for several seconds. Automated tests are run on devices that are set to never turn off their screen while tests are running, but AFAIK there's nothing to prevent the screen from going dim, as that *shouldn't* have any affect on testing at all. So when the issue repros locally for me, it repros 100% of the time. While I was running the test repeatedly, I noticed that the test always seemed to be running when the screen was dimmed. This shouldn't have any effect on screenshots or anything else as far as the OS/apps are concerned - it should just be adjusting the backlight brightness on the screen itself. Out of curiosity, I touched the screen in between test runs so that it was no longer dim, and suddenly the test started passing 100% of the time. This continues even after the screen dims itself again, and I have yet to find a way to get it back to reproducing (reboots, replugging, reinstalling all the test files, etc. all have no effect). So, this raises a few questions: 1. If this is actually related to the dimming, why does it seem to get fixed after a single screen touch instead of being directly related to the screen's brightness. 2. Why does this only affect the web contents? In the above screenshot, everything besides the web contents are correctly colored/not dimmed. 3. Is this *actually* related to screen brightness, or is Android doing something else completely unrelated when we touch the screen?
,
Nov 30
Synced to ToT, rebuilt, and the issue started reproing again. I've also noticed that when this reproduces, the first time the APK used for the server that serves the webpage crashes and we get an "app not responding" dialog that the test runner closes. Almost certainly related to this issue, although I'm not sure whether it's the server crashing or the automated touch from the test runner that's the actual cause...
,
Nov 30
Well, I found the culprit, but I have NO idea how it actually causes the issue. The test runner uses ADB key events to select and press the "close" button on the ANR https://cs.chromium.org/chromium/src/third_party/catapult/devil/devil/android/device_utils.py?q=dismiss+file:%5Esrc/third_party/catapult/devil/+package:%5Echromium$&dr=CSs&l=2708, and those keyevents are somehow causing this. E.g. the order of events to repro this locally: <0% repo> adb shell input keyevent 22 // keycode for the right arrow key <100% repro> <touch screen> <0% repro> Because this is only really doable with automation, we don't have to worry about this being a user-visible issue. However, I'm REALLY confused about how this affects the web contents.
,
Dec 3
The NextAction date has arrived: 2018-12-03
,
Dec 3
Random guess: Screen dimming modifies the color profiles? I presume this is now WontFix. Is that right?
,
Dec 3
As per c#15, it's definitely not related to screen dimming. It's some weird interaction between keyboard input on Android and Chrome. Whether it's something we can actually fix I'm not sure, but at the very least, the test runner should try to work around this.
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/9f1ac6044fbbf9ae88f4f687aa02ec2f664f947f commit 9f1ac6044fbbf9ae88f4f687aa02ec2f664f947f Author: bsheedy <bsheedy@chromium.org> Date: Tue Dec 04 00:18:32 2018 devil: Workaround adb keyevent issue when dismissing ANR dialogs Adds a workaround for a weird Android issue where using adb to input keyevents causes Chrome's web contents to become darker. This is worked around by inputting a no-op touchscreen input after keyevents are input to close an ANR dialog, which causes the issue to go away. Bug: chromium:908917 Change-Id: I67c2737db3e63ecc3d8cda321dc69344a3239585 Reviewed-on: https://chromium-review.googlesource.com/c/1358889 Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Reviewed-by: Ben Pastene <bpastene@chromium.org> [modify] https://crrev.com/9f1ac6044fbbf9ae88f4f687aa02ec2f664f947f/devil/devil/android/device_utils.py [modify] https://crrev.com/9f1ac6044fbbf9ae88f4f687aa02ec2f664f947f/devil/devil/android/device_utils_test.py
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b203448b6c739b933c340dadfc46b692ff60d26 commit 4b203448b6c739b933c340dadfc46b692ff60d26 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Tue Dec 04 05:21:20 2018 Roll src/third_party/catapult 4feee5840de2..bec19e3e3951 (2 commits) https://chromium.googlesource.com/catapult.git/+log/4feee5840de2..bec19e3e3951 git log 4feee5840de2..bec19e3e3951 --date=short --no-merges --format='%ad %ae %s' 2018-12-04 dpranke@chromium.org Add glob support to test expectations files in typ. 2018-12-04 bsheedy@chromium.org devil: Workaround adb keyevent issue when dismissing ANR dialogs Created with: gclient setdep -r src/third_party/catapult@bec19e3e3951 The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. 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 BUG= chromium:835690 , chromium:908917 TBR=sullivan@chromium.org Change-Id: I39f2d54f018f6455404f518e5cb75e4d2eccd50d Reviewed-on: https://chromium-review.googlesource.com/c/1360157 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#613432} [modify] https://crrev.com/4b203448b6c739b933c340dadfc46b692ff60d26/DEPS
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b38b5d3ba2bc4e343b756f932ac7b807ffa94da commit 8b38b5d3ba2bc4e343b756f932ac7b807ffa94da Author: bsheedy <bsheedy@chromium.org> Date: Thu Dec 06 03:06:39 2018 Add XR instrumentation web content workaround Adds a workaround for https://crbug.com/908917 into the XR instrumentation test setup since attempts to work around the issue in the test runner haven't been completely successful. This workaround performs a swipe which should be a no-op when running a RenderTest, which seems to prevent the flakiness when doing pixel diffs. Bug: 908917 Change-Id: I38bf8fb5abe1c0a2bf19e044dd9237919ec2b019 Reviewed-on: https://chromium-review.googlesource.com/c/1362293 Reviewed-by: Bill Orr <billorr@chromium.org> Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#614250} [modify] https://crrev.com/8b38b5d3ba2bc4e343b756f932ac7b807ffa94da/chrome/android/javatests/src/org/chromium/chrome/browser/vr/rules/ChromeTabbedActivityXrTestRule.java [modify] https://crrev.com/8b38b5d3ba2bc4e343b756f932ac7b807ffa94da/chrome/android/javatests/src/org/chromium/chrome/browser/vr/rules/CustomTabActivityXrTestRule.java [modify] https://crrev.com/8b38b5d3ba2bc4e343b756f932ac7b807ffa94da/chrome/android/javatests/src/org/chromium/chrome/browser/vr/rules/WebappActivityXrTestRule.java [modify] https://crrev.com/8b38b5d3ba2bc4e343b756f932ac7b807ffa94da/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/VrTestRuleUtils.java [modify] https://crrev.com/8b38b5d3ba2bc4e343b756f932ac7b807ffa94da/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/XrTestRuleUtils.java
,
Dec 6
Update on this: mthiesse@ seems to have found the root cause. Apparently keyboard input will bring Android out of touch mode, and being out of touch mode means that focus and selection are enabled. He has a fix that I was able to test locally, should be up once the current git outage is resolved.
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d732d214b84113106a00eba6c8aaca3304c7998 commit 1d732d214b84113106a00eba6c8aaca3304c7998 Author: Michael Thiessen <mthiesse@chromium.org> Date: Fri Dec 07 19:30:51 2018 Disable focus highlight overlay for non-touch mode on ContentView. On O+, Android draws a translucent grey overlay over focused views when the device is not in touch mode (as in, the user is using keyboard to navigate rather than touch/mouse). Drawing this overlay over the entire ContentView doesn't make sense, so we should just disable it. We also have to disable it for the CompositorViewHolder, or when we go between NativePages (like NTP) and WebContents, we animate into and out of the grey overlay, because the CompositorViewHolder briefly gains focus. Bug: 912724, 908917 Change-Id: I97b7f688f06581e872898028a48e6e7b4546d710 Reviewed-on: https://chromium-review.googlesource.com/c/1367926 Reviewed-by: Yaron Friedman <yfriedman@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Reviewed-by: Matthew Jones <mdjones@chromium.org> Commit-Queue: Michael Thiessen <mthiesse@chromium.org> Cr-Commit-Position: refs/heads/master@{#614783} [modify] https://crrev.com/1d732d214b84113106a00eba6c8aaca3304c7998/base/android/java/src/org/chromium/base/compat/ApiHelperForO.java [modify] https://crrev.com/1d732d214b84113106a00eba6c8aaca3304c7998/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java [modify] https://crrev.com/1d732d214b84113106a00eba6c8aaca3304c7998/components/embedder_support/android/java/src/org/chromium/components/embedder_support/view/ContentView.java
,
Dec 7
Brian, assigning to you in case you want to undo the workarounds. Otherwise, mark as fixed.
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e6434860d5a477e6dfd9650fdf9a11c771b5a38 commit 2e6434860d5a477e6dfd9650fdf9a11c771b5a38 Author: Brian Sheedy <bsheedy@chromium.org> Date: Fri Dec 07 19:52:08 2018 Revert "Add XR instrumentation web content workaround" This reverts commit 8b38b5d3ba2bc4e343b756f932ac7b807ffa94da. Reason for revert: Root cause fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1367926, workaround no longer necessary. Original change's description: > Add XR instrumentation web content workaround > > Adds a workaround for https://crbug.com/908917 into the XR > instrumentation test setup since attempts to work around the issue in > the test runner haven't been completely successful. This workaround > performs a swipe which should be a no-op when running a RenderTest, > which seems to prevent the flakiness when doing pixel diffs. > > Bug: 908917 > Change-Id: I38bf8fb5abe1c0a2bf19e044dd9237919ec2b019 > Reviewed-on: https://chromium-review.googlesource.com/c/1362293 > Reviewed-by: Bill Orr <billorr@chromium.org> > Commit-Queue: Brian Sheedy <bsheedy@chromium.org> > Cr-Commit-Position: refs/heads/master@{#614250} TBR=bsheedy@chromium.org,billorr@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 908917 Change-Id: Ie8ca239a9199cf805fb4f4d3257c1b142b3250d7 Reviewed-on: https://chromium-review.googlesource.com/c/1368037 Reviewed-by: Brian Sheedy <bsheedy@chromium.org> Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#614788} [modify] https://crrev.com/2e6434860d5a477e6dfd9650fdf9a11c771b5a38/chrome/android/javatests/src/org/chromium/chrome/browser/vr/rules/ChromeTabbedActivityXrTestRule.java [modify] https://crrev.com/2e6434860d5a477e6dfd9650fdf9a11c771b5a38/chrome/android/javatests/src/org/chromium/chrome/browser/vr/rules/CustomTabActivityXrTestRule.java [modify] https://crrev.com/2e6434860d5a477e6dfd9650fdf9a11c771b5a38/chrome/android/javatests/src/org/chromium/chrome/browser/vr/rules/WebappActivityXrTestRule.java [modify] https://crrev.com/2e6434860d5a477e6dfd9650fdf9a11c771b5a38/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/VrTestRuleUtils.java [modify] https://crrev.com/2e6434860d5a477e6dfd9650fdf9a11c771b5a38/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/XrTestRuleUtils.java
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/c017b42db491f6ad7f7d6bf3438b2bec4db13d02 commit c017b42db491f6ad7f7d6bf3438b2bec4db13d02 Author: Brian Sheedy <bsheedy@chromium.org> Date: Fri Dec 07 19:54:43 2018 Revert "devil: Workaround adb keyevent issue when dismissing ANR dialogs" This reverts commit 9f1ac6044fbbf9ae88f4f687aa02ec2f664f947f. Reason for revert: Root cause fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1367926, workaround no longer necessary. Original change's description: > devil: Workaround adb keyevent issue when dismissing ANR dialogs > > Adds a workaround for a weird Android issue where using adb to input > keyevents causes Chrome's web contents to become darker. This is worked > around by inputting a no-op touchscreen input after keyevents are input > to close an ANR dialog, which causes the issue to go away. > > Bug: chromium:908917 > Change-Id: I67c2737db3e63ecc3d8cda321dc69344a3239585 > Reviewed-on: https://chromium-review.googlesource.com/c/1358889 > Commit-Queue: Brian Sheedy <bsheedy@chromium.org> > Reviewed-by: Ben Pastene <bpastene@chromium.org> TBR=bsheedy@chromium.org,bpastene@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:908917 Change-Id: I8ab8057edf4838bdfb44ad7cfec84e0c3f8b41df Reviewed-on: https://chromium-review.googlesource.com/c/1367994 Reviewed-by: Brian Sheedy <bsheedy@chromium.org> Reviewed-by: Ben Pastene <bpastene@chromium.org> Commit-Queue: Brian Sheedy <bsheedy@chromium.org> [modify] https://crrev.com/c017b42db491f6ad7f7d6bf3438b2bec4db13d02/devil/devil/android/device_utils.py [modify] https://crrev.com/c017b42db491f6ad7f7d6bf3438b2bec4db13d02/devil/devil/android/device_utils_test.py
,
Dec 7
This should be fixed now (thanks Michael!). I'll re-open if I see it again.
,
Dec 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ebb97a2207f514996abad1b0f365228423d1475c commit ebb97a2207f514996abad1b0f365228423d1475c Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Sat Dec 08 01:07:43 2018 Roll src/third_party/catapult 62ba44ace70c..c017b42db491 (1 commits) https://chromium.googlesource.com/catapult.git/+log/62ba44ace70c..c017b42db491 git log 62ba44ace70c..c017b42db491 --date=short --no-merges --format='%ad %ae %s' 2018-12-07 bsheedy@chromium.org Revert "devil: Workaround adb keyevent issue when dismissing ANR dialogs" Created with: gclient setdep -r src/third_party/catapult@c017b42db491 The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. 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 BUG= chromium:908917 TBR=sullivan@chromium.org Change-Id: I5380cb9cf326f867b189b7271f1161e452cbef7c Reviewed-on: https://chromium-review.googlesource.com/c/1368488 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#614894} [modify] https://crrev.com/ebb97a2207f514996abad1b0f365228423d1475c/DEPS |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by bsheedy@chromium.org
, Nov 278.1 KB
8.1 KB View Download
8.1 KB
8.1 KB View Download
126 KB
126 KB View Download