New issue
Advanced search Search tips

Issue 853990 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-06-19
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Chrome OS accessibility tests DCHECK when Refresh is enabled

Project Member Reported by bsep@chromium.org, Jun 19 2018

Issue description

A bunch of tests are crashing on the DCHECK in layer.cc:1241 when Refresh is enabled. In the case of StickyKeysBrowserTest the output claims the layer bounds are 0x0 but the mask layer bounds are 28x28. In the case of SelectToSpeakTest the output claims the layer bounds are 36x28 but the mask layer bounds are 28x28. It seems to be crashing when the message loop is running, so I think it's basically unrelated to what the test is actually trying to verify.

Full list of failing tests:

StickyKeysBrowserTest.CtrlClickHomeButton

SelectToSpeakTest.SelectToSpeakTrayNotSpoken
SelectToSpeakTest.SmoothlyReadsAcrossFormattedText
SelectToSpeakTest.FocusRingMovesWithMouse
SelectToSpeakTest.SmoothlyReadsAcrossMultipleLines
SelectToSpeakTest.WorksWithStickyKeys
SelectToSpeakTest.SmoothlyReadsAcrossInlineUrl
SelectToSpeakTest.ReadsStaticTextWithoutInlineTextChildren
SelectToSpeakTest.ContinuesReadingDuringResize
SelectToSpeakTest.BreaksAtParagraphBounds
 

Comment 1 by bsep@chromium.org, Jun 19 2018

Components: UI>Accessibility>SelectToSpeak
Owner: tapted@chromium.org
Here is an example bot failure: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-chromeos-rel/28097

It seems interactive_ui_tests don't play nicely with gdb, so that exhausts my knowledge of what to do, so I need someone more familiar with Chrome OS to dig into this.

tapted@, can you triage?

Comment 2 by tapted@chromium.org, Jun 19 2018

Owner: dtseng@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by dtseng@chromium.org, Jun 19 2018

NextAction: 2018-06-19
What is Refresh?

Comment 4 by bsep@chromium.org, Jun 19 2018

#3: A new UI for the top chrome. You can try it with --top-chrome-md=material-refresh.

To debug the tests, I recommend changing the last line of MaterialDesignController::DefaultMode (material_design_controller.cc:168) from MATERIAL_NORMAL to MATERIAL_REFRESH to enable it by default.

Comment 5 by bsep@chromium.org, Jun 20 2018

Cc: abodenha@chromium.org
I have crrev.com/c/1107272 out which fixes the SelectToSpeakTest suite. But there's still some kind of problem with clicking on the Home button in StickyKeysBrowserTest.CtrlClickHomeButton. I'm guessing that the incorrect coordinates in SelectToSpeakTest were causing it to click a bookmark bar or toolbar button instead.

Comment 6 by bsep@chromium.org, Jun 20 2018

Actually, if I ctrl-click the Home button, it gives the same DCHECK... it may be worth debugging SelectToSpeakTest too even after it's fixed to see what it's clicking on, in case it's hitting a different problematic area.
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 20 2018

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

commit c9ee3bad44e8a006a05ce551c9d2ef28ce6925c5
Author: Bret Sepulveda <bsep@chromium.org>
Date: Wed Jun 20 21:00:58 2018

Disable StickyKeysBrowserTest.CtrlClickHomeButton when Refresh is on.

There is a DCHECK that triggers in cc::Layer when running the test.
This patch disables the tests temporarily so the feature can be turned
on by default. We will follow up before launch.

Bug:  853990 
Change-Id: I6033d49b11ab341079152ca876d198fc7cdfd84a
Reviewed-on: https://chromium-review.googlesource.com/1105503
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569010}
[modify] https://crrev.com/c9ee3bad44e8a006a05ce551c9d2ef28ce6925c5/chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc

Comment 8 by bsep@chromium.org, Jun 20 2018

Components: -UI>Accessibility>SelectToSpeak UI>Browser>Core
Owner: bsep@chromium.org
I managed to reproduce this locally on Windows, so I'll take over and fix it now that I can debug it.

Steps:
1. If the home button is visible, go to about:settings and disable the home button, then restart the browser.
2. Go to about:settings and enable the home button.
3. Ctrl-Click the home button. (It seems to crash on hover sometimes too.)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 20 2018

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

commit 715862a72d01559f46457bd0fcfda37bc1d6e92b
Author: Bret Sepulveda <bsep@chromium.org>
Date: Wed Jun 20 21:06:43 2018

Fix SelectToSpeakTest suite when Refresh is enabled.

The top chrome height increases when Refresh is enabled, so the insets
into the browser window to click on the web content were incorrect.

Bug:  846410 ,  853990 
Change-Id: I2f274632ca752d583a8fd0ac21721039c3cc8c1b
Reviewed-on: https://chromium-review.googlesource.com/1107272
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569013}
[modify] https://crrev.com/715862a72d01559f46457bd0fcfda37bc1d6e92b/chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc

Is this still an issue?

Comment 11 by bsep@chromium.org, Jun 21 2018

#10: Yes, but I have a CL out to fix it (crrev.com/c/1109270)
Project Member

Comment 12 by sheriffbot@chromium.org, Jun 22 2018

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 22 2018

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

commit 74064adc6b0cf6a88670b623d9a440234a50213e
Author: Bret Sepulveda <bsep@chromium.org>
Date: Fri Jun 22 23:12:19 2018

Fix home button not having an ink drop when toggled on.

The home button's width was being set to 0 when it was off, which caused
its ink drop to be also set to size 0, but the ink drop isn't updated
when the bounds change later. This patch fixes this by having the home
button's width never be set to 0.

A test was hitting a DCHECK because of this, so that test is re-enabled.

Bug:  853990 
Change-Id: I5313aead457c4c51db6c8201ad9bc8e652935e52
Reviewed-on: https://chromium-review.googlesource.com/1109270
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569826}
[modify] https://crrev.com/74064adc6b0cf6a88670b623d9a440234a50213e/chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc
[modify] https://crrev.com/74064adc6b0cf6a88670b623d9a440234a50213e/chrome/browser/ui/views/toolbar/toolbar_view.cc

Comment 14 by bsep@chromium.org, Jun 22 2018

Status: Fixed (was: Assigned)

Sign in to add a comment