New issue
Advanced search Search tips

Issue 712842 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Can't select external monitor in MD display settings

Project Member Reported by afakhry@chromium.org, Apr 18 2017

Issue description

This bug happens only when I connect the MIMO external monitor to my caroline.

Selecting the USB tab doesn't change the contents of the tab. Selecting the USB monitor rectangle representation doesn't do anything.

Many javascript errors:

Chrome Tester (testing.in.chrome@gmail.com)
[423:423:0418/134502.283958:ERROR:CONSOLE(20)] "Uncaught Error: Assertion failed", source: chrome://settings/crisper.js (20)

Uncaught Error: Assertion failed
   at assert (crisper.js:20)
   at HTMLElement.findNearestIndex_ (crisper.js:172)
   at HTMLElement.valueChanged_ (crisper.js:172)
   at HTMLElement._complexObserverEffect (polymer-extracted.js:1656)
   at HTMLElement._effectEffects (polymer-extracted.js:1491)
   at HTMLElement._propertySetter (polymer-extracted.js:1475)
   at HTMLElement.__setProperty (polymer-extracted.js:1484)
   at HTMLElement._applyEffectValue (polymer-extracted.js:1967)
   at HTMLElement._annotationEffect (polymer-extracted.js:1630)
   at HTMLElement._effectEffects (polymer-extracted.js:1491)


 
Cc: malaykeshav@chromium.org
I don't have convenient access to a USB -> display adapter right now.

Ahmed or Malay, can you build with use_vulcanize = false (in args.gn) and break on exceptions to see if you can identify the root cause? Or just click on the {} ('pretty print') icon at the bottom-left of the source view for crisper.js and debug from there.

It's all because this.$.slider.immediateValue for this USB display is NaN. I think because this monitor only supports a single resolution. I fixed the issue by using this.$.slider.value instead of this.$.slider.immediateValue here: https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/controls/settings_slider.js?q=findNearestIndex+package:%5Echromium$&l=55

Here's some analysis:

with use_vulcanize=false: we hit it from two paths:

1 --------------------------------------------------------------------
Uncaught Error: Assertion failed
    at assert (util.js:27) 
    at HTMLElement.findNearestIndex_ (settings_slider.js:132)
    at HTMLElement.valueChanged_ (settings_slider.js:107)
    at HTMLElement._complexObserverEffect (polymer-extracted.js:1656)
    at HTMLElement._complexObserverPathEffect (polymer-extracted.js:2266)
    at HTMLElement._pathEffector (polymer-extracted.js:2245)
    at HTMLElement._notifyPath (polymer-extracted.js:2146)
    at HTMLElement.set (polymer-extracted.js:2205)
    at HTMLElement.onSliderChanged_ (settings_slider.js:59)
    at HTMLElement.handler (polymer-extracted.js:550)

2 --------------------------------------------------------------------
Uncaught Error: Assertion failed
    at assert (util.js:27)
    at HTMLElement.findNearestIndex_ (settings_slider.js:formatted:131)
    at HTMLElement.valueChanged_ (settings_slider.js:formatted:107)
    at HTMLElement._complexObserverEffect (polymer-extracted.js:1656)
    at HTMLElement._effectEffects (polymer-extracted.js:1491)
    at HTMLElement._propertySetter (polymer-extracted.js:1475)
    at HTMLElement.__setProperty (polymer-extracted.js:1484)
    at HTMLElement._applyEffectValue (polymer-extracted.js:1967)
    at HTMLElement._annotationEffect (polymer-extracted.js:1630)
    at HTMLElement._effectEffects (polymer-extracted.js:1491)


It's hitting this assert: https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/controls/settings_slider.js?q=findNearestIndex+package:%5Echromium$&l=132

The value of |this.pref.value| that we send to this function is undefined: https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/controls/settings_slider.js?q=findNearestIndex+package:%5Echromium$&l=109
Cc: steve...@chromium.org
Owner: afakhry@chromium.org
OK. It's unexpected that immediateValue is NaN, but we can test for that and use this.value instead. Could you put up a CL with that change (since you can test it) and make me the reviewer? Thanks!
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 19 2017

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

commit 77ac96ae13745efb0300a75232dca1251cd5ada2
Author: afakhry <afakhry@chromium.org>
Date: Wed Apr 19 23:33:15 2017

Fix MD display settings when the display supports a single resolution

Use this.$.slider.value instead of this.$slider.immediateValue when
this.$.slider.immediateValue is invalid.

BUG= 712842 
TEST=manually by connected an MIMO external USB display.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2829663004
Cr-Commit-Position: refs/heads/master@{#465807}

[modify] https://crrev.com/77ac96ae13745efb0300a75232dca1251cd5ada2/chrome/browser/resources/settings/controls/settings_slider.js

Status: Fixed (was: Assigned)
Components: UI>Settings
Labels: Merge-Request-59 M-59
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 20 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

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

Comment 8 by bugdroid1@chromium.org, Apr 21 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee13d6d2a7d3ca2b27c897a1567d915f2cf900f6

commit ee13d6d2a7d3ca2b27c897a1567d915f2cf900f6
Author: Ahmed Fakhry <afakhry@google.com>
Date: Fri Apr 21 00:03:57 2017

[Merge to M59] Fix MD display settings when the display supports a single resolution

Use this.$.slider.value instead of this.$slider.immediateValue when
this.$.slider.immediateValue is invalid.

TBR=stevenjb@chromium.org
BUG= 712842 
TEST=manually by connected an MIMO external USB display.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2829663004
Cr-Commit-Position: refs/heads/master@{#465807}
(cherry picked from commit 77ac96ae13745efb0300a75232dca1251cd5ada2)

Review-Url: https://codereview.chromium.org/2832953002 .
Cr-Commit-Position: refs/branch-heads/3071@{#106}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/ee13d6d2a7d3ca2b27c897a1567d915f2cf900f6/chrome/browser/resources/settings/controls/settings_slider.js

Comment 9 Deleted

Labels: -MIMO-external-monitor MIMOexternalmonitor

Comment 11 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment