Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(427)

Issue 2584113002: DevTools: render progress while picking the go-to-file. (Closed)

Created:
4 years ago by pfeldman
Modified:
4 years ago
Reviewers:
caseq, alph, lushnikov
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, kozyatinskiy+blink_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

DevTools: render progress while picking the go-to-file. Committed: https://crrev.com/f1f9d506f7afb166feba4cf000aa3a1dc630f8fa Cr-Commit-Position: refs/heads/master@{#439682}

Patch Set 1 #

Total comments: 4

Patch Set 2 : rebaselined #

Messages

Total messages: 21 (10 generated)
pfeldman
4 years ago (2016-12-17 01:22:28 UTC) #2
caseq
https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js File third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js (right): https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js#newcode36 third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js:36: this._progressBarElement = this._progressElement.createChild('div', 'filtered-list-widget-progress-bar'); Any reason to not use ...
4 years ago (2016-12-17 01:28:24 UTC) #4
alph
lgtm https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js File third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js (right): https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js#newcode249 third_party/WebKit/Source/devtools/front_end/ui_lazy/FilteredListWidget.js:249: this._progressBarElement.style.transform = 'scaleX(1)'; just curious why it's better ...
4 years ago (2016-12-17 01:36:33 UTC) #6
lushnikov
https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js File third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js (right): https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js#newcode82 third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js:82: const maxDataLength = 256; nit: let's use "var" instead ...
4 years ago (2016-12-19 20:29:46 UTC) #7
alph
https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js File third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js (right): https://codereview.chromium.org/2584113002/diff/1/third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js#newcode82 third_party/WebKit/Source/devtools/front_end/sources/FilePathScoreFunction.js:82: const maxDataLength = 256; On 2016/12/19 20:29:46, lushnikov wrote: ...
4 years ago (2016-12-19 21:44:13 UTC) #8
pfeldman
I believe const-related perf concerns are now resolved, so it is fine to use it.
4 years ago (2016-12-19 22:43:15 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2584113002/1
4 years ago (2016-12-20 00:41:56 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/326527) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, ...
4 years ago (2016-12-20 00:44:46 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2584113002/20001
4 years ago (2016-12-20 01:23:24 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-12-20 02:57:57 UTC) #19
commit-bot: I haz the power
4 years ago (2016-12-20 02:59:50 UTC) #21
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f1f9d506f7afb166feba4cf000aa3a1dc630f8fa
Cr-Commit-Position: refs/heads/master@{#439682}

Powered by Google App Engine
This is Rietveld 408576698