DevTools: Convert FilteredListWidget to use an optional provider

DevTools: Convert FilteredListWidget to use an optional provider

Instead of a single required delegate, FilteredListWidget now uses a
provider that can be changed.

BUG=662081
Review-Url: https://codereview.chromium.org/2736853002
Cr-Commit-Position: refs/heads/master@{#455611}
Committed: https://chromium.googlesource.com/chromium/src/+/3854543fdcea4ffd600b2a0fbef5ef59c4ba1388

[einbinder, 2017-03-07 01:57:31]

einbinder@chromium.org changed reviewers:
+ pfeldman@chromium.org

[einbinder, 2017-03-07 01:57:32]

ptal

[pfeldman, 2017-03-07 02:17:26]

https://codereview.chromium.org/2736853002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js
(right):

https://codereview.chromium.org/2736853002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:119:
setProvider(provider) {
Annotate provider please.

https://codereview.chromium.org/2736853002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:484:
this._refreshCallback = refreshCallback;
1) why is it now on wasShown
2) could we simplify things via simply exposing refresh (_itemsLoaded) on the
widget?

[einbinder, 2017-03-07 04:27:27]

https://codereview.chromium.org/2736853002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js
(right):

https://codereview.chromium.org/2736853002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:119:
setProvider(provider) {
On 2017/03/07 at 02:17:26, pfeldman wrote:
> Annotate provider please.

Done.

https://codereview.chromium.org/2736853002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:484:
this._refreshCallback = refreshCallback;
On 2017/03/07 at 02:17:26, pfeldman wrote:
> 1) why is it now on wasShown
> 2) could we simplify things via simply exposing refresh (_itemsLoaded) on the
widget?

Attach and detach. Bound provider to _itemsLoaded.

[einbinder, 2017-03-07 06:03:58]

The CQ bit was checked by einbinder@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 06:04:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 08:55:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 08:55:07]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[pfeldman, 2017-03-07 21:55:05]

lgtm % nits

https://codereview.chromium.org/2736853002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js
(right):

https://codereview.chromium.org/2736853002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:198:
this._loadTimeout = setTimeout(this._updateAfterItemsLoaded.bind(this), 0);
You are doing another async hop that could be updating the provider. You should
bind provider again and plumb it further instead of the check above.

[einbinder, 2017-03-07 22:18:34]

https://codereview.chromium.org/2736853002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js
(right):

https://codereview.chromium.org/2736853002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:198:
this._loadTimeout = setTimeout(this._updateAfterItemsLoaded.bind(this), 0);
On 2017/03/07 at 21:55:04, pfeldman wrote:
> You are doing another async hop that could be updating the provider. You
should bind provider again and plumb it further instead of the check above.

If provider is changed between _itemsLoaded and _updateAfterItemsLoaded, the
timers will have been cleared and _updateAfterItemsLoaded won't run. The check
needs to be here to make sure that a foreign provider can't modify _loadTimeout.

I can add another check to _updateAfterItemsLoaded, but I'm not sure how it
could fire so I don't know whether to put it before or after delete
this._loadTimeout.

[pfeldman, 2017-03-08 02:02:06]

lgtm

[einbinder, 2017-03-08 02:05:56]

The CQ bit was checked by einbinder@chromium.org

[commit-bot: I haz the power, 2017-03-08 02:07:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 03:26:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 03:26:18]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[einbinder, 2017-03-08 23:12:58]

The CQ bit was checked by einbinder@chromium.org

[commit-bot: I haz the power, 2017-03-08 23:13:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 00:30:15]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489014778529630,
"parent_rev": "974594311dcca2f77edf9d5b72d6cdbf58b1303e", "commit_rev":
"3854543fdcea4ffd600b2a0fbef5ef59c4ba1388"}

[commit-bot: I haz the power, 2017-03-09 00:31:00]

Description was changed from

==========
DevTools: Convert FilteredListWidget to use an optional provider

Instead of a single required delegate, FilteredListWidget now uses a
provider that can be changed.

BUG=662081
==========

to

==========
DevTools: Convert FilteredListWidget to use an optional provider

Instead of a single required delegate, FilteredListWidget now uses a
provider that can be changed.

BUG=662081

Review-Url: https://codereview.chromium.org/2736853002
Cr-Commit-Position: refs/heads/master@{#455611}
Committed:
https://chromium.googlesource.com/chromium/src/+/3854543fdcea4ffd600b2a0fbef5...
==========

[commit-bot: I haz the power, 2017-03-09 00:31:02]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/3854543fdcea4ffd600b2a0fbef5...