Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2182283003:
MD Settings: Flush templates in settings-main before checking (Closed)
|
Created:
4 years, 4 months ago by michaelpg Modified:
4 years, 4 months ago CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@UpdateMocha Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD 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
Committed: https://crrev.com/7f2d9d4a9232dc7062689261a40a76e577d80a25
Cr-Commit-Position: refs/heads/master@{#408074}
Patch Set 1 #
Total comments: 4
Messages
Total messages: 18 (10 generated)
Description was changed from ========== 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 is a proposed fix. R=tommycli@chromium.org BUG=631891,593989 ========== to ========== 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 is a proposed fix. R=tommycli@chromium.org BUG=631891,593989 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by michaelpg@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== 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 is a proposed fix. R=tommycli@chromium.org BUG=631891,593989 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== 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 ==========
The CQ bit was checked by michaelpg@chromium.org
TBR'ing and attempting CQ to unblock other work.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 Committed: https://crrev.com/7f2d9d4a9232dc7062689261a40a76e577d80a25 Cr-Commit-Position: refs/heads/master@{#408074} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/7f2d9d4a9232dc7062689261a40a76e577d80a25 Cr-Commit-Position: refs/heads/master@{#408074}
Message was sent while issue was closed.
https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_main/settings_main.js (right): https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_main/settings_main.js:154: Polymer.dom.flush(); :_(
Message was sent while issue was closed.
https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_main/settings_main.js (right): https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_main/settings_main.js:154: Polymer.dom.flush(); On 2016/07/29 00:11:45, Dan Beam wrote: > :_( there was a ton of discussion about this, but we can talk about it in the stand-up if you like but, some fun facts: * "Insert, append, and remove operations are transacted lazily in certain cases for performance. In order to interrogate the DOM (for example, offsetHeight, getComputedStyle, etc.) immediately after one of these operations, call Polymer.dom.flush() first" * "It should be used sparingly as calling it frequently can negatively impact performance since work is often deferred for efficiency. Calling `Polymer.dom.flush()` is useful, for example, when an element has to measure itself and is unsure about the state of its internal or compoased DOM." * dom-if has a render() function..... which just calls Polymer.dom.flush()
Message was sent while issue was closed.
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_main/settings_main.js (right): https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_main/settings_main.js:154: Polymer.dom.flush(); On 2016/07/29 17:27:15, michaelpg wrote: > On 2016/07/29 00:11:45, Dan Beam wrote: > > :_( > > there was a ton of discussion about this, but we can talk about it in the > stand-up if you like > > but, some fun facts: > > * "Insert, append, and remove operations are transacted lazily in certain cases > for performance. In order to interrogate the DOM (for example, offsetHeight, > getComputedStyle, etc.) immediately after one of these operations, call > Polymer.dom.flush() first" > > * "It should be used sparingly as calling it frequently can negatively impact > performance since work is often deferred for efficiency. Calling > `Polymer.dom.flush()` is useful, for example, when an element has to measure > itself and is unsure about the state of its internal or compoased DOM." > > * dom-if has a render() function..... which just calls Polymer.dom.flush() I didn't say it was wrong, I just said I was said to see it
Message was sent while issue was closed.
https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_main/settings_main.js (right): https://codereview.chromium.org/2182283003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_main/settings_main.js:154: Polymer.dom.flush(); On 2016/07/29 18:28:39, Dan Beam wrote: > On 2016/07/29 17:27:15, michaelpg wrote: > > On 2016/07/29 00:11:45, Dan Beam wrote: > > > :_( > > > > there was a ton of discussion about this, but we can talk about it in the > > stand-up if you like > > > > but, some fun facts: > > > > * "Insert, append, and remove operations are transacted lazily in certain > cases > > for performance. In order to interrogate the DOM (for example, offsetHeight, > > getComputedStyle, etc.) immediately after one of these operations, call > > Polymer.dom.flush() first" > > > > * "It should be used sparingly as calling it frequently can negatively impact > > performance since work is often deferred for efficiency. Calling > > `Polymer.dom.flush()` is useful, for example, when an element has to measure > > itself and is unsure about the state of its internal or compoased DOM." > > > > * dom-if has a render() function..... which just calls Polymer.dom.flush() > > I didn't say it was wrong, I just said I was said to see it I was just sad** to see it |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
