New issue
Advanced search Search tips

Issue 916888 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: CrElementsSliderTest.All



Sign in to add a comment

CrElementsSliderTest.All is flaky

Project Member Reported by Findit, Dec 20

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Dec 20

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

commit 3ce569aa1b336971ef11d0ee04e6073815530417
Author: Lutz Justen <ljusten@chromium.org>
Date: Thu Dec 20 13:02:04 2018

[Sheriff] Revert "WebUI: cr-slider, remove value-changed event in favor of cr-slider-value-changed"

This reverts commit 8952c673cb0070f8a164146788bc75f33a3a8cc4.

Reason for revert: Causes flakes in CrElementsSliderTest.All.

Original change's description:
> WebUI: cr-slider, remove value-changed event in favor of cr-slider-value-changed
> 
> When |value| is updated either manually or through the UI, value-changed events
> were being fired. This could cause two issues: momentary invalid values and
> feedback loops.
> 
> When |value| is manually updated continuously to a valid value, a value-changed
> event is not necessary. The slider should accept whatever the latest updated
> |value| is and not update the source of the manual update with a potentially
> stale value.
> 
> When the slider is being interacted with via the mouse, a change event is only
> needed when the value is in a valid state. Intermediate values between
> discrete values or outside the min/max bounds should be corrected and once
> corrected fire an event. This is the root cause of the linked bug.
> 
> Bug:  913404 
> Change-Id: I842d1a27ae5a434ffe7d63efa7c7db3f00198cf8
> Reviewed-on: https://chromium-review.googlesource.com/c/1372268
> Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#618068}

TBR=dpapad@chromium.org,sammc@chromium.org,aee@chromium.org

Change-Id: Ie0839f0600393bbd38f86b96695da43e1abc1cdd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  913404 ,  916888 
Reviewed-on: https://chromium-review.googlesource.com/c/1386433
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618186}
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/chrome/browser/resources/settings/controls/settings_slider.html
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/chrome/browser/resources/settings/controls/settings_slider.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/chrome/browser/resources/settings/device_page/display.html
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/chrome/test/data/webui/cr_elements/cr_slider_test.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/chrome/test/data/webui/settings/device_page_tests.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/chrome/test/data/webui/settings/settings_slider_tests.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/ui/file_manager/audio_player/elements/control_panel.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/ui/file_manager/gallery/js/image_editor/image_editor_toolbar.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/ui/file_manager/video_player/js/media_controls.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/ui/file_manager/video_player/js/video_player.js
[modify] https://crrev.com/3ce569aa1b336971ef11d0ee04e6073815530417/ui/webui/resources/cr_elements/cr_slider/cr_slider.js

Labels: -Sheriff-Chromium
Status: Assigned (was: Untriaged)
[Sheriff]

Cannot read property 'has' of null", source: chrome://resources/cr_elements/cr_slider/cr_slider.js (290)

Cannot read property 'add' of null (chrome://resources/cr_elements/cr_slider/cr_slider.js:346)

This means both this.deltaKeyMap_ and this.draggingEventTracker_ are null, which means attached() was never called.

[9056:9056:1219/200229.695802:ERROR:CONSOLE(48)] "Mocha test failed: cr-slider key events
Error: Uncaught TypeError: Cannot read property 'has' of null (chrome://resources/cr_elements/cr_slider/cr_slider.js:290)
    at keyEventOn (file:///b/s/w/ir/third_party/polymer/v1_0/components-chromium/iron-test-helpers/mock-interactions.js:398:12)
    at keyDownOn (file:///b/s/w/ir/third_party/polymer/v1_0/components-chromium/iron-test-helpers/mock-interactions.js:411:5)
    at Object.pressAndReleaseKeyOn (file:///b/s/w/ir/third_party/polymer/v1_0/components-chromium/iron-test-helpers/mock-interactions.js:438:5)
    at pressArrowRight (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:26:22)
    at Context.test (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:81:5)
", source: file:///b/s/w/ir/chrome/test/data/webui/mocha_adapter.js (48)
[9056:9056:1219/200229.695852:ERROR:CONSOLE(290)] "Uncaught TypeError: Cannot read property 'has' of null", source: chrome://resources/cr_elements/cr_slider/cr_slider.js (290)
[9056:9056:1219/200229.697206:ERROR:CONSOLE(48)] "Mocha test failed: cr-slider key events
AssertionError: expected 0 to equal 1
    at Function.assert.strictEqual (file:///b/s/w/ir/third_party/chaijs/chai.js:2277:32)
    at assertEquals (file:///b/s/w/ir/chrome/test/data/webui/test_api.js:923:15)
    at Context.test (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:82:5)
", source: file:///b/s/w/ir/chrome/test/data/webui/mocha_adapter.js (48)
[9056:9056:1219/200229.701095:ERROR:CONSOLE(48)] "Mocha test failed: cr-slider mouse events
Error: Uncaught TypeError: Cannot read property 'add' of null (chrome://resources/cr_elements/cr_slider/cr_slider.js:346)
    at pointerEvent (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:59:14)
    at pointerDown (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:67:5)
    at Context.test (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:131:5)
", source: file:///b/s/w/ir/chrome/test/data/webui/mocha_adapter.js (48)
[9056:9056:1219/200229.701134:ERROR:CONSOLE(346)] "Uncaught TypeError: Cannot read property 'add' of null", source: chrome://resources/cr_elements/cr_slider/cr_slider.js (346)
[9056:9056:1219/200229.701309:ERROR:CONSOLE(48)] "Mocha test failed: cr-slider mouse events
TypeError: Cannot read property 'listeners_' of null
    at Context.test (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:133:52)
", source: file:///b/s/w/ir/chrome/test/data/webui/mocha_adapter.js (48)
[9056:9056:1219/200229.703440:ERROR:CONSOLE(346)] "Uncaught TypeError: Cannot read property 'add' of null", source: chrome://resources/cr_elements/cr_slider/cr_slider.js (346)
[9056:9056:1219/200229.703618:ERROR:CONSOLE(48)] "Mocha test failed: cr-slider mouse events
TypeError: Cannot read property 'listeners_' of null
    at Context.test (file:///b/s/w/ir/chrome/test/data/webui/cr_elements/cr_slider_test.js:133:52)
", source: file:///b/s/w/ir/chrome/test/data/webui/mocha_adapter.js (48)
[9056:9056:1219/200229.705535:ERROR:CONSOLE(48)] "Mocha test failed: cr-slider ticks and aria
Error: Uncaught TypeError: Cannot read property 'has' of null (chrome://resources/cr_elements/cr_slider/cr_slider.js:290)
Components: UI>Browser>WebUI
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 20

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

commit 370bd1fbfe08db1498ba360cbc4650df90d8dbfe
Author: Esmael El-Moslimany <aee@chromium.org>
Date: Thu Dec 20 22:58:12 2018

Settings WebUI: fix flaky test, add a wait to setup before running test cases

Patchset 1 is the revert. Patchset 2 is the fix.

Bug:  916888 
Change-Id: I16e2fc111c8ca6e3477414f4ed893bd4da0e2087
Reviewed-on: https://chromium-review.googlesource.com/c/1387386
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618366}
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/chrome/browser/resources/settings/controls/settings_slider.html
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/chrome/browser/resources/settings/controls/settings_slider.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/chrome/browser/resources/settings/device_page/display.html
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/chrome/test/data/webui/cr_elements/cr_slider_test.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/chrome/test/data/webui/settings/device_page_tests.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/chrome/test/data/webui/settings/settings_slider_tests.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/ui/file_manager/audio_player/elements/control_panel.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/ui/file_manager/gallery/js/image_editor/image_editor_toolbar.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/ui/file_manager/video_player/js/media_controls.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/ui/file_manager/video_player/js/video_player.js
[modify] https://crrev.com/370bd1fbfe08db1498ba360cbc4650df90d8dbfe/ui/webui/resources/cr_elements/cr_slider/cr_slider.js

Status: Fixed (was: Started)

Sign in to add a comment