New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 631891 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocked on:
issue 593989

Blocking:
issue 589681
issue 604517
issue 608115



Sign in to add a comment

MD Settings: browser test failures in settings_main.js (outside of CQ)

Project Member Reported by michae...@chromium.org, Jul 27 2016

Issue description

Changes related to  issue 593989  have made some tests flaky or fragile, which a LOT of CLs in progress have encountered today.

Symptoms: "Script error." or "Uncaught TypeError: Cannot read property '$$' of null" in MD Settings browser tests.

Cause: in settings_main.js currentRouteChanged_ we schedule an async call to overscrollHeight_(). overscrollHeight_() inspects a bunch of elements that live in dom-ifs, so it returns early if the currentRoute property shows those elements won't exist.

However, because other changes to currentRoute can be made in between one currentRouteChanged_ call and the async call to overscrollHeight_(), using async() in currentRouteChanged_ is not sufficient to ensure that the dom-ifs reflect the **current** value of currentRoute while executing overscrollHeight_().

Possible fixes:

* Use setTimeout so other async(fn) operations and dom-if stamping are guaranteed to happen together, before entering overscrollHeight_()
* Call Polymer.dom.flush in overscrollHeight_() after checking currentRoute, to ensure the dom-ifs all accurately reflect currentRoute 
 
I think Polymer.dom.flush() is the most "correct" fix here, as it will definitely prevent some issues we're having with "Cannot read property '$$' of null". Moe's guest visibility CL passes a CQ dry run with this fix: https://codereview.chromium.org/2190453003

Separately, Tommy's patch causes an assertion failure [assert(height >= 0)], which is not fully fixed by any of the above possible fixes. (Has anyone else encountered this assertion failure?) Repro steps for that at https://codereview.chromium.org/2156413002/#msg100
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27 2016

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

commit 7f2d9d4a9232dc7062689261a40a76e577d80a25
Author: michaelpg <michaelpg@chromium.org>
Date: Wed Jul 27 07:55:50 2016

MD Settings: Flush templates in settings-main before checking

An odd interleaving of async calls can cause dom-ifs not to be updated yet
in a function scheduled by async(). This causes expected elements to not
exist at the moment we look for them, even though the properties are correct.

TBR=tommycli@chromium.org
BUG= 631891 , 593989 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/7f2d9d4a9232dc7062689261a40a76e577d80a25/chrome/browser/resources/settings/settings_main/settings_main.js

Labels: Hotlist-MD-Settings-General
Status: Fixed (was: Started)

Sign in to add a comment